[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/scheduler: add gvt notification for guc submission

[Patchwork]

== Series Details ==

Series: drm/i915/scheduler: add gvt notification for guc submission
URL   : https://patchwork.freedesktop.org/series/21514/
State : success

== Summary ==

Series 21514v1 drm/i915/scheduler: add gvt notification for guc submission
https://patchwork.freedesktop.org/api/1.0/series/21514/revisions/1/mbox/

Test kms_cursor_legacy:
Subgroup basic-flip-before-cursor-atomic:
incomplete -> SKIP   (fi-byt-j1900)
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
notrun -> INCOMPLETE (fi-byt-j1900)
notrun -> INCOMPLETE (fi-byt-n2820)
Subgroup basic-plain-flip:
incomplete -> PASS   (fi-byt-n2820)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-b:
incomplete -> PASS   (fi-snb-2520m)
none   -> INCOMPLETE (fi-skl-6700hq)
notrun -> INCOMPLETE (fi-bxt-j4205)
Subgroup hang-read-crc-pipe-c:
notrun -> INCOMPLETE (fi-ivb-3520m)
incomplete -> PASS   (fi-bsw-n3050)
Subgroup read-crc-pipe-b:
incomplete -> PASS   (fi-bxt-j4205)
Subgroup read-crc-pipe-c-frame-sequence:
incomplete -> PASS   (fi-ivb-3520m)
Test kms_setmode:
Subgroup basic:
notrun -> INCOMPLETE (fi-bsw-n3050)
Test prime_busy:
Subgroup basic-after-default:
notrun -> INCOMPLETE (fi-snb-2520m)
Test prime_vgem:
Subgroup basic-fence-wait-default:
incomplete -> PASS   (fi-skl-6770hq)

fi-bdw-5557u total:300  pass:284  dwarn:0   dfail:0   fail:0   skip:16  
time: 621s
fi-bsw-n3050 total:300  pass:219  dwarn:0   dfail:0   fail:0   skip:45  
time: 0s
fi-bxt-j4205 total:257  pass:234  dwarn:0   dfail:0   fail:3   skip:19  
time: 0s
fi-byt-j1900 total:240  pass:212  dwarn:0   dfail:0   fail:0   skip:27  
time: 0s
fi-byt-n2820 total:240  pass:208  dwarn:0   dfail:0   fail:0   skip:31  
time: 0s
fi-hsw-4770  total:300  pass:279  dwarn:0   dfail:0   fail:1   skip:20  
time: 666s
fi-ilk-650   total:300  pass:240  dwarn:0   dfail:0   fail:2   skip:58  
time: 538s
fi-ivb-3520m total:263  pass:238  dwarn:0   dfail:0   fail:5   skip:19  
time: 0s
fi-kbl-7500u total:300  pass:275  dwarn:1   dfail:0   fail:4   skip:20  
time: 626s
fi-skl-6260u total:300  pass:283  dwarn:1   dfail:0   fail:4   skip:12  
time: 647s
fi-skl-6700hqtotal:257  pass:235  dwarn:1   dfail:0   fail:3   skip:17  
time: 0s
fi-skl-6700k total:300  pass:271  dwarn:5   dfail:0   fail:4   skip:20  
time: 664s
fi-skl-6770hqtotal:300  pass:283  dwarn:1   dfail:0   fail:4   skip:12  
time: 699s
fi-snb-2520m total:300  pass:231  dwarn:0   dfail:0   fail:6   skip:35  
time: 0s
fi-snb-2600  total:300  pass:259  dwarn:0   dfail:0   fail:5   skip:36  
time: 602s
fi-hsw-4770r failed to connect after reboot

1e78eaf62e1d2fd47cf6fb974876610a5786d352 drm-tip: 2017y-03m-20d-21h-54m-26s UTC 
integration manifest
e37ece2 drm/i915/scheduler: add gvt notification for guc submission

== Logs ==

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

[Intel-gfx] [PATCH v2] drm/i915/scheduler: add gvt notification for guc submission

[Chuanxiao Dong]

GVT request needs a manual mmio load/restore. Before GuC submit
a request, send notification to gvt for mmio loading. And after
the GuC finished this GVT request, notify gvt again for mmio
restore. This follows the usage when using execlists submission.

v2: use context_status_change instead of execlists_context_status_change
for better understanding (ZhengXiao)

Cc: xiao.zh...@intel.com
Signed-off-by: Chuanxiao Dong 
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 12 
 drivers/gpu/drm/i915/intel_lrc.c   | 21 +++--
 drivers/gpu/drm/i915/intel_lrc.h   | 13 +
 3 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
b/drivers/gpu/drm/i915/i915_guc_submission.c
index a3636b3..68c1e70 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -520,6 +520,12 @@ static void __i915_guc_submit(struct drm_i915_gem_request 
*rq)
unsigned long flags;
int b_ret;
 
+   /* Notify for the context status change schedule in
+* Currently only GVT care this notification for manually
+* context switch, like when using execlist mode submission
+*/
+   context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
+
/* WA to flush out the pending GMADR writes to ring buffer. */
if (i915_vma_is_map_and_fenceable(rq->ring->vma))
POSTING_READ_FW(GUC_STATUS);
@@ -623,6 +629,12 @@ static void i915_guc_irq_handler(unsigned long data)
rq = port[0].request;
while (rq && i915_gem_request_completed(rq)) {
trace_i915_gem_request_out(rq);
+   /* Notify for the context status change schedule
+* out. Currently only GVT care this notification
+* for manually context switch, like when using
+* execlist mode submission
+*/
+   context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
i915_gem_request_put(rq);
port[0].request = port[1].request;
port[1].request = NULL;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index becde55..3a5ed5c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -295,21 +295,6 @@ uint64_t intel_lr_context_descriptor(struct 
i915_gem_context *ctx,
return ctx->engine[engine->id].lrc_desc;
 }
 
-static inline void
-execlists_context_status_change(struct drm_i915_gem_request *rq,
-   unsigned long status)
-{
-   /*
-* Only used when GVT-g is enabled now. When GVT-g is disabled,
-* The compiler should eliminate this function as dead-code.
-*/
-   if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
-   return;
-
-   atomic_notifier_call_chain(>engine->context_status_notifier,
-  status, rq);
-}
-
 static void
 execlists_update_context_pdps(struct i915_hw_ppgtt *ppgtt, u32 *reg_state)
 {
@@ -350,7 +335,7 @@ static void execlists_submit_ports(struct intel_engine_cs 
*engine)
 
GEM_BUG_ON(port[0].count > 1);
if (!port[0].count)
-   execlists_context_status_change(port[0].request,
+   context_status_change(port[0].request,
INTEL_CONTEXT_SCHEDULE_IN);
desc[0] = execlists_update_context(port[0].request);
GEM_DEBUG_EXEC(port[0].context_id = upper_32_bits(desc[0]));
@@ -358,7 +343,7 @@ static void execlists_submit_ports(struct intel_engine_cs 
*engine)
 
if (port[1].request) {
GEM_BUG_ON(port[1].count);
-   execlists_context_status_change(port[1].request,
+   context_status_change(port[1].request,
INTEL_CONTEXT_SCHEDULE_IN);
desc[1] = execlists_update_context(port[1].request);
GEM_DEBUG_EXEC(port[1].context_id = upper_32_bits(desc[1]));
@@ -565,7 +550,7 @@ static void intel_lrc_irq_handler(unsigned long data)
if (--port[0].count == 0) {
GEM_BUG_ON(status & GEN8_CTX_STATUS_PREEMPTED);

GEM_BUG_ON(!i915_gem_request_completed(port[0].request));
-   execlists_context_status_change(port[0].request,
+   context_status_change(port[0].request,

INTEL_CONTEXT_SCHEDULE_OUT);
 
trace_i915_gem_request_out(port[0].request);
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index e8015e7..51e1be9 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ 

Re: [Intel-gfx] [PATCH] drm/i915/scheduler: add gvt notification for guc submission

[Dong, Chuanxiao]

> -Original Message-
> From: Zheng, Xiao
> Sent: Tuesday, March 21, 2017 9:39 AM
> To: Dong, Chuanxiao ; intel-
> g...@lists.freedesktop.org
> Cc: intel-gvt-...@lists.freedesktop.org
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/scheduler: add gvt notification for
> guc submission
> 
> The new guc submission scheduler is not to emulation Execlist mode in driver.
> You can keep existing execlist_xx function name if insisted it is better.

ok got your point. The new guc submission emulate the execlists submission way, 
indeed it is not execlist mode. Will use a clear name in the next version.

> 
> > -Original Message-
> > From: Dong, Chuanxiao
> > Sent: Monday, March 20, 2017 11:20 AM
> > To: Zheng, Xiao ;
> > intel-gfx@lists.freedesktop.org
> > Cc: intel-gvt-...@lists.freedesktop.org
> > Subject: RE: [Intel-gfx] [PATCH] drm/i915/scheduler: add gvt
> > notification for guc submission
> >
> >
> >
> > > -Original Message-
> > > From: Zheng, Xiao
> > > Sent: Monday, March 20, 2017 10:46 AM
> > > To: Dong, Chuanxiao ; intel-
> > > g...@lists.freedesktop.org
> > > Cc: intel-gvt-...@lists.freedesktop.org
> > > Subject: RE: [Intel-gfx] [PATCH] drm/i915/scheduler: add gvt
> > > notification for guc submission
> > >
> > > It may consider to change the function name:
> > > execlists_context_status_change to context_status_change_notify ()
> > instead.
> > > Otherwise confusing GUC submission path.
> > > Thanks.
> >
> > Hi Xiao, I was considering to use the name of
> > context_status_change_notify, but considering the guc submission is
> > actually has an emulation of execlists on top, so back to use the
> > original function name to align with execlists submission, as well as
> > making the code change less. Is this explanation clear enough to use the
> original function name?
> >
> > Thanks
> > Chuanxiao
> >
> > >
> > > > -Original Message-
> > > > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org]
> > > > On Behalf Of Chuanxiao Dong
> > > > Sent: Monday, March 20, 2017 9:49 AM
> > > > To: intel-gfx@lists.freedesktop.org
> > > > Cc: intel-gvt-...@lists.freedesktop.org
> > > > Subject: [Intel-gfx] [PATCH] drm/i915/scheduler: add gvt
> > > > notification for guc submission
> > > >
> > > > GVT request needs a manual mmio load/restore. Before GuC submit a
> > > > request, send notification to gvt for mmio loading. And after the
> > > > GuC finished this GVT request, notify gvt again for mmio restore.
> > > > This follows the usage when using execlists submission.
> > > >
> > > > Signed-off-by: Chuanxiao Dong 
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_guc_submission.c | 13 +
> > > >  drivers/gpu/drm/i915/intel_lrc.c   | 15 ---
> > > >  drivers/gpu/drm/i915/intel_lrc.h   | 14 ++
> > > >  3 files changed, 27 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
> > > > b/drivers/gpu/drm/i915/i915_guc_submission.c
> > > > index a3636b3..328b11c 100644
> > > > --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> > > > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> > > > @@ -520,6 +520,12 @@ static void __i915_guc_submit(struct
> > > > drm_i915_gem_request *rq)
> > > > unsigned long flags;
> > > > int b_ret;
> > > >
> > > > +   /* Notify for the context status change schedule in
> > > > +* Currently only GVT care this notification for manually
> > > > +* context switch, like when using execlist mode submission
> > > > +*/
> > > > +   execlists_context_status_change(rq,
> > > > INTEL_CONTEXT_SCHEDULE_IN);
> > > > +
> > > > /* WA to flush out the pending GMADR writes to ring buffer. */
> > > > if (i915_vma_is_map_and_fenceable(rq->ring->vma))
> > > > POSTING_READ_FW(GUC_STATUS);
> > > > @@ -623,6 +629,13 @@ static void i915_guc_irq_handler(unsigned
> > > > long
> > > data)
> > > > rq = port[0].request;
> > > > while (rq && i915_gem_request_completed(rq)) {
> > > > trace_i915_gem_request_out(rq);
> > > > +   /* Notify for the context status change schedule
> > > > +* out. Currently only GVT care this 
> > > > notification
> > > > +* for manually context switch, like when using
> > > > +* execlist mode submission
> > > > +*/
> > > > +   execlists_context_status_change(rq,
> > > > +
> > > > INTEL_CONTEXT_SCHEDULE_OUT);
> > > > i915_gem_request_put(rq);
> > > > port[0].request = port[1].request;
> > > > port[1].request = NULL;
> > > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> > > > b/drivers/gpu/drm/i915/intel_lrc.c
> > > > index 

Re: [Intel-gfx] [PATCH] drm/i915/scheduler: add gvt notification for guc submission

[Zheng, Xiao]

The new guc submission scheduler is not to emulation Execlist mode in driver.  
You can keep existing execlist_xx function name if insisted it is better.

> -Original Message-
> From: Dong, Chuanxiao
> Sent: Monday, March 20, 2017 11:20 AM
> To: Zheng, Xiao ; intel-gfx@lists.freedesktop.org
> Cc: intel-gvt-...@lists.freedesktop.org
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/scheduler: add gvt notification for
> guc submission
> 
> 
> 
> > -Original Message-
> > From: Zheng, Xiao
> > Sent: Monday, March 20, 2017 10:46 AM
> > To: Dong, Chuanxiao ; intel-
> > g...@lists.freedesktop.org
> > Cc: intel-gvt-...@lists.freedesktop.org
> > Subject: RE: [Intel-gfx] [PATCH] drm/i915/scheduler: add gvt
> > notification for guc submission
> >
> > It may consider to change the function name:
> > execlists_context_status_change to context_status_change_notify ()
> instead.
> > Otherwise confusing GUC submission path.
> > Thanks.
> 
> Hi Xiao, I was considering to use the name of context_status_change_notify,
> but considering the guc submission is actually has an emulation of execlists
> on top, so back to use the original function name to align with execlists
> submission, as well as making the code change less. Is this explanation clear
> enough to use the original function name?
> 
> Thanks
> Chuanxiao
> 
> >
> > > -Original Message-
> > > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On
> > > Behalf Of Chuanxiao Dong
> > > Sent: Monday, March 20, 2017 9:49 AM
> > > To: intel-gfx@lists.freedesktop.org
> > > Cc: intel-gvt-...@lists.freedesktop.org
> > > Subject: [Intel-gfx] [PATCH] drm/i915/scheduler: add gvt
> > > notification for guc submission
> > >
> > > GVT request needs a manual mmio load/restore. Before GuC submit a
> > > request, send notification to gvt for mmio loading. And after the
> > > GuC finished this GVT request, notify gvt again for mmio restore.
> > > This follows the usage when using execlists submission.
> > >
> > > Signed-off-by: Chuanxiao Dong 
> > > ---
> > >  drivers/gpu/drm/i915/i915_guc_submission.c | 13 +
> > >  drivers/gpu/drm/i915/intel_lrc.c   | 15 ---
> > >  drivers/gpu/drm/i915/intel_lrc.h   | 14 ++
> > >  3 files changed, 27 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
> > > b/drivers/gpu/drm/i915/i915_guc_submission.c
> > > index a3636b3..328b11c 100644
> > > --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> > > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> > > @@ -520,6 +520,12 @@ static void __i915_guc_submit(struct
> > > drm_i915_gem_request *rq)
> > >   unsigned long flags;
> > >   int b_ret;
> > >
> > > + /* Notify for the context status change schedule in
> > > +  * Currently only GVT care this notification for manually
> > > +  * context switch, like when using execlist mode submission
> > > +  */
> > > + execlists_context_status_change(rq,
> > > INTEL_CONTEXT_SCHEDULE_IN);
> > > +
> > >   /* WA to flush out the pending GMADR writes to ring buffer. */
> > >   if (i915_vma_is_map_and_fenceable(rq->ring->vma))
> > >   POSTING_READ_FW(GUC_STATUS);
> > > @@ -623,6 +629,13 @@ static void i915_guc_irq_handler(unsigned long
> > data)
> > >   rq = port[0].request;
> > >   while (rq && i915_gem_request_completed(rq)) {
> > >   trace_i915_gem_request_out(rq);
> > > + /* Notify for the context status change schedule
> > > +  * out. Currently only GVT care this notification
> > > +  * for manually context switch, like when using
> > > +  * execlist mode submission
> > > +  */
> > > + execlists_context_status_change(rq,
> > > +
> > >   INTEL_CONTEXT_SCHEDULE_OUT);
> > >   i915_gem_request_put(rq);
> > >   port[0].request = port[1].request;
> > >   port[1].request = NULL;
> > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> > > b/drivers/gpu/drm/i915/intel_lrc.c
> > > index becde55..4f5906b 100644
> > > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > > @@ -295,21 +295,6 @@ uint64_t intel_lr_context_descriptor(struct
> > > i915_gem_context *ctx,
> > >   return ctx->engine[engine->id].lrc_desc;  }
> > >
> > > -static inline void
> > > -execlists_context_status_change(struct drm_i915_gem_request *rq,
> > > - unsigned long status)
> > > -{
> > > - /*
> > > -  * Only used when GVT-g is enabled now. When GVT-g is disabled,
> > > -  * The compiler should eliminate this function as dead-code.
> > > -  */
> > > - if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
> > > - return;
> > > -
> > > - atomic_notifier_call_chain(>engine->context_status_notifier,
> > > -status, rq);
> > > -}
> > > -
> > >  static 

[Intel-gfx] linux-next: manual merge of the drm-intel tree with the drm-intel-fixes tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the drm-intel tree got a conflict in:

  drivers/gpu/drm/i915/gvt/scheduler.c

between commit:

  3cd23b828b37 ("drm/i915/gvt: GVT pin/unpin shadow context")

from the drm-intel-fixes tree and commit:

  e642c85b03de ("drm/i915: Remove superfluous i915_add_request_no_flush() 
helper")

from the drm-intel tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/gpu/drm/i915/gvt/scheduler.c
index 39a83eb7aecc,31d2240fdb1f..
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@@ -246,10 -212,7 +244,10 @@@ out
workload->status = ret;
  
if (!IS_ERR_OR_NULL(rq))
-   i915_add_request_no_flush(rq);
+   i915_add_request(rq);
 +  else
 +  engine->context_unpin(engine, shadow_ctx);
 +
mutex_unlock(_priv->drm.struct_mutex);
return ret;
  }
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] linux-next: manual merge of the drm tree with the drm-intel-fixes tree

[Stephen Rothwell]

Hi Dave,

Today's linux-next merge of the drm tree got a conflict in:

  drivers/gpu/drm/i915/gvt/cmd_parser.c

between commit:

  695fbc08d80f ("drm/i915/gvt: replace the gvt_err with gvt_vgpu_err")

from the drm-intel-fixes tree and commit:

  73dec95e6ba3 ("drm/i915: Emit to ringbuffer directly")

from the drm tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/gpu/drm/i915/gvt/cmd_parser.c
index 2b92cc8a7d1a,81076d8ec41a..
--- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
+++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
@@@ -1673,8 -1644,8 +1673,8 @@@ static int perform_bb_shadow(struct par
ret = copy_gma_to_hva(s->vgpu, s->vgpu->gtt.ggtt_mm,
  gma, gma + bb_size,
  dst);
-   if (ret) {
+   if (ret < 0) {
 -  gvt_err("fail to copy guest ring buffer\n");
 +  gvt_vgpu_err("fail to copy guest ring buffer\n");
goto unmap_src;
}
  
@@@ -2668,26 -2633,23 +2665,24 @@@ static int shadow_workload_ring_buffer(
/* head > tail --> copy head <-> top */
if (gma_head > gma_tail) {
ret = copy_gma_to_hva(vgpu, vgpu->gtt.ggtt_mm,
-   gma_head, gma_top,
-   workload->shadow_ring_buffer_va);
-   if (ret) {
+ gma_head, gma_top, cs);
+   if (ret < 0) {
+   gvt_err("fail to copy guest ring buffer\n");
 +  gvt_vgpu_err("fail to copy guest ring buffer\n");
return ret;
}
-   copy_len = gma_top - gma_head;
+   cs += ret / sizeof(u32);
gma_head = workload->rb_start;
}
  
/* copy head or start <-> tail */
-   ret = copy_gma_to_hva(vgpu, vgpu->gtt.ggtt_mm,
-   gma_head, gma_tail,
-   workload->shadow_ring_buffer_va + copy_len);
-   if (ret) {
+   ret = copy_gma_to_hva(vgpu, vgpu->gtt.ggtt_mm, gma_head, gma_tail, cs);
+   if (ret < 0) {
 -  gvt_err("fail to copy guest ring buffer\n");
 +  gvt_vgpu_err("fail to copy guest ring buffer\n");
return ret;
}
-   ring->tail += workload->rb_len;
-   intel_ring_advance(ring);
+   cs += ret / sizeof(u32);
+   intel_ring_advance(workload->req, cs);
return 0;
  }
  
@@@ -2743,8 -2703,8 +2738,8 @@@ static int shadow_indirect_ctx(struct i
wa_ctx->workload->vgpu->gtt.ggtt_mm,
guest_gma, guest_gma + ctx_size,
map);
-   if (ret) {
+   if (ret < 0) {
 -  gvt_err("fail to copy guest indirect ctx\n");
 +  gvt_vgpu_err("fail to copy guest indirect ctx\n");
goto unmap_src;
}
  
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/uc: Make intel_uc_prepare_fw() static

[Oscar Mateo]

On 03/15/2017 12:38 PM, Michal Wajdeczko wrote:

There is no need to expose this function as it is called from
one function only. Also move it up to avoid forward declaration.

Signed-off-by: Michal Wajdeczko 
Cc: Arkadiusz Hiler 
Cc: Joonas Lahtinen 
---
  drivers/gpu/drm/i915/intel_uc.c | 269 
  drivers/gpu/drm/i915/intel_uc.h |   2 -
  2 files changed, 135 insertions(+), 136 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 54c5aff..a0d8833 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -86,6 +86,141 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
mutex_init(_priv->guc.send_mutex);
  }
  
+static void intel_uc_prepare_fw(struct drm_i915_private *dev_priv,

+   struct intel_uc_fw *uc_fw)
+{
+   struct pci_dev *pdev = dev_priv->drm.pdev;


No need for prefix intel_*, since it's a static function?

-- Oscar
___
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: use static const array for PICK macro

[Patchwork]

== Series Details ==

Series: drm/i915: use static const array for PICK macro
URL   : https://patchwork.freedesktop.org/series/21561/
State : warning

== Summary ==

Series 21561v1 drm/i915: use static const array for PICK macro
https://patchwork.freedesktop.org/api/1.0/series/21561/revisions/1/mbox/

Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass   -> DMESG-WARN (fi-snb-2600)
Test kms_cursor_legacy:
Subgroup basic-flip-before-cursor-atomic:
incomplete -> SKIP   (fi-byt-j1900)
Test kms_flip:
Subgroup basic-flip-vs-modeset:
notrun -> INCOMPLETE (fi-byt-j1900)
Subgroup basic-flip-vs-wf_vblank:
notrun -> INCOMPLETE (fi-byt-n2820)
Subgroup basic-plain-flip:
incomplete -> PASS   (fi-byt-n2820)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-b:
incomplete -> PASS   (fi-snb-2520m)
Subgroup hang-read-crc-pipe-c:
incomplete -> PASS   (fi-bsw-n3050)
notrun -> INCOMPLETE (fi-ivb-3520m)
Subgroup read-crc-pipe-c-frame-sequence:
incomplete -> PASS   (fi-ivb-3520m)
Test pm_rpm:
Subgroup basic-pci-d3-state:
notrun -> INCOMPLETE (fi-bsw-n3050)
Test prime_self_import:
Subgroup basic-llseek-bad:
notrun -> INCOMPLETE (fi-snb-2520m)
Test prime_vgem:
Subgroup basic-fence-wait-default:
incomplete -> PASS   (fi-skl-6770hq)

fi-bdw-5557u total:300  pass:284  dwarn:0   dfail:0   fail:0   skip:16  
time: 617s
fi-bsw-n3050 total:300  pass:221  dwarn:0   dfail:0   fail:1   skip:46  
time: 0s
fi-byt-j1900 total:239  pass:211  dwarn:0   dfail:0   fail:0   skip:27  
time: 0s
fi-byt-n2820 total:300  pass:208  dwarn:0   dfail:0   fail:0   skip:31  
time: 0s
fi-hsw-4770  total:300  pass:279  dwarn:0   dfail:0   fail:1   skip:20  
time: 660s
fi-ilk-650   total:300  pass:240  dwarn:0   dfail:0   fail:2   skip:58  
time: 534s
fi-ivb-3520m total:263  pass:238  dwarn:0   dfail:0   fail:5   skip:19  
time: 0s
fi-kbl-7500u total:300  pass:275  dwarn:1   dfail:0   fail:4   skip:20  
time: 635s
fi-skl-6260u total:300  pass:283  dwarn:1   dfail:0   fail:4   skip:12  
time: 648s
fi-skl-6700k total:300  pass:271  dwarn:5   dfail:0   fail:4   skip:20  
time: 664s
fi-skl-6770hqtotal:300  pass:283  dwarn:1   dfail:0   fail:4   skip:12  
time: 687s
fi-snb-2520m total:300  pass:238  dwarn:0   dfail:0   fail:6   skip:35  
time: 0s
fi-snb-2600  total:300  pass:258  dwarn:1   dfail:0   fail:5   skip:36  
time: 594s
fi-bxt-j4205 failed to connect after reboot
fi-hsw-4770r failed to connect after reboot
fi-skl-6700hq failed to collect. IGT log at Patchwork_4236/fi-skl-6700hq/igt.log

1e78eaf62e1d2fd47cf6fb974876610a5786d352 drm-tip: 2017y-03m-20d-21h-54m-26s UTC 
integration manifest
a058c30 drm/i915: use static const array for PICK macro

== Logs ==

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

[Intel-gfx] [PATCH] drm/i915: use static const array for PICK macro

[Arnd Bergmann]

The varargs macro trick in _PIPE3/_PHY3/_PORT3 was meant as an optimization
to shrink the i915 kernel module by around 1000 bytes. However, the
downside is a size regression with CONFIG_KASAN, as I found from stack size
warnings with gcc-7.0.1:

before:
drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'bxt_ddi_pll_get_hw_state':
drivers/gpu/drm/i915/intel_dpll_mgr.c:1644:1: error: the frame size of 176 
bytes is larger than 100 bytes [-Werror=frame-larger-than=]
drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'bxt_ddi_pll_enable':
drivers/gpu/drm/i915/intel_dpll_mgr.c:1548:1: error: the frame size of 224 
bytes is larger than 100 bytes [-Werror=frame-larger-than=]

after:
drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'bxt_ddi_pll_get_hw_state':
drivers/gpu/drm/i915/intel_dpll_mgr.c:1644:1: error: the frame size of 1016 
bytes is larger than 1000 bytes [-Werror=frame-larger-than=]
drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'bxt_ddi_pll_enable':
drivers/gpu/drm/i915/intel_dpll_mgr.c:1548:1: error: the frame size of 1960 
bytes is larger than 1000 bytes [-Werror=frame-larger-than=]

I also checked the module sizes and got with gcc-7.0.1

original:
   textdata bss dec hex filename
2380830 11554364448 3540714  3606ea drivers/gpu/drm/i915/i915-kasan.o
1298054  5436922884 1844630  1c2596 drivers/gpu/drm/i915/i915-nokasan.o

after ce64645d86ac:
   textdata bss dec hex filename
2389515 11544764448 3548439  362517 drivers/gpu/drm/i915/i915-kasan.o
1299639  5436922884 1846215  1c2bc7 drivers/gpu/drm/i915/i915-nokasan.o

with this patch:
   textdata bss dec hex filename
2381275 11638844448 3549607  3629a7 drivers/gpu/drm/i915/i915-kasan.o
1296038  5436922884 1842614  1c1db6 drivers/gpu/drm/i915/i915-nokasan.o

Actually showing a code size growth in .text both with and without kasan,
and my version gets most of it back at the expense of larger .data when
kasan is enabled.

Fixes: ce64645d86ac ("drm/i915: use variadic macros and arrays to choose 
port/pipe based registers")
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80114
Cc: Jani Nikula 
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/i915/i915_reg.h | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 04c8f69fcc62..39b53878a188 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -48,7 +48,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
return !i915_mmio_reg_equal(reg, INVALID_MMIO_REG);
 }
 
-#define _PICK(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
+#define _PICK(__index, ...) ({static const u32 __arr[] = { __VA_ARGS__ }; 
__arr[__index];})
 
 #define _PIPE(pipe, a, b) ((a) + (pipe)*((b)-(a)))
 #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
@@ -2657,10 +2657,10 @@ enum skl_disp_power_wells {
 /*
  * Clock control & power management
  */
-#define _DPLL_A (dev_priv->info.display_mmio_offset + 0x6014)
-#define _DPLL_B (dev_priv->info.display_mmio_offset + 0x6018)
-#define _CHV_DPLL_C (dev_priv->info.display_mmio_offset + 0x6030)
-#define DPLL(pipe) _MMIO_PIPE3((pipe), _DPLL_A, _DPLL_B, _CHV_DPLL_C)
+#define _DPLL_A0x6014
+#define _DPLL_B0x6018
+#define _CHV_DPLL_C0x6030
+#define DPLL(pipe) _MMIO(dev_priv->info.display_mmio_offset + _PIPE3((pipe), 
_DPLL_A, _DPLL_B, _CHV_DPLL_C))
 
 #define VGA0   _MMIO(0x6000)
 #define VGA1   _MMIO(0x6004)
@@ -2756,10 +2756,10 @@ enum skl_disp_power_wells {
 #define   SDVO_MULTIPLIER_SHIFT_HIRES  4
 #define   SDVO_MULTIPLIER_SHIFT_VGA0
 
-#define _DPLL_A_MD (dev_priv->info.display_mmio_offset + 0x601c)
-#define _DPLL_B_MD (dev_priv->info.display_mmio_offset + 0x6020)
-#define _CHV_DPLL_C_MD (dev_priv->info.display_mmio_offset + 0x603c)
-#define DPLL_MD(pipe) _MMIO_PIPE3((pipe), _DPLL_A_MD, _DPLL_B_MD, 
_CHV_DPLL_C_MD)
+#define _DPLL_A_MD 0x601c
+#define _DPLL_B_MD 0x6020
+#define _CHV_DPLL_C_MD 0x603c
+#define DPLL_MD(pipe) _MMIO(dev_priv->info.display_mmio_offset + 
_PIPE3((pipe), _DPLL_A_MD, _DPLL_B_MD, _CHV_DPLL_C_MD))
 
 /*
  * UDI pixel divider, controlling how many pixels are stuffed into a packet.
-- 
2.9.0

___
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: Remove intel_ring.last_retired_head

[Patchwork]

== Series Details ==

Series: drm/i915: Remove intel_ring.last_retired_head
URL   : https://patchwork.freedesktop.org/series/21559/
State : failure

== Summary ==

Series 21559v1 drm/i915: Remove intel_ring.last_retired_head
https://patchwork.freedesktop.org/api/1.0/series/21559/revisions/1/mbox/

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
fail   -> PASS   (fi-snb-2600) fdo#17
Test gem_exec_reloc:
Subgroup basic-write-cpu-noreloc:
pass   -> INCOMPLETE (fi-hsw-4770r)
Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass   -> DMESG-WARN (fi-snb-2600)
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
pass   -> INCOMPLETE (fi-byt-j1900)
pass   -> INCOMPLETE (fi-byt-n2820)
Test kms_setmode:
Subgroup basic:
notrun -> INCOMPLETE (fi-ivb-3520m)
notrun -> INCOMPLETE (fi-snb-2520m)
Test pm_rpm:
Subgroup basic-pci-d3-state:
pass   -> INCOMPLETE (fi-bsw-n3050)

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

fi-bdw-5557u total:300  pass:283  dwarn:0   dfail:0   fail:1   skip:16  
time: 619s
fi-bsw-n3050 total:300  pass:221  dwarn:0   dfail:0   fail:1   skip:46  
time: 0s
fi-byt-j1900 total:240  pass:212  dwarn:0   dfail:0   fail:0   skip:27  
time: 0s
fi-byt-n2820 total:300  pass:208  dwarn:0   dfail:0   fail:0   skip:31  
time: 0s
fi-hsw-4770  total:300  pass:279  dwarn:0   dfail:0   fail:1   skip:20  
time: 666s
fi-hsw-4770r total:300  pass:115  dwarn:0   dfail:0   fail:0   skip:13  
time: 0s
fi-ilk-650   total:300  pass:240  dwarn:0   dfail:0   fail:2   skip:58  
time: 537s
fi-ivb-3520m total:300  pass:239  dwarn:0   dfail:0   fail:5   skip:20  
time: 0s
fi-kbl-7500u total:300  pass:275  dwarn:1   dfail:0   fail:4   skip:20  
time: 635s
fi-skl-6260u total:300  pass:283  dwarn:1   dfail:0   fail:4   skip:12  
time: 645s
fi-skl-6700k total:300  pass:271  dwarn:5   dfail:0   fail:4   skip:20  
time: 664s
fi-skl-6770hqtotal:300  pass:283  dwarn:1   dfail:0   fail:4   skip:12  
time: 697s
fi-snb-2520m total:300  pass:226  dwarn:0   dfail:0   fail:5   skip:33  
time: 0s
fi-snb-2600  total:300  pass:258  dwarn:1   dfail:0   fail:5   skip:36  
time: 599s
fi-bxt-j4205 failed to collect. IGT log at Patchwork_4235/fi-bxt-j4205/igt.log
fi-skl-6700hq failed to connect after reboot

a4d4230315e8bd8ce20fc86d860ec342c630da65 drm-tip: 2017y-03m-20d-14h-40m-55s UTC 
integration manifest
efceca6 drm/i915: Remove intel_ring.last_retired_head

== Logs ==

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

[Intel-gfx] [PATCH] drm/i915: Remove intel_ring.last_retired_head

[Chris Wilson]

Storing the position of the breadcrumb of the last retired request as
a separate last_retired_head is superfluous as we always copy that into
head prior to recalculation of the intel_ring.space.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  5 ++---
 drivers/gpu/drm/i915/i915_gem_request.c  |  2 +-
 drivers/gpu/drm/i915/intel_lrc.c |  2 --
 drivers/gpu/drm/i915/intel_ringbuffer.c  | 21 -
 drivers/gpu/drm/i915/intel_ringbuffer.h  | 10 --
 drivers/gpu/drm/i915/selftests/mock_engine.c |  1 -
 6 files changed, 7 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 47e707d83c4d..29bf11d8b620 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1938,9 +1938,8 @@ static int i915_gem_framebuffer_info(struct seq_file *m, 
void *data)
 
 static void describe_ctx_ring(struct seq_file *m, struct intel_ring *ring)
 {
-   seq_printf(m, " (ringbuffer, space: %d, head: %u, tail: %u, last head: 
%d)",
-  ring->space, ring->head, ring->tail,
-  ring->last_retired_head);
+   seq_printf(m, " (ringbuffer, space: %d, head: %u, tail: %u)",
+  ring->space, ring->head, ring->tail);
 }
 
 static int i915_context_status(struct seq_file *m, void *unused)
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c 
b/drivers/gpu/drm/i915/i915_gem_request.c
index 0e8d1010cecb..dbfa9db2419d 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -295,7 +295,7 @@ static void i915_gem_request_retire(struct 
drm_i915_gem_request *request)
 * completion order.
 */
list_del(>ring_link);
-   request->ring->last_retired_head = request->postfix;
+   request->ring->head = request->postfix;
if (!--request->i915->gt.active_requests) {
GEM_BUG_ON(!request->i915->gt.awake);
mod_delayed_work(request->i915->wq,
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index dab73e7d9a6b..29dd614aa54a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1255,7 +1255,6 @@ static void reset_common_ring(struct intel_engine_cs 
*engine,
ce->lrc_reg_state[CTX_RING_HEAD+1] = request->postfix;
 
request->ring->head = request->postfix;
-   request->ring->last_retired_head = -1;
intel_ring_update_space(request->ring);
 
/* Catch up with any missed context-switch interrupts */
@@ -2042,7 +2041,6 @@ void intel_lr_context_resume(struct drm_i915_private 
*dev_priv)
i915_gem_object_unpin_map(ce->state->obj);
 
ce->ring->head = ce->ring->tail = 0;
-   ce->ring->last_retired_head = -1;
intel_ring_update_space(ce->ring);
}
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index d9b8d17c3fc6..0ca5ea7a9407 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -49,13 +49,7 @@ static int __intel_ring_space(int head, int tail, int size)
 
 void intel_ring_update_space(struct intel_ring *ring)
 {
-   if (ring->last_retired_head != -1) {
-   ring->head = ring->last_retired_head;
-   ring->last_retired_head = -1;
-   }
-
-   ring->space = __intel_ring_space(ring->head & HEAD_ADDR,
-ring->tail, ring->size);
+   ring->space = __intel_ring_space(ring->head, ring->tail, ring->size);
 }
 
 static int
@@ -618,12 +612,8 @@ static void reset_ring_common(struct intel_engine_cs 
*engine,
}
 
/* If the rq hung, jump to its breadcrumb and skip the batch */
-   if (request->fence.error == -EIO) {
-   struct intel_ring *ring = request->ring;
-
-   ring->head = request->postfix;
-   ring->last_retired_head = -1;
-   }
+   if (request->fence.error == -EIO)
+   request->ring->head = request->postfix;
} else {
engine->legacy_active_context = NULL;
}
@@ -1392,7 +1382,6 @@ intel_engine_create_ring(struct intel_engine_cs *engine, 
int size)
if (IS_I830(engine->i915) || IS_I845G(engine->i915))
ring->effective_size -= 2 * CACHELINE_BYTES;
 
-   ring->last_retired_head = -1;
intel_ring_update_space(ring);
 
vma = intel_ring_create_vma(engine->i915, size);
@@ -1571,10 +1560,8 @@ void intel_legacy_submission_resume(struct 
drm_i915_private *dev_priv)
struct intel_engine_cs *engine;
enum intel_engine_id id;
 
-   for_each_engine(engine, dev_priv, id) {
+   for_each_engine(engine, dev_priv, 

Re: [Intel-gfx] [PATCH 2/2] drm/i915/vbt: split out defaults that are set when there is no VBT

[Manasi Navare]

On Mon, Mar 20, 2017 at 01:57:51PM -0300, Paulo Zanoni wrote:
> Em Seg, 2017-03-20 às 11:38 +0200, Jani Nikula escreveu:
> > On Fri, 17 Mar 2017, Paulo Zanoni  wrote:
> > > 
> > > Em Ter, 2017-03-14 às 11:09 +0200, Jani Nikula escreveu:
> > > > 
> > > > On Mon, 13 Mar 2017, Manasi Navare 
> > > > wrote:
> > > > > 
> > > > > 
> > > > > On Mon, Mar 13, 2017 at 11:55:31AM +0200, Jani Nikula wrote:
> > > > > > 
> > > > > > 
> > > > > > On Sat, 11 Mar 2017, Manasi Navare  > > > > > >
> > > > > > wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > On Fri, Mar 10, 2017 at 03:27:58PM +0200, Jani Nikula
> > > > > > > wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > The main thing are the DDI ports. If there's a VBT that
> > > > > > > > says
> > > > > > > > there are
> > > > > > > > no outputs, we should trust that, and not have semi-
> > > > > > > > random
> > > > > > > > defaults. Unfortunately, the defaults have resulted in
> > > > > > > > some
> > > > > > > > Chromebooks
> > > > > > > > without VBT to rely on this behaviour, so we split out
> > > > > > > > the
> > > > > > > > defaults for
> > > > > > > > the missing VBT case.
> > > > > > > > 
> > > > > > > > Cc: Manasi Navare 
> > > > > > > > Cc: Ville Syrjälä 
> > > > > > > > Signed-off-by: Jani Nikula 
> > > > > > > > ---
> > > > > > > >  drivers/gpu/drm/i915/intel_bios.c | 17 -
> > > > > > > >  1 file changed, 16 insertions(+), 1 deletion(-)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_bios.c
> > > > > > > > b/drivers/gpu/drm/i915/intel_bios.c
> > > > > > > > index 710988d72253..639d45c1dd2e 100644
> > > > > > > > --- a/drivers/gpu/drm/i915/intel_bios.c
> > > > > > > > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > > > > > > > @@ -1341,6 +1341,7 @@ parse_device_mapping(struct
> > > > > > > > drm_i915_private *dev_priv,
> > > > > > > >     return;
> > > > > > > >  }
> > > > > > > >  
> > > > > > > > +/* Common defaults which may be overridden by VBT. */
> > > > > > > >  static void
> > > > > > > >  init_vbt_defaults(struct drm_i915_private *dev_priv)
> > > > > > > >  {
> > > > > > > > @@ -1377,6 +1378,18 @@ init_vbt_defaults(struct
> > > > > > > > drm_i915_private *dev_priv)
> > > > > > > >     _priv-
> > > > > > > > >vbt.ddi_port_info[port];
> > > > > > > >  
> > > > > > > >     info->hdmi_level_shift =
> > > > > > > > HDMI_LEVEL_SHIFT_UNKNOWN;
> > > > > > > > +   }
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +/* Defaults to initialize only if there is no VBT. */
> > > > > > > > +static void
> > > > > > > > +init_vbt_missing_defaults(struct drm_i915_private
> > > > > > > > *dev_priv)
> > > > > > > > +{
> > > > > > > > +   enum port port;
> > > > > > > > +
> > > > > > > > +   for (port = PORT_A; port < I915_MAX_PORTS;
> > > > > > > > port++) {
> > > > > > > > +   struct ddi_vbt_port_info *info =
> > > > > > > > +   _priv-
> > > > > > > > >vbt.ddi_port_info[port];
> > > > > > > >  
> > > > > > > >     info->supports_dvi = (port != PORT_A &&
> > > > > > > > port
> > > > > > > > != PORT_E);
> > > > > > > >     info->supports_hdmi = info-
> > > > > > > > >supports_dvi;
> > > > > > > > @@ -1516,8 +1529,10 @@ void intel_bios_init(struct
> > > > > > > > drm_i915_private *dev_priv)
> > > > > > > >     parse_ddi_ports(dev_priv, bdb);
> > > > > > > >  
> > > > > > > >  out:
> > > > > > > > -   if (!vbt)
> > > > > > > > +   if (!vbt) {
> > > > > > > >     DRM_INFO("Failed to find VBIOS tables
> > > > > > > > (VBT)\n");
> > > > > > > > +   init_vbt_missing_defaults(dev_priv);
> > > > > > > > +   }
> > > > > > > 
> > > > > > > So in case there is no VBT, this will set supports_DP flag
> > > > > > > on
> > > > > > > Port A.
> > > > > > > What is there is no VBT and there is no eDP on Port A?
> > > > > > > In this case it will still try to link train on Port A and
> > > > > > > fail..?
> > > > > > > I am not sure if this case exists, but just a thought
> > > > > > > looking
> > > > > > > at it.
> > > > > > 
> > > > > > It's possible the case exists, but the point is that the
> > > > > > behaviour for
> > > > > > the no-VBT case remains the same before and after this patch.
> > > > > > 
> > > > > > BR,
> > > > > > Jani.
> > > > > > 
> > > > > > 
> > > > > 
> > > > > Ok agreed. In that case Reviewed-by: Manasi Navare
> > > > > 
> > > > 
> > > > Pushed to drm-intel-next-queued, thanks for the review.
> > > > 
> > > > I really hope there are no machines out there that have a
> > > > crippled
> > > > VBT
> > > > with no child device config. I guess we'll find out...
> > > 
> > > I have access to this very interesting machine with DDB version 163
> > > and
> > > a child device size config that's 1 

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Enable atomic for VLV/CHV

[Patchwork]

== Series Details ==

Series: drm/i915: Enable atomic for VLV/CHV
URL   : https://patchwork.freedesktop.org/series/20634/
State : failure

== Summary ==

Series 20634v1 drm/i915: Enable atomic for VLV/CHV
https://patchwork.freedesktop.org/api/1.0/series/20634/revisions/1/mbox/

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
fail   -> PASS   (fi-snb-2600) fdo#17
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-atomic:
skip   -> PASS   (fi-bsw-n3050)
skip   -> PASS   (fi-byt-j1900)
skip   -> FAIL   (fi-byt-n2820)
Subgroup basic-busy-flip-before-cursor-legacy:
pass   -> FAIL   (fi-byt-n2820)
Subgroup basic-flip-after-cursor-atomic:
skip   -> PASS   (fi-bsw-n3050)
skip   -> PASS   (fi-byt-j1900)
skip   -> FAIL   (fi-byt-n2820)
Subgroup basic-flip-after-cursor-legacy:
pass   -> FAIL   (fi-byt-n2820)
Subgroup basic-flip-after-cursor-varying-size:
pass   -> FAIL   (fi-byt-n2820) fdo#100094
Subgroup basic-flip-before-cursor-atomic:
skip   -> PASS   (fi-bsw-n3050)
skip   -> PASS   (fi-byt-j1900)
skip   -> FAIL   (fi-byt-n2820)
Subgroup basic-flip-before-cursor-legacy:
pass   -> FAIL   (fi-byt-n2820)
Subgroup basic-flip-before-cursor-varying-size:
pass   -> FAIL   (fi-byt-n2820) fdo#100094
Test kms_flip:
Subgroup basic-flip-vs-dpms:
pass   -> INCOMPLETE (fi-byt-n2820)
Subgroup basic-plain-flip:
pass   -> INCOMPLETE (fi-byt-j1900)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-c:
pass   -> INCOMPLETE (fi-ivb-3520m)
Subgroup nonblocking-crc-pipe-b:
pass   -> INCOMPLETE (fi-bxt-j4205)
Test kms_setmode:
Subgroup basic-clone-single-crtc:
pass   -> INCOMPLETE (fi-bsw-n3050)
Test pm_backlight:
Subgroup basic-brightness:
pass   -> INCOMPLETE (fi-snb-2520m)

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

fi-bdw-5557u total:300  pass:284  dwarn:0   dfail:0   fail:0   skip:16  
time: 619s
fi-bsw-n3050 total:300  pass:223  dwarn:0   dfail:0   fail:1   skip:41  
time: 0s
fi-bxt-j4205 total:300  pass:231  dwarn:0   dfail:0   fail:3   skip:19  
time: 0s
fi-byt-j1900 total:237  pass:214  dwarn:0   dfail:0   fail:0   skip:22  
time: 0s
fi-byt-n2820 total:300  pass:203  dwarn:0   dfail:0   fail:8   skip:26  
time: 0s
fi-hsw-4770  total:300  pass:279  dwarn:0   dfail:0   fail:1   skip:20  
time: 660s
fi-ilk-650   total:300  pass:240  dwarn:0   dfail:0   fail:2   skip:58  
time: 533s
fi-ivb-3520m total:263  pass:238  dwarn:0   dfail:0   fail:5   skip:19  
time: 0s
fi-kbl-7500u total:300  pass:275  dwarn:1   dfail:0   fail:4   skip:20  
time: 634s
fi-skl-6260u total:300  pass:283  dwarn:1   dfail:0   fail:4   skip:12  
time: 646s
fi-skl-6700k total:300  pass:271  dwarn:5   dfail:0   fail:4   skip:20  
time: 667s
fi-skl-6770hqtotal:300  pass:283  dwarn:1   dfail:0   fail:4   skip:12  
time: 696s
fi-snb-2520m total:300  pass:227  dwarn:0   dfail:0   fail:6   skip:33  
time: 0s
fi-snb-2600  total:300  pass:258  dwarn:0   dfail:0   fail:6   skip:36  
time: 586s
fi-hsw-4770r failed to connect after reboot
fi-skl-6700hq failed to connect after reboot

a4d4230315e8bd8ce20fc86d860ec342c630da65 drm-tip: 2017y-03m-20d-14h-40m-55s UTC 
integration manifest
a58a5bd drm/i915: Enable atomic on VLV/CHV
0521fa8 drm/i915: Use intel_wm_plane_visible() on VLV/CHV as well
0c5b12f drm/i915: Check for id==PLANE_CURSOR instead of 
type==DRM_PLANE_TYPE_CURSOR
f821f04 drm/i915: Extract intel_wm_plane_visible()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4234/
___
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/glk: CDCLK calculation changes for glk (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/glk: CDCLK calculation changes for glk (rev3)
URL   : https://patchwork.freedesktop.org/series/19226/
State : failure

== Summary ==

Series 19226v3 drm/i915/glk: CDCLK calculation changes for glk
https://patchwork.freedesktop.org/api/1.0/series/19226/revisions/3/mbox/

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
fail   -> PASS   (fi-snb-2600) fdo#17
Test gem_exec_suspend:
Subgroup basic-s4:
notrun -> INCOMPLETE (fi-snb-2600)
Test gem_mmap_gtt:
Subgroup basic-copy:
pass   -> INCOMPLETE (fi-hsw-4770r)
Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass   -> INCOMPLETE (fi-byt-n2820)
Subgroup basic-flip-vs-wf_vblank:
pass   -> INCOMPLETE (fi-byt-j1900)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-c:
pass   -> INCOMPLETE (fi-ivb-3520m)
Test kms_setmode:
Subgroup basic:
notrun -> INCOMPLETE (fi-snb-2520m)
Test pm_rpm:
Subgroup basic-pci-d3-state:
pass   -> INCOMPLETE (fi-bsw-n3050)

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

fi-bdw-5557u total:300  pass:284  dwarn:0   dfail:0   fail:0   skip:16  
time: 626s
fi-bsw-n3050 total:300  pass:221  dwarn:0   dfail:0   fail:1   skip:46  
time: 0s
fi-byt-j1900 total:240  pass:212  dwarn:0   dfail:0   fail:0   skip:27  
time: 0s
fi-byt-n2820 total:239  pass:207  dwarn:0   dfail:0   fail:0   skip:31  
time: 0s
fi-hsw-4770  total:300  pass:279  dwarn:0   dfail:0   fail:1   skip:20  
time: 666s
fi-hsw-4770r total:300  pass:138  dwarn:0   dfail:0   fail:0   skip:13  
time: 0s
fi-ilk-650   total:300  pass:240  dwarn:0   dfail:0   fail:2   skip:58  
time: 540s
fi-ivb-3520m total:263  pass:238  dwarn:0   dfail:0   fail:5   skip:19  
time: 0s
fi-kbl-7500u total:300  pass:276  dwarn:1   dfail:0   fail:3   skip:20  
time: 638s
fi-skl-6260u total:300  pass:283  dwarn:1   dfail:0   fail:4   skip:12  
time: 662s
fi-skl-6700k total:300  pass:271  dwarn:5   dfail:0   fail:4   skip:20  
time: 667s
fi-skl-6770hqtotal:300  pass:283  dwarn:1   dfail:0   fail:4   skip:12  
time: 695s
fi-snb-2520m total:300  pass:226  dwarn:0   dfail:0   fail:5   skip:33  
time: 0s
fi-snb-2600  total:113  pass:88   dwarn:0   dfail:0   fail:5   skip:19  
time: 0s
fi-bxt-j4205 failed to connect after reboot
fi-bxt-t5700 failed to connect after reboot
fi-ivb-3770 failed to collect. IGT log at Patchwork_4233/fi-ivb-3770/igt.log
fi-skl-6700hq failed to connect after reboot

a4d4230315e8bd8ce20fc86d860ec342c630da65 drm-tip: 2017y-03m-20d-14h-40m-55s UTC 
integration manifest
aff56a8 drm/i915/glk: CDCLK calculation changes for glk

== Logs ==

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

[Intel-gfx] [PATCH] drm/i915/glk: CDCLK calculation changes for glk

[Madhav Chauhan]

As per BSPEC, valid cdclk values for glk are 79.2, 158.4, 316.8 Mhz.
Practically we can achive only 99% of these cdclk values(HW team
checking on this). So cdclk should be calculated for the given pixclk as
per that otherwise it may lead to screen corruption for some scenarios.

v2: Rebased to new CDLCK code framework
v3: Addressed review comments from Ander/Jani
- Add comment in code about 99% usage of CDCLK
- Calculate max dot clock as well with 99% limit

Signed-off-by: Madhav Chauhan 
---
 drivers/gpu/drm/i915/intel_cdclk.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c 
b/drivers/gpu/drm/i915/intel_cdclk.c
index c2cc33f..a661c7e 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1071,9 +1071,13 @@ static int bxt_calc_cdclk(int max_pixclk)
 
 static int glk_calc_cdclk(int max_pixclk)
 {
-   if (max_pixclk > 2 * 158400)
+   /*
+* For GLK platform 316.8, 158.4, 79.2 MHz are the CDCLK values
+* as per BSPEC. But practically we can only achieve 99% of these.
+*/
+   if (max_pixclk > DIV_ROUND_UP(2 * 158400 * 99, 100))
return 316800;
-   else if (max_pixclk > 2 * 79200)
+   else if (max_pixclk > DIV_ROUND_UP(2 * 79200 * 99, 100))
return 158400;
else
return 79200;
@@ -1613,7 +1617,6 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state 
*state)
cdclk = bxt_calc_cdclk(max_pixclk);
vco = bxt_de_pll_vco(dev_priv, cdclk);
}
-
if (cdclk > dev_priv->max_cdclk_freq) {
DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
  cdclk, dev_priv->max_cdclk_freq);
@@ -1647,7 +1650,7 @@ static int intel_compute_max_dotclk(struct 
drm_i915_private *dev_priv)
int max_cdclk_freq = dev_priv->max_cdclk_freq;
 
if (IS_GEMINILAKE(dev_priv))
-   return 2 * max_cdclk_freq;
+   return 2 * max_cdclk_freq * 99/100;
else if (INTEL_INFO(dev_priv)->gen >= 9 ||
 IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
return max_cdclk_freq;
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH] fs/pstore: Perform erase from a worker

[Kees Cook]

On Fri, Mar 17, 2017 at 2:52 AM, Chris Wilson  wrote:
> In order to prevent a cyclic recursion between psi->read_mutex and the
> inode_lock, we need to move the pse->erase to a worker.
>
> [  605.374955] ==
> [  605.381281] [ INFO: possible circular locking dependency detected ]
> [  605.387679] 4.11.0-rc2-CI-CI_DRM_2352+ #1 Not tainted
> [  605.392826] ---
> [  605.399196] rm/7298 is trying to acquire lock:
> [  605.403720]  (>read_mutex){+.+.+.}, at: [] 
> pstore_unlink+0x3f/0xa0
> [  605.412300]
> [  605.412300] but task is already holding lock:
> [  605.418237]  (>s_type->i_mutex_key#14){++}, at: 
> [] vfs_unlink+0x4c/0x19
> 0
> [  605.427397]
> [  605.427397] which lock already depends on the new lock.
> [  605.427397]
> [  605.435770]
> [  605.435770] the existing dependency chain (in reverse order) is:
> [  605.443396]
> [  605.443396] -> #1 (>s_type->i_mutex_key#14){++}:
> [  605.450347]lock_acquire+0xc9/0x220
> [  605.454551]down_write+0x3f/0x70
> [  605.458484]pstore_mkfile+0x1f4/0x460
> [  605.462835]pstore_get_records+0x17a/0x320
> [  605.467664]pstore_fill_super+0xa4/0xc0
> [  605.472205]mount_single+0x89/0xb0
> [  605.476314]pstore_mount+0x13/0x20
> [  605.480411]mount_fs+0xf/0x90
> [  605.484122]vfs_kern_mount+0x66/0x170
> [  605.488464]do_mount+0x190/0xd50
> [  605.492397]SyS_mount+0x90/0xd0
> [  605.496212]entry_SYSCALL_64_fastpath+0x1c/0xb1
> [  605.501496]
> [  605.501496] -> #0 (>read_mutex){+.+.+.}:
> [  605.507747]__lock_acquire+0x1ac0/0x1bb0
> [  605.512401]lock_acquire+0xc9/0x220
> [  605.516594]__mutex_lock+0x6e/0x990
> [  605.520755]mutex_lock_nested+0x16/0x20
> [  605.525279]pstore_unlink+0x3f/0xa0
> [  605.529465]vfs_unlink+0xb5/0x190
> [  605.533477]do_unlinkat+0x24c/0x2a0
> [  605.537672]SyS_unlinkat+0x16/0x30
> [  605.541781]entry_SYSCALL_64_fastpath+0x1c/0xb1

If I'm reading this right it's a race between mount and unlink...
that's quite a corner case. :)

> [  605.547067]
> [  605.547067] other info that might help us debug this:
> [  605.547067]
> [  605.555221]  Possible unsafe locking scenario:
> [  605.555221]
> [  605.561280]CPU0CPU1
> [  605.565883]
> [  605.570502]   lock(>s_type->i_mutex_key#14);
> [  605.575217]lock(>read_mutex);
> [  605.581803]
> lock(>s_type->i_mutex_key#14);
> [  605.589159]   lock(>read_mutex);

I haven't had time to dig much yet, but I wonder if the locking order
on unlink could just be reversed, and the deadlock would go away?

> [  605.593156]
> [  605.593156]  *** DEADLOCK ***
> [  605.593156]
> [  605.599214] 3 locks held by rm/7298:
> [  605.602896]  #0:  (sb_writers#11){.+.+..}, at: [] 
> mnt_want_write+0x1f/0x50
> [  605.611490]  #1:  (>s_type->i_mutex_key#14/1){+.+...}, at: 
> [] do_unlinkat+0
> x11c/0x2a0
> [  605.621417]  #2:  (>s_type->i_mutex_key#14){++}, at: 
> [] vfs_unlink+0x4c
> /0x190
> [  605.630995]
> [  605.630995] stack backtrace:
> [  605.635450] CPU: 7 PID: 7298 Comm: rm Not tainted 
> 4.11.0-rc2-CI-CI_DRM_2352+ #1
> [  605.642999] Hardware name: Gigabyte Technology Co., Ltd. 
> Z170X-UD5/Z170X-UD5-CF, BIOS F21 01/06/2
> 017
> [  605.652305] Call Trace:
> [  605.654814]  dump_stack+0x67/0x92
> [  605.658184]  print_circular_bug+0x1e0/0x2e0
> [  605.662465]  __lock_acquire+0x1ac0/0x1bb0
> [  605.34]  ? retint_kernel+0x2d/0x2d
> [  605.670456]  lock_acquire+0xc9/0x220
> [  605.674112]  ? pstore_unlink+0x3f/0xa0
> [  605.677970]  ? pstore_unlink+0x3f/0xa0
> [  605.681818]  __mutex_lock+0x6e/0x990
> [  605.685456]  ? pstore_unlink+0x3f/0xa0
> [  605.689791]  ? pstore_unlink+0x3f/0xa0
> [  605.694124]  ? vfs_unlink+0x4c/0x190
> [  605.698310]  mutex_lock_nested+0x16/0x20
> [  605.702859]  pstore_unlink+0x3f/0xa0
> [  605.707021]  vfs_unlink+0xb5/0x190
> [  605.711024]  do_unlinkat+0x24c/0x2a0
> [  605.715194]  SyS_unlinkat+0x16/0x30
> [  605.719275]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> [  605.724543] RIP: 0033:0x7f8b08073ed7
> [  605.728676] RSP: 002b:7ffe70eff628 EFLAGS: 0206 ORIG_RAX: 
> 0107
> [  605.736929] RAX: ffda RBX: 8147ea93 RCX: 
> 7f8b08073ed7
> [  605.744711] RDX:  RSI: 0145 RDI: 
> ff9c
> [  605.752512] RBP: c9000338ff88 R08: 0003 R09: 
> 
> [  605.760276] R10: 015e R11: 0206 R12: 
> 
> [  605.768040] R13: 7ffe70eff750 R14: 0144ff70 R15: 
> 01451230
> [  605.775800]  ? __this_cpu_preempt_check+0x13/0x20
>
> Reported-by: Tomi Sarvela 
> Fixes: e9e360b08a44 ("pstore: Protect 

Re: [Intel-gfx] [PATCH 09/14] drm/i915: Introduce i9xx_check_plane_surface()

[Chris Wilson]

On Mon, Mar 20, 2017 at 07:07:55PM +0200, Ville Syrjälä wrote:
> On Fri, Mar 17, 2017 at 10:04:32PM +, Chris Wilson wrote:
> > On Fri, Mar 17, 2017 at 11:18:03PM +0200, ville.syrj...@linux.intel.com 
> > wrote:
> > > From: Ville Syrjälä 
> > > 
> > > Extract the primary plane surfae offset/x/y calculations for
> > > pre-SKL platforms into a common function, and call it during the
> > > atomic check phase to reduce the amount of stuff we have to do
> > > during the commit phase. SKL is already doing this.
> > > 
> > > Signed-off-by: Ville Syrjälä 
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 82 
> > > ++--
> > >  1 file changed, 50 insertions(+), 32 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 2e0106a11f8f..024614cb47b6 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3026,6 +3026,43 @@ static u32 i9xx_plane_ctl(const struct 
> > > intel_crtc_state *crtc_state,
> > >   return dspcntr;
> > >  }
> > >  
> > > +static int i9xx_check_plane_surface(struct intel_plane_state 
> > > *plane_state)
> > > +{
> > > + struct drm_i915_private *dev_priv =
> > > + to_i915(plane_state->base.plane->dev);
> > > + int src_x = plane_state->base.src.x1 >> 16;
> > > + int src_y = plane_state->base.src.y1 >> 16;
> > > + u32 offset;
> > > +
> > > + intel_add_fb_offsets(_x, _y, plane_state, 0);
> > > +
> > > + if (INTEL_GEN(dev_priv) >= 4)
> > > + offset = intel_compute_tile_offset(_x, _y,
> > > +plane_state, 0);
> > > + else
> > > + offset = 0;
> > > +
> > > + /* HSW+ does this automagically in hardware */
> > > + if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv)) {
> > 
> > if (INTEL_GEN() <= 7 && !IS_HASWELL()) {
> > 
> > would match the comment better.
> 
> That would leave out CHV.
> 
> I think 'HAS_GMCH || IS_GEN5 || IS_GEN6 || IS_IVB' might be
> a semi-decent way to put this. But it's still not quite as
> succinct as '!HSW && !BDW'.
> 
> What about if I just change the comment to "HSW/BDW do this ..."?

Prevents me showing my ignorance in that chv isn't include in that set.
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 09/14] drm/i915: Introduce i9xx_check_plane_surface()

[Ville Syrjälä]

On Fri, Mar 17, 2017 at 10:04:32PM +, Chris Wilson wrote:
> On Fri, Mar 17, 2017 at 11:18:03PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > Extract the primary plane surfae offset/x/y calculations for
> > pre-SKL platforms into a common function, and call it during the
> > atomic check phase to reduce the amount of stuff we have to do
> > during the commit phase. SKL is already doing this.
> > 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 82 
> > ++--
> >  1 file changed, 50 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 2e0106a11f8f..024614cb47b6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3026,6 +3026,43 @@ static u32 i9xx_plane_ctl(const struct 
> > intel_crtc_state *crtc_state,
> > return dspcntr;
> >  }
> >  
> > +static int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
> > +{
> > +   struct drm_i915_private *dev_priv =
> > +   to_i915(plane_state->base.plane->dev);
> > +   int src_x = plane_state->base.src.x1 >> 16;
> > +   int src_y = plane_state->base.src.y1 >> 16;
> > +   u32 offset;
> > +
> > +   intel_add_fb_offsets(_x, _y, plane_state, 0);
> > +
> > +   if (INTEL_GEN(dev_priv) >= 4)
> > +   offset = intel_compute_tile_offset(_x, _y,
> > +  plane_state, 0);
> > +   else
> > +   offset = 0;
> > +
> > +   /* HSW+ does this automagically in hardware */
> > +   if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv)) {
> 
> if (INTEL_GEN() <= 7 && !IS_HASWELL()) {
> 
> would match the comment better.

That would leave out CHV.

I think 'HAS_GMCH || IS_GEN5 || IS_GEN6 || IS_IVB' might be
a semi-decent way to put this. But it's still not quite as
succinct as '!HSW && !BDW'.

What about if I just change the comment to "HSW/BDW do this ..."?

> 
> > +   unsigned int rotation = plane_state->base.rotation;
> > +   int src_w = drm_rect_width(_state->base.src) >> 16;
> > +   int src_h = drm_rect_height(_state->base.src) >> 16;
> > +
> > +   if (rotation & DRM_ROTATE_180) {
> > +   src_x += src_w - 1;
> > +   src_y += src_h - 1;
> > +   } else if (rotation & DRM_REFLECT_X) {
> > +   src_x += src_w - 1;
> > +   }
> > +   }
> > +
> > +   plane_state->main.offset = offset;
> > +   plane_state->main.x = src_x;
> > +   plane_state->main.y = src_y;
> 
> plane_state->actual.offset, ->actual.x, ->actual.y ?
> plane_state->commit.offset, ->commit.x, ->commit.y ?
> 
> Movement looks fine,
> Reviewed-by: Chris Wilson 
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/vbt: split out defaults that are set when there is no VBT

[Paulo Zanoni]

Em Seg, 2017-03-20 às 11:38 +0200, Jani Nikula escreveu:
> On Fri, 17 Mar 2017, Paulo Zanoni  wrote:
> > 
> > Em Ter, 2017-03-14 às 11:09 +0200, Jani Nikula escreveu:
> > > 
> > > On Mon, 13 Mar 2017, Manasi Navare 
> > > wrote:
> > > > 
> > > > 
> > > > On Mon, Mar 13, 2017 at 11:55:31AM +0200, Jani Nikula wrote:
> > > > > 
> > > > > 
> > > > > On Sat, 11 Mar 2017, Manasi Navare  > > > > >
> > > > > wrote:
> > > > > > 
> > > > > > 
> > > > > > On Fri, Mar 10, 2017 at 03:27:58PM +0200, Jani Nikula
> > > > > > wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > The main thing are the DDI ports. If there's a VBT that
> > > > > > > says
> > > > > > > there are
> > > > > > > no outputs, we should trust that, and not have semi-
> > > > > > > random
> > > > > > > defaults. Unfortunately, the defaults have resulted in
> > > > > > > some
> > > > > > > Chromebooks
> > > > > > > without VBT to rely on this behaviour, so we split out
> > > > > > > the
> > > > > > > defaults for
> > > > > > > the missing VBT case.
> > > > > > > 
> > > > > > > Cc: Manasi Navare 
> > > > > > > Cc: Ville Syrjälä 
> > > > > > > Signed-off-by: Jani Nikula 
> > > > > > > ---
> > > > > > >  drivers/gpu/drm/i915/intel_bios.c | 17 -
> > > > > > >  1 file changed, 16 insertions(+), 1 deletion(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/gpu/drm/i915/intel_bios.c
> > > > > > > b/drivers/gpu/drm/i915/intel_bios.c
> > > > > > > index 710988d72253..639d45c1dd2e 100644
> > > > > > > --- a/drivers/gpu/drm/i915/intel_bios.c
> > > > > > > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > > > > > > @@ -1341,6 +1341,7 @@ parse_device_mapping(struct
> > > > > > > drm_i915_private *dev_priv,
> > > > > > >   return;
> > > > > > >  }
> > > > > > >  
> > > > > > > +/* Common defaults which may be overridden by VBT. */
> > > > > > >  static void
> > > > > > >  init_vbt_defaults(struct drm_i915_private *dev_priv)
> > > > > > >  {
> > > > > > > @@ -1377,6 +1378,18 @@ init_vbt_defaults(struct
> > > > > > > drm_i915_private *dev_priv)
> > > > > > >   _priv-
> > > > > > > >vbt.ddi_port_info[port];
> > > > > > >  
> > > > > > >   info->hdmi_level_shift =
> > > > > > > HDMI_LEVEL_SHIFT_UNKNOWN;
> > > > > > > + }
> > > > > > > +}
> > > > > > > +
> > > > > > > +/* Defaults to initialize only if there is no VBT. */
> > > > > > > +static void
> > > > > > > +init_vbt_missing_defaults(struct drm_i915_private
> > > > > > > *dev_priv)
> > > > > > > +{
> > > > > > > + enum port port;
> > > > > > > +
> > > > > > > + for (port = PORT_A; port < I915_MAX_PORTS;
> > > > > > > port++) {
> > > > > > > + struct ddi_vbt_port_info *info =
> > > > > > > + _priv-
> > > > > > > >vbt.ddi_port_info[port];
> > > > > > >  
> > > > > > >   info->supports_dvi = (port != PORT_A &&
> > > > > > > port
> > > > > > > != PORT_E);
> > > > > > >   info->supports_hdmi = info-
> > > > > > > >supports_dvi;
> > > > > > > @@ -1516,8 +1529,10 @@ void intel_bios_init(struct
> > > > > > > drm_i915_private *dev_priv)
> > > > > > >   parse_ddi_ports(dev_priv, bdb);
> > > > > > >  
> > > > > > >  out:
> > > > > > > - if (!vbt)
> > > > > > > + if (!vbt) {
> > > > > > >   DRM_INFO("Failed to find VBIOS tables
> > > > > > > (VBT)\n");
> > > > > > > + init_vbt_missing_defaults(dev_priv);
> > > > > > > + }
> > > > > > 
> > > > > > So in case there is no VBT, this will set supports_DP flag
> > > > > > on
> > > > > > Port A.
> > > > > > What is there is no VBT and there is no eDP on Port A?
> > > > > > In this case it will still try to link train on Port A and
> > > > > > fail..?
> > > > > > I am not sure if this case exists, but just a thought
> > > > > > looking
> > > > > > at it.
> > > > > 
> > > > > It's possible the case exists, but the point is that the
> > > > > behaviour for
> > > > > the no-VBT case remains the same before and after this patch.
> > > > > 
> > > > > BR,
> > > > > Jani.
> > > > > 
> > > > > 
> > > > 
> > > > Ok agreed. In that case Reviewed-by: Manasi Navare
> > > > 
> > > 
> > > Pushed to drm-intel-next-queued, thanks for the review.
> > > 
> > > I really hope there are no machines out there that have a
> > > crippled
> > > VBT
> > > with no child device config. I guess we'll find out...
> > 
> > I have access to this very interesting machine with DDB version 163
> > and
> > a child device size config that's 1 instead of the expected 33.
> > 
> > So what happens here is that since the VBT is supposed to be valid
> > we
> > don't end up calling init_vbt_missing_defauilts(). We return early
> > from
> > parse_device_mapping(), which means we don't set vbt.child_dev_num,
> > which means that parse_ddi_port() returns early. So info-
> > >supports_*
> > stays false, and intel_ddi_init() fails.
> > 
> > Given your 

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Moar plane update optimizations (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Moar plane update optimizations (rev3)
URL   : https://patchwork.freedesktop.org/series/21475/
State : failure

== Summary ==

  LD [M]  drivers/net/ethernet/broadcom/genet/genet.o
  LD  drivers/gpu/drm/drm.o
  LD  drivers/thermal/thermal_sys.o
  LD  drivers/misc/built-in.o
  LD  drivers/thermal/built-in.o
  LD  drivers/acpi/acpica/built-in.o
  LD  lib/raid6/raid6_pq.o
  LD [M]  drivers/mmc/core/mmc_block.o
  LD  drivers/pci/pcie/pcieportdrv.o
  LD  drivers/mmc/built-in.o
  LD  net/xfrm/built-in.o
  LD  lib/raid6/built-in.o
  LD  drivers/acpi/built-in.o
  LD  kernel/sched/built-in.o
  LD  drivers/iommu/built-in.o
  LD  kernel/built-in.o
  LD [M]  drivers/usb/serial/usbserial.o
  LD [M]  drivers/net/ethernet/intel/igbvf/igbvf.o
  LD  drivers/base/power/built-in.o
  LD  drivers/spi/built-in.o
  LD  drivers/video/fbdev/core/fb.o
  LD [M]  sound/pci/hda/snd-hda-codec-generic.o
  LD  drivers/tty/serial/8250/8250.o
  LD  drivers/video/fbdev/core/built-in.o
  LD  sound/pci/built-in.o
  LD  drivers/pci/pcie/aer/aerdriver.o
  LD  drivers/usb/gadget/libcomposite.o
  LD  drivers/pci/pcie/aer/built-in.o
drivers/gpu/drm/i915/intel_display.c: In function 
‘ironlake_update_primary_plane’:
drivers/gpu/drm/i915/intel_display.c:3187:1: error: expected expression before 
‘<<’ token
 <<< acaa0c0b22fe8c7bf6e7adb2337ba23bce6f9147
 ^
drivers/gpu/drm/i915/intel_display.c:3190:1: error: expected expression before 
‘==’ token
 ===
 ^
  LD  net/ipv6/ipv6.o
  LD  drivers/pci/pcie/built-in.o
  LD  sound/built-in.o
  LD  drivers/base/regmap/built-in.o
  LD  net/ipv6/built-in.o
  LD  drivers/scsi/scsi_mod.o
  LD  drivers/base/built-in.o
drivers/gpu/drm/i915/intel_display.c: At top level:
drivers/gpu/drm/i915/intel_display.c:3120:12: error: ‘ironlake_plane_ctl’ 
defined but not used [-Werror=unused-function]
 static u32 ironlake_plane_ctl(const struct intel_crtc_state *crtc_state,
^
  LD  drivers/usb/gadget/udc/udc-core.o
  LD [M]  drivers/net/ethernet/intel/e1000/e1000.o
cc1: all warnings being treated as errors
scripts/Makefile.build:294: recipe for target 
'drivers/gpu/drm/i915/intel_display.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_display.o] Error 1
make[4]: *** Waiting for unfinished jobs
  LD  drivers/usb/gadget/udc/built-in.o
  LD  drivers/usb/gadget/built-in.o
  LD  drivers/video/fbdev/built-in.o
  LD  drivers/pci/built-in.o
  AR  lib/lib.a
  EXPORTS lib/lib-ksyms.o
  LD  net/ipv4/built-in.o
  LD  lib/built-in.o
  LD  drivers/tty/serial/8250/8250_base.o
  LD  drivers/tty/serial/8250/built-in.o
  LD  drivers/video/console/built-in.o
  LD  drivers/video/built-in.o
  LD  drivers/tty/serial/built-in.o
  LD [M]  drivers/net/ethernet/intel/igb/igb.o
  LD  drivers/scsi/sd_mod.o
  LD  fs/btrfs/btrfs.o
  LD  drivers/scsi/built-in.o
  LD  fs/btrfs/built-in.o
  LD  drivers/usb/core/usbcore.o
  LD  drivers/usb/core/built-in.o
  CC  arch/x86/kernel/cpu/capflags.o
  LD  arch/x86/kernel/cpu/built-in.o
  LD  arch/x86/kernel/built-in.o
  LD  drivers/md/md-mod.o
  LD  arch/x86/built-in.o
  LD  drivers/md/built-in.o
  LD  drivers/usb/host/xhci-hcd.o
  LD  fs/ext4/ext4.o
  LD  drivers/tty/vt/built-in.o
  LD  drivers/tty/built-in.o
  LD  fs/ext4/built-in.o
  LD  fs/built-in.o
  LD  drivers/usb/host/built-in.o
  LD  drivers/usb/built-in.o
  LD  net/core/built-in.o
  LD [M]  drivers/net/ethernet/intel/e1000e/e1000e.o
  LD  net/built-in.o
scripts/Makefile.build:553: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:553: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:553: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
make[1]: *** Waiting for unfinished jobs
  LD  drivers/net/ethernet/built-in.o
  LD  drivers/net/built-in.o
Makefile:1002: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

[Intel-gfx] [PATCH 03.5/14] drm/i915: Nuke ironlake_plane_ctl()

[ville . syrjala]

From: Ville Syrjälä 

Share the code to compute the primary plane control register value
between the i9xx and ilk codepaths as the differences are minimal.
Actually there are no differences between g4x and ilk, so the
current split doesn't really make any sense.

Cc: Chris Wilson 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_display.c | 58 
 1 file changed, 6 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index bd6c1c5469e4..99b72c4219a8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2974,9 +2974,13 @@ static u32 i9xx_plane_ctl(const struct intel_crtc_state 
*crtc_state,
 
dspcntr = DISPLAY_PLANE_ENABLE | DISPPLANE_GAMMA_ENABLE;
 
-   if (IS_G4X(dev_priv))
+   if (IS_G4X(dev_priv) || IS_GEN5(dev_priv) ||
+   IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
 
+   if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+   dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
+
if (INTEL_GEN(dev_priv) < 4) {
if (crtc->pipe == PIPE_B)
dspcntr |= DISPPLANE_SEL_PIPE_B;
@@ -3119,56 +3123,6 @@ static void i9xx_disable_primary_plane(struct drm_plane 
*primary,
spin_unlock_irqrestore(_priv->uncore.lock, irqflags);
 }
 
-static u32 ironlake_plane_ctl(const struct intel_crtc_state *crtc_state,
- const struct intel_plane_state *plane_state)
-{
-   struct drm_i915_private *dev_priv =
-   to_i915(plane_state->base.plane->dev);
-   const struct drm_framebuffer *fb = plane_state->base.fb;
-   unsigned int rotation = plane_state->base.rotation;
-   u32 dspcntr;
-
-   dspcntr = DISPLAY_PLANE_ENABLE | DISPPLANE_GAMMA_ENABLE;
-
-   if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv))
-   dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
-
-   if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-   dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
-
-   switch (fb->format->format) {
-   case DRM_FORMAT_C8:
-   dspcntr |= DISPPLANE_8BPP;
-   break;
-   case DRM_FORMAT_RGB565:
-   dspcntr |= DISPPLANE_BGRX565;
-   break;
-   case DRM_FORMAT_XRGB:
-   dspcntr |= DISPPLANE_BGRX888;
-   break;
-   case DRM_FORMAT_XBGR:
-   dspcntr |= DISPPLANE_RGBX888;
-   break;
-   case DRM_FORMAT_XRGB2101010:
-   dspcntr |= DISPPLANE_BGRX101010;
-   break;
-   case DRM_FORMAT_XBGR2101010:
-   dspcntr |= DISPPLANE_RGBX101010;
-   break;
-   default:
-   MISSING_CASE(fb->format->format);
-   return 0;
-   }
-
-   if (fb->modifier == I915_FORMAT_MOD_X_TILED)
-   dspcntr |= DISPPLANE_TILED;
-
-   if (rotation & DRM_ROTATE_180)
-   dspcntr |= DISPPLANE_ROTATE_180;
-
-   return dspcntr;
-}
-
 static void ironlake_update_primary_plane(struct drm_plane *primary,
  const struct intel_crtc_state 
*crtc_state,
  const struct intel_plane_state 
*plane_state)
@@ -3186,7 +3140,7 @@ static void ironlake_update_primary_plane(struct 
drm_plane *primary,
int y = plane_state->base.src.y1 >> 16;
unsigned long irqflags;
 
-   dspcntr = ironlake_plane_ctl(crtc_state, plane_state);
+   dspcntr = i9xx_plane_ctl(crtc_state, plane_state);
 
intel_add_fb_offsets(, , plane_state, 0);
 
-- 
2.10.2

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

[Intel-gfx] [PATCH v2 03/14] drm/i915: Extract i9xx_plane_ctl() and ironlake_plane_ctl()

[ville . syrjala]

From: Ville Syrjälä 

Pull the code to calculate the pre-SKL primary plane control register
value into separate functions. Allows us to pre-compute it in the
future.

v2: Split the pre-ilk vs. ilk+ unification to a separate patch (Chris)

Cc: Chris Wilson 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_display.c | 104 ++-
 1 file changed, 66 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 8a9f1bd21e0e..bd6c1c5469e4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2962,28 +2962,23 @@ int skl_check_plane_surface(struct intel_plane_state 
*plane_state)
return 0;
 }
 
-static void i9xx_update_primary_plane(struct drm_plane *primary,
- const struct intel_crtc_state *crtc_state,
- const struct intel_plane_state 
*plane_state)
+static u32 i9xx_plane_ctl(const struct intel_crtc_state *crtc_state,
+ const struct intel_plane_state *plane_state)
 {
-   struct drm_i915_private *dev_priv = to_i915(primary->dev);
-   struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
-   struct drm_framebuffer *fb = plane_state->base.fb;
-   int plane = intel_crtc->plane;
-   u32 linear_offset;
-   u32 dspcntr;
-   i915_reg_t reg = DSPCNTR(plane);
+   struct drm_i915_private *dev_priv =
+   to_i915(plane_state->base.plane->dev);
+   struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+   const struct drm_framebuffer *fb = plane_state->base.fb;
unsigned int rotation = plane_state->base.rotation;
-   int x = plane_state->base.src.x1 >> 16;
-   int y = plane_state->base.src.y1 >> 16;
-   unsigned long irqflags;
+   u32 dspcntr;
 
-   dspcntr = DISPPLANE_GAMMA_ENABLE;
+   dspcntr = DISPLAY_PLANE_ENABLE | DISPPLANE_GAMMA_ENABLE;
 
-   dspcntr |= DISPLAY_PLANE_ENABLE;
+   if (IS_G4X(dev_priv))
+   dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
 
if (INTEL_GEN(dev_priv) < 4) {
-   if (intel_crtc->pipe == PIPE_B)
+   if (crtc->pipe == PIPE_B)
dspcntr |= DISPPLANE_SEL_PIPE_B;
}
 
@@ -3010,7 +3005,8 @@ static void i9xx_update_primary_plane(struct drm_plane 
*primary,
dspcntr |= DISPPLANE_RGBX101010;
break;
default:
-   BUG();
+   MISSING_CASE(fb->format->format);
+   return 0;
}
 
if (INTEL_GEN(dev_priv) >= 4 &&
@@ -3023,8 +3019,26 @@ static void i9xx_update_primary_plane(struct drm_plane 
*primary,
if (rotation & DRM_REFLECT_X)
dspcntr |= DISPPLANE_MIRROR;
 
-   if (IS_G4X(dev_priv))
-   dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
+   return dspcntr;
+}
+
+static void i9xx_update_primary_plane(struct drm_plane *primary,
+ const struct intel_crtc_state *crtc_state,
+ const struct intel_plane_state 
*plane_state)
+{
+   struct drm_i915_private *dev_priv = to_i915(primary->dev);
+   struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+   struct drm_framebuffer *fb = plane_state->base.fb;
+   int plane = intel_crtc->plane;
+   u32 linear_offset;
+   u32 dspcntr;
+   i915_reg_t reg = DSPCNTR(plane);
+   unsigned int rotation = plane_state->base.rotation;
+   int x = plane_state->base.src.x1 >> 16;
+   int y = plane_state->base.src.y1 >> 16;
+   unsigned long irqflags;
+
+   dspcntr = i9xx_plane_ctl(crtc_state, plane_state);
 
intel_add_fb_offsets(, , plane_state, 0);
 
@@ -3105,25 +3119,19 @@ static void i9xx_disable_primary_plane(struct drm_plane 
*primary,
spin_unlock_irqrestore(_priv->uncore.lock, irqflags);
 }
 
-static void ironlake_update_primary_plane(struct drm_plane *primary,
- const struct intel_crtc_state 
*crtc_state,
- const struct intel_plane_state 
*plane_state)
+static u32 ironlake_plane_ctl(const struct intel_crtc_state *crtc_state,
+ const struct intel_plane_state *plane_state)
 {
-   struct drm_device *dev = primary->dev;
-   struct drm_i915_private *dev_priv = to_i915(dev);
-   struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
-   struct drm_framebuffer *fb = plane_state->base.fb;
-   int plane = intel_crtc->plane;
-   u32 linear_offset;
-   u32 dspcntr;
-   i915_reg_t reg = DSPCNTR(plane);
+   struct drm_i915_private *dev_priv =
+   to_i915(plane_state->base.plane->dev);
+   const struct drm_framebuffer *fb = 

Re: [Intel-gfx] [PATCH 6/6] drm/i915: Add more OA configs for BDW, CHV, SKL + BXT

[Robert Bragg]

On Wed, Mar 1, 2017 at 1:00 PM, Matthew Auld
 wrote:
> On 02/22, Robert Bragg wrote:
>> These are auto generated from an XML description of metric sets,
>> currently maintained in gputop, ref:
>>
>>  https://github.com/rib/gputop
>>  > gputop-data/oa-*.xml
>>  > scripts/i915-perf-kernelgen.py
>>
>>  $ make -C gputop-data -f Makefile.xml
>>
>> Signed-off-by: Robert Bragg 
>> ---
>
> 
>
>> +
>> +static const struct i915_oa_reg *
>> +get_compute_extended_mux_config(struct drm_i915_private *dev_priv,
>> + int *len)
>> +{
>> + if (INTEL_INFO(dev_priv)->sseu.subslice_mask & 0x01) {
>> + *len = 
>> ARRAY_SIZE(mux_config_compute_extended_1_0_subslices_0x01);
>> + return mux_config_compute_extended_1_0_subslices_0x01;
>> + } else if (INTEL_INFO(dev_priv)->sseu.subslice_mask & 0x08) {
>> + *len = 
>> ARRAY_SIZE(mux_config_compute_extended_1_1_subslices_0x08);
>> + return mux_config_compute_extended_1_1_subslices_0x08;
>> + } else if (INTEL_INFO(dev_priv)->sseu.subslice_mask & 0x02) {
>> + *len = 
>> ARRAY_SIZE(mux_config_compute_extended_1_2_subslices_0x02);
>> + return mux_config_compute_extended_1_2_subslices_0x02;
>> + } else if (INTEL_INFO(dev_priv)->sseu.subslice_mask & 0x10) {
>> + *len = 
>> ARRAY_SIZE(mux_config_compute_extended_1_3_subslices_0x10);
>> + return mux_config_compute_extended_1_3_subslices_0x10;
>> + } else if (INTEL_INFO(dev_priv)->sseu.subslice_mask & 0x04) {
>> + *len = 
>> ARRAY_SIZE(mux_config_compute_extended_1_4_subslices_0x04);
>> + return mux_config_compute_extended_1_4_subslices_0x04;
>> + } else if (INTEL_INFO(dev_priv)->sseu.subslice_mask & 0x20) {
>> + *len = 
>> ARRAY_SIZE(mux_config_compute_extended_1_5_subslices_0x20);
>> + return mux_config_compute_extended_1_5_subslices_0x20;
>> + *len = ARRAY_SIZE(mux_config_compute_extended);
>> + return mux_config_compute_extended;
> It looks like your script doesn't properly handle the unconditional mux
> config here.

Very well spotted!

This is a pretty ambiguous case and I'm not entirely sure what the
right thing to do here is and have asked the folks that came up with
the original OA config for their feedback. I *think* in this case it's
supposed to combine both as if the unconditional config were logically
concatenated at the end of all the conditional configs.

Locally I've at least updated the i915-perf-kernelgen.py script to
print a loud warning out for this so I could double check in case
there were other similar cases I'd missed - this BDW config seems to
be the only one with this ambiguity though. This makes me think that I
should update the mdapi-xml-convert.py script to just hide this
special case from our oa-*.xml file by explicitly concatenating with
the conditional configs if that's appropriate - then we don't need to
support multiple NOA configs in the i915 perf implementation. The
scripts can then have strict assertions that this doesn't change in
the future.

>
> 
>
>> +
>> +static const struct i915_oa_reg *
>> +get_test_oa_mux_config(struct drm_i915_private *dev_priv,
>> +int *len)
>> +{
>> + *len = ARRAY_SIZE(mux_config_test_oa);
>> + return mux_config_test_oa;
>> +}
>> +
>> +int i915_oa_select_metric_set_bdw(struct drm_i915_private *dev_priv)
>> +{
>> + dev_priv->perf.oa.mux_regs = NULL;
>> + dev_priv->perf.oa.mux_regs_len = 0;
>> + dev_priv->perf.oa.b_counter_regs = NULL;
>> + dev_priv->perf.oa.b_counter_regs_len = 0;
>> + dev_priv->perf.oa.flex_regs = NULL;
>> + dev_priv->perf.oa.flex_regs_len = 0;
>> +
>> + switch (dev_priv->perf.oa.metrics_set) {
>> + case METRIC_SET_ID_RENDER_BASIC:
>> + dev_priv->perf.oa.mux_regs =
>> + get_render_basic_mux_config(dev_priv,
>> + 
>> _priv->perf.oa.mux_regs_len);
>> + if (!dev_priv->perf.oa.mux_regs) {
>> + DRM_DEBUG_DRIVER("No suitable MUX config for 
>> \"RENDER_BASIC\" metric set");
> Your script also needs to output a newline for DRM_DEBUG_DRIVER.

Ah yep, fixed locally thanks,

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

[Intel-gfx] University Survey

[Giacomo Marzi]

Hi! We are conducting an international project with the Stockholm School 
of Economics and the Politecnico of Milan to assess some relevant issues 
about software development.
We’d like you to answer the following survey, which is completely 
anonymous. It’ll take you only 10 minutes!

Link: https://goo.gl/7hCqHT

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

Re: [Intel-gfx] [PATCH 3/3] drm/i915/perf: rate limit spurious oa report notice

[Robert Bragg]

On Tue, Feb 28, 2017 at 1:33 PM, Chris Wilson  wrote:
> On Tue, Feb 28, 2017 at 01:28:13PM +, Matthew Auld wrote:
>> On 22 February 2017 at 15:25, Robert Bragg  wrote:
>> > This change is pre-emptively aiming to avoid a potential cause of kernel
>> > logging noise in case some condition were to result in us seeing invalid
>> > OA reports.
>> >
>> > The workaround for the OA unit's tail pointer race condition is what
>> > avoids the primary know cause of invalid reports being seen and with
>> > that in place we aren't expecting to see this notice but it can't be
>> > entirely ruled out.
>> >
>> > Just in case some condition does lead to the notice then it's likely
>> > that it will be triggered repeatedly while attempting to append a
>> > sequence of reports and depending on the configured OA sampling
>> > frequency that might be a large number of repeat notices.
>> >
>> > Signed-off-by: Robert Bragg 
>> Reviewed-by: Matthew Auld 
>
> printk_ratelimits emits a WARN when triggered, defeating the purpose of
> using NOTE.

Hmm, that's a slightly awkward problem.

The warning comes from the common __ratelimit utility api that's used
for numerous things but by default will warn if the rate limiting
kicked in (once printing resumes again).

There's a RATELIMIT_MSG_ON_RELEASE flag that can be set to def the
warning until ratelimit_state_exit() is called (which looks optional
or at least the flag could also be cleared before calling it to avoid
any warnings).

In general printk_ratelimit() doesn't seem like an ideal mechanism for
throttling messages, even if they are warnings, since the rate limit
state is shared across orthogonal components, so maybe it's best
anyway to use a custom rate limit state.

Considering the _MSG_ON_RELEASE flag, I think I'll init some ratelimit
state in dev_priv->perf.oa just for this message, use
ratelimit_set_flags() to set _MSG_ON_RELEASE and have a corresponding
debug/note in i915_perf_fini after checking the rs->missed counter.

Actually at this point I'm slightly doubting whether having a warning
for throttling is that terrible in this case. Although I tend to think
it could be good to have dedicated ratelimit state here, there
conceptually is a point a which a visible warning could be appropriate
if we're really seeing a lot of these spurious reports. It's a grey
area since it's ok as note if it's rare/intermittent, but somthing's
maybe gone wrong if we see *lots* of these. hmm.

Incidentally, I suppose this same observation is relevant to many of
the DRM_*_RATELIMITED macros too - there will be an inconsistent level
used to notify about any throttling?

Br,
- Robert

> -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 02/14] drm/i915: Use skl_plane_ctl() for the SKL "sprite" planes

[Chris Wilson]

On Mon, Mar 20, 2017 at 06:09:44PM +0200, Ville Syrjälä wrote:
> On Fri, Mar 17, 2017 at 09:48:26PM +, Chris Wilson wrote:
> > On Fri, Mar 17, 2017 at 11:17:56PM +0200, ville.syrj...@linux.intel.com 
> > wrote:
> > > From: Ville Syrjälä 
> > > 
> > > On SKL the planes are uniform so the "sprites" can use the
> > > primary plane code perfectly fine. The only difference we
> > > have is the color key handling, but since we never enable that
> > > for the primary plane the same code works just fine.
> > 
> > Rationale works for me. Does setting the color key report an -EINVAL on
> > the primary?
> 
> Yes.

Reviewed-by: Chris Wilson 
-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 02/14] drm/i915: Use skl_plane_ctl() for the SKL "sprite" planes

[Ville Syrjälä]

On Fri, Mar 17, 2017 at 09:48:26PM +, Chris Wilson wrote:
> On Fri, Mar 17, 2017 at 11:17:56PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > On SKL the planes are uniform so the "sprites" can use the
> > primary plane code perfectly fine. The only difference we
> > have is the color key handling, but since we never enable that
> > for the primary plane the same code works just fine.
> 
> Rationale works for me. Does setting the color key report an -EINVAL on
> the primary?

Yes.

> Even via the legacy setcolorkey ioctl?

That's actually the only way to set it. We never got around to defining
a property for this stuff.

-- 
Ville Syrjälä
Intel OTC
___
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: intel_engine_init_global_seqno() requires atomic kmap

[Patchwork]

== Series Details ==

Series: drm/i915: intel_engine_init_global_seqno() requires atomic kmap
URL   : https://patchwork.freedesktop.org/series/21541/
State : failure

== Summary ==

Series 21541v1 drm/i915: intel_engine_init_global_seqno() requires atomic kmap
https://patchwork.freedesktop.org/api/1.0/series/21541/revisions/1/mbox/

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
fail   -> PASS   (fi-snb-2600) fdo#17
Test gem_mmap_gtt:
Subgroup basic-write-gtt:
pass   -> INCOMPLETE (fi-hsw-4770r)
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
pass   -> INCOMPLETE (fi-byt-j1900)
pass   -> INCOMPLETE (fi-byt-n2820)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-b:
pass   -> INCOMPLETE (fi-skl-6700hq)
Subgroup hang-read-crc-pipe-c:
pass   -> INCOMPLETE (fi-ivb-3520m)
pass   -> INCOMPLETE (fi-bxt-j4205)
Test pm_backlight:
Subgroup basic-brightness:
skip   -> INCOMPLETE (fi-bsw-n3050)
Test pm_rpm:
Subgroup basic-pci-d3-state:
skip   -> INCOMPLETE (fi-snb-2520m)

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

fi-bdw-5557u total:300  pass:284  dwarn:0   dfail:0   fail:0   skip:16  
time: 617s
fi-bsw-n3050 total:300  pass:220  dwarn:0   dfail:0   fail:1   skip:45  
time: 0s
fi-bxt-j4205 total:263  pass:240  dwarn:0   dfail:0   fail:3   skip:19  
time: 0s
fi-byt-j1900 total:240  pass:212  dwarn:0   dfail:0   fail:0   skip:27  
time: 0s
fi-byt-n2820 total:300  pass:208  dwarn:0   dfail:0   fail:0   skip:31  
time: 0s
fi-hsw-4770  total:300  pass:279  dwarn:0   dfail:0   fail:1   skip:20  
time: 664s
fi-hsw-4770r total:300  pass:141  dwarn:0   dfail:0   fail:0   skip:13  
time: 0s
fi-ilk-650   total:300  pass:240  dwarn:0   dfail:0   fail:2   skip:58  
time: 532s
fi-ivb-3520m total:263  pass:238  dwarn:0   dfail:0   fail:5   skip:19  
time: 0s
fi-kbl-7500u total:300  pass:275  dwarn:1   dfail:0   fail:4   skip:20  
time: 634s
fi-skl-6260u total:300  pass:283  dwarn:1   dfail:0   fail:4   skip:12  
time: 650s
fi-skl-6700hqtotal:257  pass:235  dwarn:1   dfail:0   fail:3   skip:17  
time: 0s
fi-skl-6700k total:300  pass:271  dwarn:5   dfail:0   fail:4   skip:20  
time: 665s
fi-skl-6770hqtotal:300  pass:283  dwarn:1   dfail:0   fail:4   skip:12  
time: 691s
fi-snb-2520m total:300  pass:228  dwarn:0   dfail:0   fail:6   skip:34  
time: 0s
fi-snb-2600  total:300  pass:259  dwarn:0   dfail:0   fail:5   skip:36  
time: 597s
fi-bxt-t5700 failed to connect after reboot
fi-ivb-3770 failed to connect after reboot

a4d4230315e8bd8ce20fc86d860ec342c630da65 drm-tip: 2017y-03m-20d-14h-40m-55s UTC 
integration manifest
3f07cb65 drm/i915: intel_engine_init_global_seqno() requires atomic kmap

== Logs ==

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

Re: [Intel-gfx] Potential BUG in drm/i915/execlists: Reset RING registers upon resume

[Jani Nikula]

On Mon, 20 Mar 2017, Greg Kroah-Hartman  wrote:
> On Mon, Mar 20, 2017 at 05:01:34PM +0200, Jani Nikula wrote:
>> On Tue, 14 Mar 2017, Eric Blau  wrote:
>> > That's funny. I have a MacBook Pro 12,1 from late 2015. Hibernate
>> > failed for me in 4.9.6 through 4.9.8 (possibly earlier as well, I do
>> > no recall) without the patch. The patch you reference fixed my problem
>> > and apparently many others based on the bug reports:
>> >
>> > https://bugs.freedesktop.org/show_bug.cgi?id=96526
>> >
>> > I applied the patch myself in 4.9.6 through 4.9.8 and hibernate works
>> > for me. I have run vanilla 4.9.9 and 4.10.1 and in both hibernate
>> > works.
>> 
>> So I'm quite surprised
>> 
>> commit f2a0409a08502d64fbe3990354dff5902b08d2fb
>> Author: Chris Wilson 
>> Date:   Wed Sep 21 14:51:08 2016 +0100
>> 
>> drm/i915/execlists: Reset RING registers upon resume
>> 
>> commit bafb2f7d4755bf1571bd5e9a03b97f3fc4fe69ae upstream.
>> 
>> ended up in v4.9.9 considering that the upstream commit is not annotated
>> for stable, it has no Fixes: tags, and I can't find any backport
>> requests or even notifications for it in my mails. (Admittedly I'm not
>> subscribed on stable@, but I'd expect our lists, maintainers or
>> developers be Cc'd.)
>
> Eric sent this to the stable list a few times, my mistake for not seeing
> he hadn't also cc:ed the developer list.
>
> Want me to revert it?

Chris says that would be the correct thing to do. I'm no expert in the
area, but IIUC having just that commit backported is not enough,
something more would be needed, but we're not sure yet what exactly and
if those dependencies can easily be backported.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

Hi Dave,

drm-intel-next-2017-03-20:
More in i915 for 4.12:

- designware i2c fixes from Hans de Goede, in a topic branch shared
  with other subsystems (maybe, they didn't confirm, but requested the
  pull)
- drop drm_panel usage from the intel dsi vbt panel (Jani)
- vblank evasion improvements and tracing (Maarten and Ville)
- clarify spinlock irq semantics again a bit (Tvrtko)
- new ->pwrite backend hook (right now just for shmem pageche writes),
  from Chris
- more planar/ccs work from Ville
- hotplug safe connector iterators everywhere
- userptr fixes (Chris)
- selftests for cache coloring eviction (Matthew Auld)
- extend debugfs drop_caches interface for shrinker testing (Chris)
- baytrail "the rps kills the machine" fix (Chris)
- use new atomic state iterators, a lot (Maarten)
- refactor guc/huc code some (Arkadiusz Hiler)
- tighten breadcrumbs rbtree a bit (Chris)
- improve wrap-around and time handling in rps residency counters
  (Mika)
- split reset-in-progress in two flags, backoff and handoff (Chris)
- other misc reset improvements from a few people
- bunch of vgpu interaction fixes with recent code changes
- misc stuff all over, as usual

I need backmerges for reasons and a resync with Linus' tree might not hurt
either, so please pull in -rc3 too (won't matter much whether before or
after).

There shouldn't be any conflicts, but the next -fixes pull with gvt stuff
conflicts horribly with -next. Just a heads up, you can deal with that
next week or maybe even later :-)

Cheers, Daniel


The following changes since commit 6796b129b0e98162a84e0b6322ac28587556d427:

  Merge branch 'linux-4.12' of git://github.com/skeggsb/linux into drm-next 
(2017-03-08 12:54:58 +1000)

are available in the git repository at:

  git://anongit.freedesktop.org/git/drm-intel tags/drm-intel-next-2017-03-20

for you to fetch changes up to c5bd2e14e85d180bc7fb3b8b62ac9348bddaf898:

  drm/i915: Update DRIVER_DATE to 20170320 (2017-03-20 08:21:05 +0100)


More in i915 for 4.12:

- designware i2c fixes from Hans de Goede, in a topic branch shared
  with other subsystems (maybe, they didn't confirm, but requested the
  pull)
- drop drm_panel usage from the intel dsi vbt panel (Jani)
- vblank evasion improvements and tracing (Maarten and Ville)
- clarify spinlock irq semantics again a bit (Tvrtko)
- new ->pwrite backend hook (right now just for shmem pageche writes),
  from Chris
- more planar/ccs work from Ville
- hotplug safe connector iterators everywhere
- userptr fixes (Chris)
- selftests for cache coloring eviction (Matthew Auld)
- extend debugfs drop_caches interface for shrinker testing (Chris)
- baytrail "the rps kills the machine" fix (Chris)
- use new atomic state iterators, a lot (Maarten)
- refactor guc/huc code some (Arkadiusz Hiler)
- tighten breadcrumbs rbtree a bit (Chris)
- improve wrap-around and time handling in rps residency counters
  (Mika)
- split reset-in-progress in two flags, backoff and handoff (Chris)
- other misc reset improvements from a few people
- bunch of vgpu interaction fixes with recent code changes
- misc stuff all over, as usual


Ander Conselvan de Oliveira (3):
  drm/i915/glk: Remove MODULE_FIRMWARE() tag from Geminilake's DMC
  drm/i915/glk: Improve rounding caused by pre-CSC gamma tables
  drm/i915/glk: Enable pooled EUs for Geminilake

Andrew Morton (1):
  drivers/gpu/drm/i915/selftests/i915_selftest.c: fix build with gcc-4.4.4

Anusha Srivatsa (1):
  drm/i915/: DMC 1.04 for Geminilake

Arkadiusz Hiler (11):
  drm/i915/uc: Drop superfluous externs in intel_uc.h
  drm/i915/huc: Add huc_to_i915
  drm/i915/uc: Rename intel_?uc_{setup, load}() to _init_hw()
  drm/i915/uc: Move intel_uc_fw_fetch() to intel_uc.c
  drm/i915/uc: Introduce intel_uc_init_fw()
  drm/i915/guc: Extract param logic form guc_init_fw()
  drm/i915/guc: Simplify intel_guc_init_hw()
  drm/i915/uc: Simplify firmware path handling
  drm/i915/uc: Separate firmware selection and preparation
  drm/i915/uc: Add params for specifying firmware
  drm/i915/uc: Rename intel_uc_fw.fw to .type

Bing Niu (1):
  drm/i915: suppress atomic commit error message under gvt-g env

Changbin Du (1):
  drm/i915: make context status notifier head be per engine

Chris Wilson (50):
  drm/i915: Wake up all waiters before idling
  drm/i915: Take rpm wakelock for releasing the fence on unbind
  drm/i915: Avoid clearing the base drm_crtc_state
  drm/i915: Flush idle work when changing missed-irq fault injection
  drm/i915: Store a permanent error in obj->mm.pages
  drm/i915: Use pagecache write to prepopulate shmemfs from pwrite-ioctl
  drm/i915: Avoiding recursing on ww_mutex inside shrinker
  drm/i915: Purge i915_gem_object_is_dead()
  drm/i915: Check for an invalid seqno befo

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Protect intel_engine_wakeup() for call from irq context

[Patchwork]

== Series Details ==

Series: drm/i915: Protect intel_engine_wakeup() for call from irq context
URL   : https://patchwork.freedesktop.org/series/21540/
State : failure

== Summary ==

Series 21540v1 drm/i915: Protect intel_engine_wakeup() for call from irq context
https://patchwork.freedesktop.org/api/1.0/series/21540/revisions/1/mbox/

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
fail   -> PASS   (fi-snb-2600) fdo#17
Test kms_pipe_crc_basic:
Subgroup nonblocking-crc-pipe-a-frame-sequence:
pass   -> FAIL   (fi-skl-6700hq)

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: 462s
fi-bsw-n3050 total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  
time: 580s
fi-bxt-j4205 total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  
time: 533s
fi-bxt-t5700 total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  
time: 554s
fi-byt-j1900 total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  
time: 504s
fi-byt-n2820 total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  
time: 495s
fi-hsw-4770  total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time: 441s
fi-hsw-4770r total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time: 432s
fi-ilk-650   total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  
time: 439s
fi-ivb-3520m total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 514s
fi-ivb-3770  total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 494s
fi-kbl-7500u total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 478s
fi-skl-6260u total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time: 480s
fi-skl-6700hqtotal:278  pass:260  dwarn:0   dfail:0   fail:1   skip:17  
time: 589s
fi-skl-6700k total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  
time: 489s
fi-skl-6770hqtotal:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time: 510s
fi-snb-2520m total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time: 548s
fi-snb-2600  total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  
time: 413s

a4d4230315e8bd8ce20fc86d860ec342c630da65 drm-tip: 2017y-03m-20d-14h-40m-55s UTC 
integration manifest
8c872bd drm/i915: Protect intel_engine_wakeup() for call from irq context

== Logs ==

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

Re: [Intel-gfx] Potential BUG in drm/i915/execlists: Reset RING registers upon resume

[Eric Blau]

On Mon, Mar 20, 2017 at 11:13 AM, Greg Kroah-Hartman
 wrote:
> On Mon, Mar 20, 2017 at 05:01:34PM +0200, Jani Nikula wrote:
>> On Tue, 14 Mar 2017, Eric Blau  wrote:
>> > That's funny. I have a MacBook Pro 12,1 from late 2015. Hibernate
>> > failed for me in 4.9.6 through 4.9.8 (possibly earlier as well, I do
>> > no recall) without the patch. The patch you reference fixed my problem
>> > and apparently many others based on the bug reports:
>> >
>> > https://bugs.freedesktop.org/show_bug.cgi?id=96526
>> >
>> > I applied the patch myself in 4.9.6 through 4.9.8 and hibernate works
>> > for me. I have run vanilla 4.9.9 and 4.10.1 and in both hibernate
>> > works.
>>
>> So I'm quite surprised
>>
>> commit f2a0409a08502d64fbe3990354dff5902b08d2fb
>> Author: Chris Wilson 
>> Date:   Wed Sep 21 14:51:08 2016 +0100
>>
>> drm/i915/execlists: Reset RING registers upon resume
>>
>> commit bafb2f7d4755bf1571bd5e9a03b97f3fc4fe69ae upstream.
>>
>> ended up in v4.9.9 considering that the upstream commit is not annotated
>> for stable, it has no Fixes: tags, and I can't find any backport
>> requests or even notifications for it in my mails. (Admittedly I'm not
>> subscribed on stable@, but I'd expect our lists, maintainers or
>> developers be Cc'd.)
>
> Eric sent this to the stable list a few times, my mistake for not seeing
> he hadn't also cc:ed the developer list.
>
> Want me to revert it?
>
> thanks,
>
> greg k-h

My apologies. I'm new to requesting stable patches, but I thought
sending to the stable mailing lists would keep the proper folks in the
loop. Sorry about that.

The bug that the commit fixes has quite a long documentation trail:

[BAT execlists] Sporadic - gem_exec_suspend basic-s4 GPU hang after resume
https://bugs.freedesktop.org/show_bug.cgi?id=96526

I've had problems with that one and this one that is not fixed in 4.10.x stable:

[Regression BDW] kernel panic in Intel i915 module, complete system
freeze in 4.10-rc2
https://bugs.freedesktop.org/show_bug.cgi?id=99295

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

Re: [Intel-gfx] [PATCH] drm/atomic: protect crtc|connector->state with rcu

[Daniel Vetter]

On Mon, Mar 20, 2017 at 11:01:59AM +0100, Maarten Lankhorst wrote:
> Op 20-03-17 om 09:59 schreef Daniel Vetter:
> > On Mon, Mar 20, 2017 at 09:38:52AM +0100, Maarten Lankhorst wrote:
> >> Op 20-03-17 om 09:18 schreef Daniel Vetter:
> >>> On Fri, Mar 17, 2017 at 05:52:32PM +0100, Maarten Lankhorst wrote:
>  Op 16-03-17 om 21:15 schreef Daniel Vetter:
> > On Thu, Mar 16, 2017 at 5:09 PM, Maarten Lankhorst
> >  wrote:
> >> Op 16-03-17 om 16:52 schreef Daniel Vetter:
> >>> The vblank code really wants to look at crtc->state without having to
> >>> take a ww_mutex. One option might be to take one of the vblank locks
> >>> right when assigning crtc->state, which would ensure that the vblank
> >>> code doesn't race and access freed memory.
> >> I'm not sure this is the right approach for vblank.
> > It's not, it's just that I've had to resurrect this patch quickly
> > before leaving and accidentally left the vblank stuff in.
> >
> >> crtc->state might not be the same as the current state in case of a 
> >> nonblocking
> >> modeset that hasn't even disabled the old crtc yet.
> >>> But userspace tends to poke the vblank_ioctl to query the current
> >>> vblank timestamp rather often, and going all in with rcu would help a
> >>> bit. We're not there yet, but also doesn't really hurt.
> >>>
> >>> v2: Maarten needs this to make connector properties atomic, so he can
> >>> peek at state from the various ->mode_valid hooks.
> >>>
> >>> Cc: Maarten Lankhorst 
> >>> Signed-off-by: Daniel Vetter 
> >>> ---
> >>>  drivers/gpu/drm/drm_atomic.c| 26 +-
> >>>  drivers/gpu/drm/drm_atomic_helper.c |  2 +-
> >>>  include/drm/drm_atomic.h|  5 +
> >>>  include/drm/drm_connector.h | 13 -
> >>>  include/drm/drm_crtc.h  |  9 -
> >>>  5 files changed, 43 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_atomic.c 
> >>> b/drivers/gpu/drm/drm_atomic.c
> >>> index 9b892af7811a..ba11e31ff9ba 100644
> >>> --- a/drivers/gpu/drm/drm_atomic.c
> >>> +++ b/drivers/gpu/drm/drm_atomic.c
> >>> @@ -213,16 +213,10 @@ void drm_atomic_state_clear(struct 
> >>> drm_atomic_state *state)
> >>>  }
> >>>  EXPORT_SYMBOL(drm_atomic_state_clear);
> >>>
> >>> -/**
> >>> - * __drm_atomic_state_free - free all memory for an atomic state
> >>> - * @ref: This atomic state to deallocate
> >>> - *
> >>> - * This frees all memory associated with an atomic state, including 
> >>> all the
> >>> - * per-object state for planes, crtcs and connectors.
> >>> - */
> >>> -void __drm_atomic_state_free(struct kref *ref)
> >>> +void ___drm_atomic_state_free(struct rcu_head *rhead)
> >>>  {
> >>> - struct drm_atomic_state *state = container_of(ref, 
> >>> typeof(*state), ref);
> >>> + struct drm_atomic_state *state =
> >>> + container_of(rhead, typeof(*state), rhead);
> >>>   struct drm_mode_config *config = >dev->mode_config;
> >>>
> >>>   drm_atomic_state_clear(state);
> >>> @@ -236,6 +230,20 @@ void __drm_atomic_state_free(struct kref *ref)
> >>>   kfree(state);
> >>>   }
> >>>  }
> >> whatisRCU.txt:
> >> "This function invokes func(head) after a grace period has elapsed.
> >> This invocation might happen from either softirq or process context,
> >> so the function is not permitted to block."
> >>
> >> Looking at
> >> commit 6f0f02dc56f18760b46dc1bf5b3f7386869d4162
> >> Author: Chris Wilson 
> >> Date:   Mon Jan 23 21:29:39 2017 +
> >>
> >> drm/i915: Move atomic state free from out of fence release
> >>
> >> I would say that drm_atomic_state_free would definitely block..
> >>
> >> The only thing that makes sense IMO is doing kfree_rcu on the 
> >> object_states.
> >> But I don't think RCU is the answer here, it won't protect you against 
> >> using
> >> the wrong crtc state.
> >>
> >> I think I would try to use the crtc ww_mutex in the vblank code and 
> >> serialize it to pending commits, if at all possible.
> > Oops. I guess it should have come with "totally untested". In that
> > case we need a workter which does a synchronize_rcu() before
> > releasing. I don't just want to do a simple kfree_rcu() because by
> > that point we've (partially) destroyed the state alreayd (so it's
> > already unsafe to access, and special ruels are ugly). And doing it
> > here before we release anything in the core would avoid the need for
> > drivers to bother with kfree_rcu().
> >
> > I'll try to respin something less obviously buggy 

Re: [Intel-gfx] Potential BUG in drm/i915/execlists: Reset RING registers upon resume

[Greg Kroah-Hartman]

On Mon, Mar 20, 2017 at 05:01:34PM +0200, Jani Nikula wrote:
> On Tue, 14 Mar 2017, Eric Blau  wrote:
> > That's funny. I have a MacBook Pro 12,1 from late 2015. Hibernate
> > failed for me in 4.9.6 through 4.9.8 (possibly earlier as well, I do
> > no recall) without the patch. The patch you reference fixed my problem
> > and apparently many others based on the bug reports:
> >
> > https://bugs.freedesktop.org/show_bug.cgi?id=96526
> >
> > I applied the patch myself in 4.9.6 through 4.9.8 and hibernate works
> > for me. I have run vanilla 4.9.9 and 4.10.1 and in both hibernate
> > works.
> 
> So I'm quite surprised
> 
> commit f2a0409a08502d64fbe3990354dff5902b08d2fb
> Author: Chris Wilson 
> Date:   Wed Sep 21 14:51:08 2016 +0100
> 
> drm/i915/execlists: Reset RING registers upon resume
> 
> commit bafb2f7d4755bf1571bd5e9a03b97f3fc4fe69ae upstream.
> 
> ended up in v4.9.9 considering that the upstream commit is not annotated
> for stable, it has no Fixes: tags, and I can't find any backport
> requests or even notifications for it in my mails. (Admittedly I'm not
> subscribed on stable@, but I'd expect our lists, maintainers or
> developers be Cc'd.)

Eric sent this to the stable list a few times, my mistake for not seeing
he hadn't also cc:ed the developer list.

Want me to revert it?

thanks,

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

[Intel-gfx] [PATCH] drm/i915: intel_engine_init_global_seqno() requires atomic kmap

[Chris Wilson]

As intel_engine_init_global_seqno() may be called by
nop_submit_request() from inside irq context, we have to use atomic
versions of kmap/kunmap. This is rare as this requires using gen8 legacy
ringbuffer submission.

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

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c
index 4200faa520c7..ef3c62000697 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -242,12 +242,12 @@ void intel_engine_init_global_seqno(struct 
intel_engine_cs *engine, u32 seqno)
void *semaphores;
 
/* Semaphores are in noncoherent memory, flush to be safe */
-   semaphores = kmap(page);
+   semaphores = kmap_atomic(page);
memset(semaphores + GEN8_SEMAPHORE_OFFSET(engine->id, 0),
   0, I915_NUM_ENGINES * gen8_semaphore_seqno_size);
drm_clflush_virt_range(semaphores + 
GEN8_SEMAPHORE_OFFSET(engine->id, 0),
   I915_NUM_ENGINES * 
gen8_semaphore_seqno_size);
-   kunmap(page);
+   kunmap_atomic(semaphores);
}
 
intel_write_status_page(engine, I915_GEM_HWS_INDEX, seqno);
-- 
2.11.0

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

Re: [Intel-gfx] Potential BUG in drm/i915/execlists: Reset RING registers upon resume

[Jani Nikula]

On Tue, 14 Mar 2017, Eric Blau  wrote:
> That's funny. I have a MacBook Pro 12,1 from late 2015. Hibernate
> failed for me in 4.9.6 through 4.9.8 (possibly earlier as well, I do
> no recall) without the patch. The patch you reference fixed my problem
> and apparently many others based on the bug reports:
>
> https://bugs.freedesktop.org/show_bug.cgi?id=96526
>
> I applied the patch myself in 4.9.6 through 4.9.8 and hibernate works
> for me. I have run vanilla 4.9.9 and 4.10.1 and in both hibernate
> works.

So I'm quite surprised

commit f2a0409a08502d64fbe3990354dff5902b08d2fb
Author: Chris Wilson 
Date:   Wed Sep 21 14:51:08 2016 +0100

drm/i915/execlists: Reset RING registers upon resume

commit bafb2f7d4755bf1571bd5e9a03b97f3fc4fe69ae upstream.

ended up in v4.9.9 considering that the upstream commit is not annotated
for stable, it has no Fixes: tags, and I can't find any backport
requests or even notifications for it in my mails. (Admittedly I'm not
subscribed on stable@, but I'd expect our lists, maintainers or
developers be Cc'd.)

BR,
Jani.


>
> Regards,
> Eric
>
> On Mon, Mar 13, 2017 at 6:23 PM, Damian Dominik Martinez Dreyer
>  wrote:
>> I am writing regarding this kernel patch: https://git.kernel.org/pub/sc
>> m/linux/kernel/git/stable/stable-queue.git/tree/releases/4.9.9/drm-
>> i915-execlists-reset-ring-registers-upon-resume.patch .
>>
>> I have identified this by bisect to be the cause of a suspend and
>> resume issue. Please see https://github.com/kxv/linux49.9 for the
>> bisect history.
>>
>> After this change, a Laptop (HP Spectre x360) with Intel HD Graphics
>> 520 will wake up from suspend to a blank screen.
>>
>> Please also see this thread on the manjaro linux forum: https://forum.m
>> anjaro.org/t/linux-4-9-9-breaks-hibernation-4-9-8-works/17499 .
>>
>>
>> Best regards
>> Damian Dominik Martinez Dreyer
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] University Survey

[Giacomo Marzi]

Hi! We are conducting an international project with the Stockholm School 
of Economics and the Politecnico of Milan to assess some relevant issues 
about software development.
We’d like you to answer the following survey, which is completely 
anonymous. It’ll take you only 10 minutes!

Link: https://goo.gl/7hCqHT

Giacomo Marzi
___
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/selftest: Mark the mock struct file backpointer as invalid

[Patchwork]

== Series Details ==

Series: drm/i915/selftest: Mark the mock struct file backpointer as invalid
URL   : https://patchwork.freedesktop.org/series/21539/
State : success

== Summary ==

Series 21539v1 drm/i915/selftest: Mark the mock struct file backpointer as 
invalid
https://patchwork.freedesktop.org/api/1.0/series/21539/revisions/1/mbox/

fi-bdw-5557u total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  
time: 466s
fi-bsw-n3050 total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  
time: 576s
fi-bxt-j4205 total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  
time: 538s
fi-bxt-t5700 total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  
time: 560s
fi-byt-j1900 total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  
time: 502s
fi-byt-n2820 total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  
time: 500s
fi-hsw-4770  total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time: 430s
fi-hsw-4770r total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time: 434s
fi-ilk-650   total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  
time: 433s
fi-ivb-3520m total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 505s
fi-ivb-3770  total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 495s
fi-kbl-7500u total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 476s
fi-skl-6260u total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time: 476s
fi-skl-6700hqtotal:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  
time: 603s
fi-skl-6700k total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  
time: 485s
fi-skl-6770hqtotal:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time: 517s
fi-snb-2520m total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time: 552s
fi-snb-2600  total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  
time: 417s

c19e7f792122951505d7a58d71cd4558d2e926dd drm-tip: 2017y-03m-20d-13h-29m-58s UTC 
integration manifest
d6fee0e drm/i915/selftest: Mark the mock struct file backpointer as invalid

== Logs ==

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

Re: [Intel-gfx] [PATCH] drm/i915: Protect intel_engine_wakeup() for call from irq context

[Chris Wilson]

On Mon, Mar 20, 2017 at 02:39:39PM +, Tvrtko Ursulin wrote:
> 
> On 20/03/2017 14:31, Chris Wilson wrote:
> >intel_engine_wakeup() is called by nop_request_submit() which is
> >installed to handle third party fences completed from within irq
> >context. As such, it needs the full irqsave/irqrestore and not the
> >partial spin_irq_lock handling.
> >
> >[18942.714467] =
> >[18942.719076] [ INFO: inconsistent lock state ]
> >[18942.723522] 4.11.0-rc2-CI-CI_DRM_2368+ #1 Tainted: G U  W
> >[18942.729970] -
> >[18942.734466] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
> >[18942.740594] gem_eio/1275 [HC0[0]:SC0[0]:HE1:SE1] takes:
> >[18942.745932]  (&(>lock)->rlock){+.?...}, at: [] 
> >dma_fence_signal+0x100/0x
> >230
> >[18942.755331] {IN-SOFTIRQ-W} state was registered at:
> >[18942.760356]   __lock_acquire+0x5d0/0x1bb0
> >[18942.76]   lock_acquire+0xc9/0x220
> >[18942.768196]   _raw_spin_lock_irqsave+0x41/0x60
> >[18942.772747]   dma_fence_signal+0x100/0x230
> >[18942.776927]   vgem_fence_timeout+0x9/0x10 [vgem]
> >[18942.781701]   call_timer_fn+0x92/0x380
> >[18942.785557]   expire_timers+0x150/0x1f0
> >[18942.789491]   run_timer_softirq+0x7c/0x160
> >[18942.793705]   __do_softirq+0x116/0x4c0
> >[18942.797560]   irq_exit+0xa9/0xc0
> >[18942.800873]   smp_apic_timer_interrupt+0x38/0x50
> >[18942.805611]   apic_timer_interrupt+0x90/0xa0
> >[18942.810008]   cpuidle_enter_state+0x135/0x380
> >[18942.814503]   cpuidle_enter+0x12/0x20
> >[18942.818250]   call_cpuidle+0x1e/0x40
> >[18942.821906]   do_idle+0x17e/0x1f0
> >[18942.825333]   cpu_startup_entry+0x18/0x20
> >[18942.829463]   rest_init+0x127/0x130
> >[18942.833025]   start_kernel+0x3f1/0x3fe
> >[18942.836908]   x86_64_start_reservations+0x2a/0x2c
> >[18942.841733]   x86_64_start_kernel+0x173/0x186
> >[18942.846234]   verify_cpu+0x0/0xfc
> >[18942.849604] irq event stamp: 30568
> >[18942.853140] hardirqs last  enabled at (30567): [] 
> >ktime_get+0xef/0x120
> >[18942.861468] hardirqs last disabled at (30568): [] 
> >_raw_spin_lock_irqsave+0x17/0
> >x60
> >[18942.870812] softirqs last  enabled at (30462): [] 
> >__do_softirq+0x1d9/0x4c0
> >[18942.879443] softirqs last disabled at (30439): [] 
> >irq_exit+0xa9/0xc0
> >[18942.887616]
> >[18942.887616] other info that might help us debug this:
> >[18942.894279]  Possible unsafe locking scenario:
> >[18942.894279]
> >[18942.900336]CPU0
> >[18942.902851]
> >[18942.905362]   lock(&(>lock)->rlock);
> >[18942.909647]   
> >[18942.912330] lock(&(>lock)->rlock);
> >[18942.916821]
> >[18942.916821]  *** DEADLOCK ***
> >[18942.916821]
> >[18942.922862] 1 lock held by gem_eio/1275:
> >[18942.926859]  #0:  (&(>lock)->rlock){+.?...}, at: 
> >[] dma_fence_signal+0x1
> >00/0x230
> >[18942.936651]
> >[18942.936651] stack backtrace:
> >[18942.941142] CPU: 3 PID: 1275 Comm: gem_eio Tainted: G U  W   
> >4.11.0-rc2-CI-CI_DRM_2368+ #
> >1
> >[18942.950367] Hardware name: Gigabyte Technology Co., Ltd. 
> >Z170X-UD5/Z170X-UD5-CF, BIOS F21 01/06/2
> >017
> >[18942.959756] Call Trace:
> >[18942.962244]  dump_stack+0x67/0x92
> >[18942.965626]  print_usage_bug.part.23+0x259/0x268
> >[18942.970362]  mark_lock+0x12c/0x6f0
> >[18942.973851]  ? check_usage_forwards+0x130/0x130
> >[18942.978487]  mark_held_locks+0x6f/0xa0
> >[18942.982329]  ? _raw_spin_unlock_irq+0x27/0x50
> >[18942.986797]  trace_hardirqs_on_caller+0x150/0x200
> >[18942.991599]  trace_hardirqs_on+0xd/0x10
> >[18942.995515]  _raw_spin_unlock_irq+0x27/0x50
> >[18942.999796]  intel_engine_wakeup+0x26/0x30 [i915]
> >[18943.004670]  intel_engine_init_global_seqno+0x131/0x1a0 [i915]
> >[18943.010745]  nop_submit_request+0x2e/0x40 [i915]
> >[18943.015476]  submit_notify+0x3f/0x5c [i915]
> >[18943.019763]  __i915_sw_fence_complete+0x176/0x220 [i915]
> >[18943.025234]  ? try_to_del_timer_sync+0x4d/0x60
> >[18943.029825]  i915_sw_fence_complete+0x25/0x40 [i915]
> >[18943.034887]  dma_i915_sw_fence_wake+0x26/0x60 [i915]
> >[18943.039959]  dma_fence_signal+0x146/0x230
> >[18943.044109]  vgem_fence_signal_ioctl+0x6c/0xc0 [vgem]
> >[18943.049275]  drm_ioctl+0x200/0x450
> >[18943.052758]  ? vgem_fence_attach_ioctl+0x270/0x270 [vgem]
> >[18943.058334]  do_vfs_ioctl+0x90/0x6e0
> >[18943.061991]  ? entry_SYSCALL_64_fastpath+0x5/0xb1
> >[18943.066843]  ? __this_cpu_preempt_check+0x13/0x20
> >[18943.071643]  ? trace_hardirqs_on_caller+0xe7/0x200
> >[18943.076532]  SyS_ioctl+0x3c/0x70
> >[18943.079842]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> >[18943.084558] RIP: 0033:0x7f0dfcc14357
> >[18943.088240] RSP: 002b:7ffeb4628da8 EFLAGS: 0246 ORIG_RAX: 
> >0010
> >[18943.095996] RAX: ffda RBX: 8147eb93 RCX: 
> >7f0dfcc14357
> >[18943.103311] RDX: 7ffeb4628de0 RSI: 40086442 RDI: 
> >0005
> >[18943.110574] RBP: c9000176ff88 R08: 0004 R09: 
> >
> >[18943.117845] R10: 0029 R11: 

Re: [Intel-gfx] [PATCH] drm/i915: Protect intel_engine_wakeup() for call from irq context

[Tvrtko Ursulin]

On 20/03/2017 14:31, Chris Wilson wrote:

intel_engine_wakeup() is called by nop_request_submit() which is
installed to handle third party fences completed from within irq
context. As such, it needs the full irqsave/irqrestore and not the
partial spin_irq_lock handling.

[18942.714467] =
[18942.719076] [ INFO: inconsistent lock state ]
[18942.723522] 4.11.0-rc2-CI-CI_DRM_2368+ #1 Tainted: G U  W
[18942.729970] -
[18942.734466] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
[18942.740594] gem_eio/1275 [HC0[0]:SC0[0]:HE1:SE1] takes:
[18942.745932]  (&(>lock)->rlock){+.?...}, at: [] 
dma_fence_signal+0x100/0x
230
[18942.755331] {IN-SOFTIRQ-W} state was registered at:
[18942.760356]   __lock_acquire+0x5d0/0x1bb0
[18942.76]   lock_acquire+0xc9/0x220
[18942.768196]   _raw_spin_lock_irqsave+0x41/0x60
[18942.772747]   dma_fence_signal+0x100/0x230
[18942.776927]   vgem_fence_timeout+0x9/0x10 [vgem]
[18942.781701]   call_timer_fn+0x92/0x380
[18942.785557]   expire_timers+0x150/0x1f0
[18942.789491]   run_timer_softirq+0x7c/0x160
[18942.793705]   __do_softirq+0x116/0x4c0
[18942.797560]   irq_exit+0xa9/0xc0
[18942.800873]   smp_apic_timer_interrupt+0x38/0x50
[18942.805611]   apic_timer_interrupt+0x90/0xa0
[18942.810008]   cpuidle_enter_state+0x135/0x380
[18942.814503]   cpuidle_enter+0x12/0x20
[18942.818250]   call_cpuidle+0x1e/0x40
[18942.821906]   do_idle+0x17e/0x1f0
[18942.825333]   cpu_startup_entry+0x18/0x20
[18942.829463]   rest_init+0x127/0x130
[18942.833025]   start_kernel+0x3f1/0x3fe
[18942.836908]   x86_64_start_reservations+0x2a/0x2c
[18942.841733]   x86_64_start_kernel+0x173/0x186
[18942.846234]   verify_cpu+0x0/0xfc
[18942.849604] irq event stamp: 30568
[18942.853140] hardirqs last  enabled at (30567): [] 
ktime_get+0xef/0x120
[18942.861468] hardirqs last disabled at (30568): [] 
_raw_spin_lock_irqsave+0x17/0
x60
[18942.870812] softirqs last  enabled at (30462): [] 
__do_softirq+0x1d9/0x4c0
[18942.879443] softirqs last disabled at (30439): [] 
irq_exit+0xa9/0xc0
[18942.887616]
[18942.887616] other info that might help us debug this:
[18942.894279]  Possible unsafe locking scenario:
[18942.894279]
[18942.900336]CPU0
[18942.902851]
[18942.905362]   lock(&(>lock)->rlock);
[18942.909647]   
[18942.912330] lock(&(>lock)->rlock);
[18942.916821]
[18942.916821]  *** DEADLOCK ***
[18942.916821]
[18942.922862] 1 lock held by gem_eio/1275:
[18942.926859]  #0:  (&(>lock)->rlock){+.?...}, at: [] 
dma_fence_signal+0x1
00/0x230
[18942.936651]
[18942.936651] stack backtrace:
[18942.941142] CPU: 3 PID: 1275 Comm: gem_eio Tainted: G U  W   
4.11.0-rc2-CI-CI_DRM_2368+ #
1
[18942.950367] Hardware name: Gigabyte Technology Co., Ltd. 
Z170X-UD5/Z170X-UD5-CF, BIOS F21 01/06/2
017
[18942.959756] Call Trace:
[18942.962244]  dump_stack+0x67/0x92
[18942.965626]  print_usage_bug.part.23+0x259/0x268
[18942.970362]  mark_lock+0x12c/0x6f0
[18942.973851]  ? check_usage_forwards+0x130/0x130
[18942.978487]  mark_held_locks+0x6f/0xa0
[18942.982329]  ? _raw_spin_unlock_irq+0x27/0x50
[18942.986797]  trace_hardirqs_on_caller+0x150/0x200
[18942.991599]  trace_hardirqs_on+0xd/0x10
[18942.995515]  _raw_spin_unlock_irq+0x27/0x50
[18942.999796]  intel_engine_wakeup+0x26/0x30 [i915]
[18943.004670]  intel_engine_init_global_seqno+0x131/0x1a0 [i915]
[18943.010745]  nop_submit_request+0x2e/0x40 [i915]
[18943.015476]  submit_notify+0x3f/0x5c [i915]
[18943.019763]  __i915_sw_fence_complete+0x176/0x220 [i915]
[18943.025234]  ? try_to_del_timer_sync+0x4d/0x60
[18943.029825]  i915_sw_fence_complete+0x25/0x40 [i915]
[18943.034887]  dma_i915_sw_fence_wake+0x26/0x60 [i915]
[18943.039959]  dma_fence_signal+0x146/0x230
[18943.044109]  vgem_fence_signal_ioctl+0x6c/0xc0 [vgem]
[18943.049275]  drm_ioctl+0x200/0x450
[18943.052758]  ? vgem_fence_attach_ioctl+0x270/0x270 [vgem]
[18943.058334]  do_vfs_ioctl+0x90/0x6e0
[18943.061991]  ? entry_SYSCALL_64_fastpath+0x5/0xb1
[18943.066843]  ? __this_cpu_preempt_check+0x13/0x20
[18943.071643]  ? trace_hardirqs_on_caller+0xe7/0x200
[18943.076532]  SyS_ioctl+0x3c/0x70
[18943.079842]  entry_SYSCALL_64_fastpath+0x1c/0xb1
[18943.084558] RIP: 0033:0x7f0dfcc14357
[18943.088240] RSP: 002b:7ffeb4628da8 EFLAGS: 0246 ORIG_RAX: 
0010
[18943.095996] RAX: ffda RBX: 8147eb93 RCX: 7f0dfcc14357
[18943.103311] RDX: 7ffeb4628de0 RSI: 40086442 RDI: 0005
[18943.110574] RBP: c9000176ff88 R08: 0004 R09: 
[18943.117845] R10: 0029 R11: 0246 R12: 0001
[18943.125168] R13: 0005 R14: 40086442 R15: 
[18943.132520]  ? __this_cpu_preempt_check+0x13/0x20

Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: Mika Kuoppala 
---
 drivers/gpu/drm/i915/intel_breadcrumbs.c | 5 +++--
 1 file changed, 3 

[Intel-gfx] [PATCH] drm/i915: Protect intel_engine_wakeup() for call from irq context

[Chris Wilson]

intel_engine_wakeup() is called by nop_request_submit() which is
installed to handle third party fences completed from within irq
context. As such, it needs the full irqsave/irqrestore and not the
partial spin_irq_lock handling.

[18942.714467] =
[18942.719076] [ INFO: inconsistent lock state ]
[18942.723522] 4.11.0-rc2-CI-CI_DRM_2368+ #1 Tainted: G U  W
[18942.729970] -
[18942.734466] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
[18942.740594] gem_eio/1275 [HC0[0]:SC0[0]:HE1:SE1] takes:
[18942.745932]  (&(>lock)->rlock){+.?...}, at: [] 
dma_fence_signal+0x100/0x
230
[18942.755331] {IN-SOFTIRQ-W} state was registered at:
[18942.760356]   __lock_acquire+0x5d0/0x1bb0
[18942.76]   lock_acquire+0xc9/0x220
[18942.768196]   _raw_spin_lock_irqsave+0x41/0x60
[18942.772747]   dma_fence_signal+0x100/0x230
[18942.776927]   vgem_fence_timeout+0x9/0x10 [vgem]
[18942.781701]   call_timer_fn+0x92/0x380
[18942.785557]   expire_timers+0x150/0x1f0
[18942.789491]   run_timer_softirq+0x7c/0x160
[18942.793705]   __do_softirq+0x116/0x4c0
[18942.797560]   irq_exit+0xa9/0xc0
[18942.800873]   smp_apic_timer_interrupt+0x38/0x50
[18942.805611]   apic_timer_interrupt+0x90/0xa0
[18942.810008]   cpuidle_enter_state+0x135/0x380
[18942.814503]   cpuidle_enter+0x12/0x20
[18942.818250]   call_cpuidle+0x1e/0x40
[18942.821906]   do_idle+0x17e/0x1f0
[18942.825333]   cpu_startup_entry+0x18/0x20
[18942.829463]   rest_init+0x127/0x130
[18942.833025]   start_kernel+0x3f1/0x3fe
[18942.836908]   x86_64_start_reservations+0x2a/0x2c
[18942.841733]   x86_64_start_kernel+0x173/0x186
[18942.846234]   verify_cpu+0x0/0xfc
[18942.849604] irq event stamp: 30568
[18942.853140] hardirqs last  enabled at (30567): [] 
ktime_get+0xef/0x120
[18942.861468] hardirqs last disabled at (30568): [] 
_raw_spin_lock_irqsave+0x17/0
x60
[18942.870812] softirqs last  enabled at (30462): [] 
__do_softirq+0x1d9/0x4c0
[18942.879443] softirqs last disabled at (30439): [] 
irq_exit+0xa9/0xc0
[18942.887616]
[18942.887616] other info that might help us debug this:
[18942.894279]  Possible unsafe locking scenario:
[18942.894279]
[18942.900336]CPU0
[18942.902851]
[18942.905362]   lock(&(>lock)->rlock);
[18942.909647]   
[18942.912330] lock(&(>lock)->rlock);
[18942.916821]
[18942.916821]  *** DEADLOCK ***
[18942.916821]
[18942.922862] 1 lock held by gem_eio/1275:
[18942.926859]  #0:  (&(>lock)->rlock){+.?...}, at: [] 
dma_fence_signal+0x1
00/0x230
[18942.936651]
[18942.936651] stack backtrace:
[18942.941142] CPU: 3 PID: 1275 Comm: gem_eio Tainted: G U  W   
4.11.0-rc2-CI-CI_DRM_2368+ #
1
[18942.950367] Hardware name: Gigabyte Technology Co., Ltd. 
Z170X-UD5/Z170X-UD5-CF, BIOS F21 01/06/2
017
[18942.959756] Call Trace:
[18942.962244]  dump_stack+0x67/0x92
[18942.965626]  print_usage_bug.part.23+0x259/0x268
[18942.970362]  mark_lock+0x12c/0x6f0
[18942.973851]  ? check_usage_forwards+0x130/0x130
[18942.978487]  mark_held_locks+0x6f/0xa0
[18942.982329]  ? _raw_spin_unlock_irq+0x27/0x50
[18942.986797]  trace_hardirqs_on_caller+0x150/0x200
[18942.991599]  trace_hardirqs_on+0xd/0x10
[18942.995515]  _raw_spin_unlock_irq+0x27/0x50
[18942.999796]  intel_engine_wakeup+0x26/0x30 [i915]
[18943.004670]  intel_engine_init_global_seqno+0x131/0x1a0 [i915]
[18943.010745]  nop_submit_request+0x2e/0x40 [i915]
[18943.015476]  submit_notify+0x3f/0x5c [i915]
[18943.019763]  __i915_sw_fence_complete+0x176/0x220 [i915]
[18943.025234]  ? try_to_del_timer_sync+0x4d/0x60
[18943.029825]  i915_sw_fence_complete+0x25/0x40 [i915]
[18943.034887]  dma_i915_sw_fence_wake+0x26/0x60 [i915]
[18943.039959]  dma_fence_signal+0x146/0x230
[18943.044109]  vgem_fence_signal_ioctl+0x6c/0xc0 [vgem]
[18943.049275]  drm_ioctl+0x200/0x450
[18943.052758]  ? vgem_fence_attach_ioctl+0x270/0x270 [vgem]
[18943.058334]  do_vfs_ioctl+0x90/0x6e0
[18943.061991]  ? entry_SYSCALL_64_fastpath+0x5/0xb1
[18943.066843]  ? __this_cpu_preempt_check+0x13/0x20
[18943.071643]  ? trace_hardirqs_on_caller+0xe7/0x200
[18943.076532]  SyS_ioctl+0x3c/0x70
[18943.079842]  entry_SYSCALL_64_fastpath+0x1c/0xb1
[18943.084558] RIP: 0033:0x7f0dfcc14357
[18943.088240] RSP: 002b:7ffeb4628da8 EFLAGS: 0246 ORIG_RAX: 
0010
[18943.095996] RAX: ffda RBX: 8147eb93 RCX: 7f0dfcc14357
[18943.103311] RDX: 7ffeb4628de0 RSI: 40086442 RDI: 0005
[18943.110574] RBP: c9000176ff88 R08: 0004 R09: 
[18943.117845] R10: 0029 R11: 0246 R12: 0001
[18943.125168] R13: 0005 R14: 40086442 R15: 
[18943.132520]  ? __this_cpu_preempt_check+0x13/0x20

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

diff --git 

Re: [Intel-gfx] [PATCH] drm/i915: Reset tasklet back to execlists after disabling guc

[Chris Wilson]

On Sun, Mar 19, 2017 at 10:58:12PM +0100, Michał Winiarski wrote:
> On Sat, Mar 18, 2017 at 10:28:59AM +, Chris Wilson wrote:
> > When switching back to execlists, we also now need to restore the
> > tasklet handler.
> > 
> > Reported-by: Oscar Mateo 
> > Fixes: 31de73501ac9 ("drm/i915/scheduler: emulate a scheduler for guc")
> > Signed-off-by: Chris Wilson 
> > Cc: Tvrtko Ursulin 
> > Cc: Joonas Lahtinen 
> > Cc: Michał Winiarski 
> > Cc: Oscar Mateo 
> 
> Reviewed-by: Michał Winiarski 

Idempotency is good. Pushed, thanks.
-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] drm/i915: Skip force-wake for uncached mmio flush of GGTT writes

[Chris Wilson]

On Mon, Mar 20, 2017 at 12:38:31PM +0200, Mika Kuoppala wrote:
> Chris Wilson  writes:
> 
> > The trick of using an uncached mmio read to ensure that the GGTT writes
> > are flushed does not require us to do the forcewake dance, so avoid it
> > in the hope of reducing the frequency that we do keep the device forced
> > awake.
> >
> > Signed-off-by: Chris Wilson 
> 
> Reviewed-by: Mika Kuoppala 

Applied, thanks.
-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] drm/i915/guc: Correct the request_in tracepoint position

[Chris Wilson]

On Mon, Mar 20, 2017 at 01:41:28PM +, Tvrtko Ursulin wrote:
> 
> On 20/03/2017 13:37, Chris Wilson wrote:
> >On Mon, Mar 20, 2017 at 01:25:56PM +, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin 
> >>
> >>It has to be called after the global seqno has been assigned.
> >>
> >>Signed-off-by: Tvrtko Ursulin 
> >>Fixes: 31de73501ac9 ("drm/i915/scheduler: emulate a scheduler for guc")
> >>Cc: Chris Wilson 
> >
> >So be it. Your script is ready for the same request to be submitted
> 
> Why so be it? It matches the one in execlists and is logical.

Just the staircasing, that's all.
-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/guc: Correct the request_in tracepoint position

[Patchwork]

== Series Details ==

Series: drm/i915/guc: Correct the request_in tracepoint position
URL   : https://patchwork.freedesktop.org/series/21538/
State : success

== Summary ==

Series 21538v1 drm/i915/guc: Correct the request_in tracepoint position
https://patchwork.freedesktop.org/api/1.0/series/21538/revisions/1/mbox/

Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass   -> DMESG-WARN (fi-bxt-t5700) fdo#100125

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

fi-bdw-5557u total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  
time: 460s
fi-bsw-n3050 total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  
time: 574s
fi-bxt-j4205 total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  
time: 532s
fi-bxt-t5700 total:278  pass:257  dwarn:1   dfail:0   fail:0   skip:20  
time: 559s
fi-byt-j1900 total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  
time: 498s
fi-byt-n2820 total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  
time: 495s
fi-hsw-4770  total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time: 433s
fi-hsw-4770r total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time: 437s
fi-ilk-650   total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  
time: 438s
fi-ivb-3520m total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 515s
fi-ivb-3770  total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 494s
fi-kbl-7500u total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 482s
fi-skl-6260u total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time: 477s
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: 484s
fi-skl-6770hqtotal:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time: 515s
fi-snb-2520m total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time: 552s
fi-snb-2600  total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  
time: 425s

3f1ca80b4b2cd698202fc8121029bdc094bf7d7f drm-tip: 2017y-03m-20d-10h-48m-39s UTC 
integration manifest
b69cfd8 drm/i915/guc: Correct the request_in tracepoint position

== Logs ==

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

Re: [Intel-gfx] [PATCH] drm/i915/guc: Correct the request_in tracepoint position

[Tvrtko Ursulin]

On 20/03/2017 13:37, Chris Wilson wrote:

On Mon, Mar 20, 2017 at 01:25:56PM +, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

It has to be called after the global seqno has been assigned.

Signed-off-by: Tvrtko Ursulin 
Fixes: 31de73501ac9 ("drm/i915/scheduler: emulate a scheduler for guc")
Cc: Chris Wilson 


So be it. Your script is ready for the same request to be submitted


Why so be it? It matches the one in execlists and is logical.


multiple times with different global_seqno?


Nope, but it survived the zero global seqno unexpectedly well! :)


Reviewed-by: Chris Wilson 


Thanks!

Regards,

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

Re: [Intel-gfx] [PATCH] drm/i915/selftest: Mark the mock struct file backpointer as invalid

[Chris Wilson]

On Mon, Mar 20, 2017 at 01:30:21PM +, Chris Wilson wrote:
> We do not use the struct file backpointer so struct drm_file, so mark it
s/so/from/
> as invalid when creating a mock file for i915 selftests.

It is only used by legacy vm mmapping, not by i915.ko
-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] drm/i915/guc: Correct the request_in tracepoint position

[Chris Wilson]

On Mon, Mar 20, 2017 at 01:25:56PM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin 
> 
> It has to be called after the global seqno has been assigned.
> 
> Signed-off-by: Tvrtko Ursulin 
> Fixes: 31de73501ac9 ("drm/i915/scheduler: emulate a scheduler for guc")
> Cc: Chris Wilson 

So be it. Your script is ready for the same request to be submitted
multiple times with different global_seqno?

Reviewed-by: Chris Wilson 
-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] [PATCH] drm/i915/selftest: Mark the mock struct file backpointer as invalid

[Chris Wilson]

We do not use the struct file backpointer so struct drm_file, so mark it
as invalid when creating a mock file for i915 selftests.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/selftests/mock_drm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/selftests/mock_drm.c 
b/drivers/gpu/drm/i915/selftests/mock_drm.c
index 113dec05c7dc..8fd1c4ded60f 100644
--- a/drivers/gpu/drm/i915/selftests/mock_drm.c
+++ b/drivers/gpu/drm/i915/selftests/mock_drm.c
@@ -41,6 +41,7 @@ struct drm_file *mock_file(struct drm_i915_private *i915)
return ERR_PTR(err);
 
file = filp.private_data;
+   memset(>filp, POISON_INUSE, sizeof(file->filp));
file->authenticated = true;
return file;
 }
-- 
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/guc: Correct the request_in tracepoint position

[Tvrtko Ursulin]

From: Tvrtko Ursulin 

It has to be called after the global seqno has been assigned.

Signed-off-by: Tvrtko Ursulin 
Fixes: 31de73501ac9 ("drm/i915/scheduler: emulate a scheduler for guc")
Cc: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: Joonas Lahtinen 
Cc: Michał Winiarski 
Cc: Daniel Vetter 
Cc: Jani Nikula 
Cc: intel-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
b/drivers/gpu/drm/i915/i915_guc_submission.c
index 4514168c55f3..67fa6fdfad77 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -597,8 +597,8 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
RB_CLEAR_NODE(>priotree.node);
rq->priotree.priority = INT_MAX;
 
-   trace_i915_gem_request_in(rq, port - engine->execlist_port);
i915_guc_submit(rq);
+   trace_i915_gem_request_in(rq, port - engine->execlist_port);
last = rq;
submit = true;
}
-- 
2.9.3

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

Re: [Intel-gfx] [dim PATCH 00/10] dim: shellcheck fixes

[Jani Nikula]

On Fri, 17 Mar 2017, Daniel Vetter  wrote:
> On Fri, Mar 17, 2017 at 12:42:51PM +0200, Jani Nikula wrote:
>> Noticing that 64a54c5e1a5d ("dim: Add add-link command") added a
>> condition that is always true (if [ -n $foo ]), despite my review of the
>> patch, I refreshed some of my old patches to fix issues that can be
>> caught by shellcheck.
>> 
>> Starting from patch 1, the minimal merge criteria for any new dim
>> patches is that 'make shellcheck' passes.
>> 
>> I add shellcheck exceptions for starters, so there are no errors
>> reported, and then remove the exceptions by fixing them one by one. I'm
>> not sure if we'll ever fix them all, but let's at least make sure we're
>> not adding any new silly mistakes.
>
> Ack on the entire series. checkpatch for dim!

Thanks, pushed.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [09/11] drm/i915/guc: A little bit more of doorbell sanitization

[Joonas Lahtinen]

On pe, 2017-03-10 at 08:28 -0800, Oscar Mateo wrote:
> Some recent refactoring patches have left the doorbell creation outside
> the GuC client allocation, which does not make a lot of sense (a client
> without a doorbell is something useless). Move it back there, and
> refactor the init_doorbell_hw consequently.
> 
> Thanks to this, we can do some other improvements, like hoisting the
> check for GuC submission enabled out of the enable function.
> 
> Cc: Joonas Lahtinen 
> Cc: Michal Wajdeczko 
> Cc: Arkadiusz Hiler 
> Cc: Tvrtko Ursulin 
> Cc: Chris Wilson 
> Cc: Daniele Ceraolo Spurio 
> Signed-off-by: Oscar Mateo 

Looks good.

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/atomic: protect crtc|connector->state with rcu

[Maarten Lankhorst]

Op 20-03-17 om 11:22 schreef Chris Wilson:
> On Mon, Mar 20, 2017 at 11:01:59AM +0100, Maarten Lankhorst wrote:
>> Op 20-03-17 om 09:59 schreef Daniel Vetter:
>>> But my idea was kinda that we'd do the same for probe -> modeset data
>>> flows like here for the other way round: Just a bunch of READ_ONCE and
>>> maybe lookup the edid with rcu too. That way it's clear to anybody that
>>> probe and modeset are entirely not synchronized.
>> Though I think it's beneficial to lock them. if it is. I'm not sure there
>> are many usecases for parallel modeset vs probe. And if someone does care,
>> they can use nonblocking modesets, maybe.
>>
>> Legacy userspace probably can't do blocking modeset and probe at the same
>> time, so I don't think it will regress.
> It can happen - just consider two clients, such as system-logind walking
> sysfs. Delaying most modesets would not be an issue, except where the
> modeset is being uses to e.g. changes strides before in the middle of a
> pageflip sequence.
With atomic pageflip, this is no longer required on i915. :)
> I would welcome kms_flip flip/set/vblank-vs-probe, and definitely should
> add it to legacy-cursor and legacy-plane as well.
Could be useful, the normal paths should never take connection_mutex anyway,
so either way it should pass.

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

Re: [Intel-gfx] [PATCH 1/3] drm/i915: allocate mock file pointer dynamically

[Arnd Bergmann]

On Mon, Mar 20, 2017 at 1:02 PM, Arnd Bergmann  wrote:
> On Mon, Mar 20, 2017 at 1:00 PM, Joonas Lahtinen
>  wrote:
>> On ma, 2017-03-20 at 10:40 +0100, Arnd Bergmann wrote:
>
>>> diff --git a/drivers/gpu/drm/i915/selftests/mock_drm.c 
>>> b/drivers/gpu/drm/i915/selftests/mock_drm.c
>>> index 113dec05c7dc..18514065c93d 100644
>>> --- a/drivers/gpu/drm/i915/selftests/mock_drm.c
>>> +++ b/drivers/gpu/drm/i915/selftests/mock_drm.c
>>> @@ -32,15 +32,15 @@ static inline struct inode fake_inode(struct 
>>> drm_i915_private *i915)
>>>  struct drm_file *mock_file(struct drm_i915_private *i915)
>>>  {
>>> > struct inode inode = fake_inode(i915);
>>> > -   struct file filp = {};
>>> > +   struct file *filp = kzalloc(sizeof(struct file), GFP_KERNEL);
>>> > struct drm_file *file;
>>> > int err;
>>>
>>
>> filp = kzalloc(sizeof(*filp), GFP_KERNEL);
>> if (unlikely(!filp)) {
>> err = -ENOMEM;
>> goto err;
>> }
>>
>> And appropriate onion teardown in case drm_open fails, so that we don't
>> leak memory.
>
> Oops, of course you are right, sorry about that.

Actually it's even worse, as I had originally planned to also allocate the inode
dynamically, but for some reasons didn't do that in the patch I sent.

This version of the patch as I sent it is rather bogus, so please ignore it
(but not the other two patches I sent). If David's solution to this problem
can make it into 4.12, there is no problem at all. Another alternative
would be to use anon_inode_getfile(), again with proper error handling
and cleanup. This is a bit slower but gives us valid file and inode structures.

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

Re: [Intel-gfx] [PATCH 1/3] drm/i915: allocate mock file pointer dynamically

[Arnd Bergmann]

On Mon, Mar 20, 2017 at 1:00 PM, Joonas Lahtinen
 wrote:
> On ma, 2017-03-20 at 10:40 +0100, Arnd Bergmann wrote:

>> diff --git a/drivers/gpu/drm/i915/selftests/mock_drm.c 
>> b/drivers/gpu/drm/i915/selftests/mock_drm.c
>> index 113dec05c7dc..18514065c93d 100644
>> --- a/drivers/gpu/drm/i915/selftests/mock_drm.c
>> +++ b/drivers/gpu/drm/i915/selftests/mock_drm.c
>> @@ -32,15 +32,15 @@ static inline struct inode fake_inode(struct 
>> drm_i915_private *i915)
>>  struct drm_file *mock_file(struct drm_i915_private *i915)
>>  {
>> > struct inode inode = fake_inode(i915);
>> > -   struct file filp = {};
>> > +   struct file *filp = kzalloc(sizeof(struct file), GFP_KERNEL);
>> > struct drm_file *file;
>> > int err;
>>
>
> filp = kzalloc(sizeof(*filp), GFP_KERNEL);
> if (unlikely(!filp)) {
> err = -ENOMEM;
> goto err;
> }
>
> And appropriate onion teardown in case drm_open fails, so that we don't
> leak memory.

Oops, of course you are right, sorry about that.

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

Re: [Intel-gfx] [PATCH 1/3] drm/i915: allocate mock file pointer dynamically

[Joonas Lahtinen]

On ma, 2017-03-20 at 10:40 +0100, Arnd Bergmann wrote:
> A struct file is a bit too large to put on the kernel stack in general
> and triggers a warning for low settings of CONFIG_FRAME_WARN:
> 
> drivers/gpu/drm/i915/selftests/mock_drm.c: In function 'mock_file':
> drivers/gpu/drm/i915/selftests/mock_drm.c:46:1: error: the frame size of 1328 
> bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
> drivers/gpu/drm/i915/selftests/mock_drm.c: In function 'mock_file_free':
> drivers/gpu/drm/i915/selftests/mock_drm.c:54:1: error: the frame size of 1312 
> bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
> 
> It's also slightly dangerous to leave a reference to a stack object
> in the drm_file structure after leaving the stack frame.
> This changes the code to just allocate the object dynamically
> and release it when we are done with it.
> 
> Fixes: 66d9cb5d805a ("drm/i915: Mock the GEM device for self-testing")
> Signed-off-by: Arnd Bergmann 



> ---
>  drivers/gpu/drm/i915/selftests/mock_drm.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/mock_drm.c 
> b/drivers/gpu/drm/i915/selftests/mock_drm.c
> index 113dec05c7dc..18514065c93d 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_drm.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_drm.c
> @@ -32,15 +32,15 @@ static inline struct inode fake_inode(struct 
> drm_i915_private *i915)
>  struct drm_file *mock_file(struct drm_i915_private *i915)
>  {
> >     struct inode inode = fake_inode(i915);
> > -   struct file filp = {};
> > +   struct file *filp = kzalloc(sizeof(struct file), GFP_KERNEL);
> >     struct drm_file *file;
> >     int err;
>  

filp = kzalloc(sizeof(*filp), GFP_KERNEL);
if (unlikely(!filp)) {
err = -ENOMEM;
goto err;
}

And appropriate onion teardown in case drm_open fails, so that we don't
leak memory.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] [RFC] Revert "drm/i915: use variadic macros and arrays to choose port/pipe based registers"

[Jani Nikula]

On Mon, 20 Mar 2017, Arnd Bergmann  wrote:
> I don't know how to generate a URL for it, but after adding this to the
> command line for gcc-7,
>
> -fsanitize=kernel-address -fasan-shadow-offset=0xdfff9000
> --param asan-stack=1 --param asan-globals=1 --param
> asan-instrumentation-with-call-threshold=1
> -fsanitize-address-use-after-scope
>
> the code turned from really nice into the log series of checks below.
> Without  -fsanitize-address-use-after-scope (which didn't exist before gcc-7),
> it's less bad but still exceeds the (arbitrary) 1536 byte limit.

It seems to be the combination of --param asan-stack=1 and
-fsanitize-address-use-after-scope that really blows up the code [1]. I
filed a GCC bug on it, mostly to see what they say [2]. I don't know,
maybe they think it's expected. *shrug*.


BR,
Jani.

[1] https://godbolt.org/g/hgS817
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80114


-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Skip force-wake for uncached mmio flush of GGTT writes

[Mika Kuoppala]

Chris Wilson  writes:

> The trick of using an uncached mmio read to ensure that the GGTT writes
> are flushed does not require us to do the forcewake dance, so avoid it
> in the hope of reducing the frequency that we do keep the device forced
> awake.
>
> Signed-off-by: Chris Wilson 

Reviewed-by: Mika Kuoppala 

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 5e280e3bfd86..d468300e2f05 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3094,8 +3094,11 @@ i915_gem_object_flush_gtt_write_domain(struct 
> drm_i915_gem_object *obj)
>* system agents we cannot reproduce this behaviour).
>*/
>   wmb();
> - if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv))
> - POSTING_READ(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
> + if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
> + spin_lock_irq(_priv->uncore.lock);
> + POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
> + spin_unlock_irq(_priv->uncore.lock);
> + }
>  
>   intel_fb_obj_flush(obj, write_origin(obj, I915_GEM_DOMAIN_GTT));
>  
> -- 
> 2.11.0
>
> ___
> 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 3/3] [RFC] Revert "drm/i915: use variadic macros and arrays to choose port/pipe based registers"

[Arnd Bergmann]

On Mon, Mar 20, 2017 at 11:08 AM, Jani Nikula
 wrote:
> On Mon, 20 Mar 2017, Arnd Bergmann  wrote:
>> The varargs macro trick in _PIPE3/_PHY3/_PORT3 was meant as an optimization
>> to shrink the i915 kernel module by around 1000 bytes.
>
> To be clear, it was not at all intended to be an optimization, nothing
> of the sort. The intention was to make it easier and less error prone to
> add more parameters to said macros. The text size shring was just a
> bonus.
>
>> However, the
>> downside is a size regression with CONFIG_KASAN, as I found from stack size
>> warnings with gcc-7.0.1:
>
> In his review of the original change, Chris provided this comparison
> https://godbolt.org/g/YCK1od
>
> How does CONFIG_KASAN change this? Would be nice to see how the
> generated code blows up.
>
I don't know how to generate a URL for it, but after adding this to the
command line for gcc-7,

-fsanitize=kernel-address -fasan-shadow-offset=0xdfff9000
--param asan-stack=1 --param asan-globals=1 --param
asan-instrumentation-with-call-threshold=1
-fsanitize-address-use-after-scope

the code turned from really nice into the log series of checks below.
Without  -fsanitize-address-use-after-scope (which didn't exist before gcc-7),
it's less bad but still exceeds the (arbitrary) 1536 byte limit.

 Arnd

.LC0:
.string "2 32 4 1 i 96 24 9  "
main:
.LASANPC0:
pushq   %r12
pushq   %rbp
movabsq $-2305966154516004864, %rdx
pushq   %rbx
subq$160, %rsp
movq%rsp, %rbp
leaq96(%rsp), %rbx
movq$1102416563, (%rsp)
shrq$3, %rbp
movq$.LC0, 8(%rsp)
movq$.LASANPC0, 16(%rsp)
leaq0(%rbp,%rdx), %rax
movl$-235802127, (%rax)
movl$-218959356, 4(%rax)
movl$-218959118, 8(%rax)
movl$-234881024, 12(%rax)
movl$-202116109, 16(%rax)
movq%rbx, %rax
movl$1, 32(%rsp)
shrq$3, %rax
movzbl  (%rax,%rdx), %eax
testb   %al, %al
je  .L2
cmpb$3, %al
jle .L53
.L2:
leaq4(%rbx), %rdi
movabsq $-2305966154516004864, %rax
movl$0, 96(%rsp)
movq%rdi, %rdx
shrq$3, %rdx
movzbl  (%rdx,%rax), %edx
movq%rdi, %rax
andl$7, %eax
addl$3, %eax
cmpb%dl, %al
jl  .L3
testb   %dl, %dl
jne .L54
.L3:
leaq8(%rbx), %rdi
movabsq $-2305966154516004864, %rax
movl$1, 100(%rsp)
movq%rdi, %rdx
shrq$3, %rdx
movzbl  (%rdx,%rax), %eax
testb   %al, %al
je  .L4
cmpb$3, %al
jle .L55
.L4:
leaq12(%rbx), %rdi
movabsq $-2305966154516004864, %rax
movl$2, 104(%rsp)
movq%rdi, %rdx
shrq$3, %rdx
movzbl  (%rdx,%rax), %edx
movq%rdi, %rax
andl$7, %eax
addl$3, %eax
cmpb%dl, %al
jl  .L5
testb   %dl, %dl
jne .L56
.L5:
leaq16(%rbx), %rdi
movabsq $-2305966154516004864, %rax
movl$3, 108(%rsp)
movq%rdi, %rdx
shrq$3, %rdx
movzbl  (%rdx,%rax), %eax
testb   %al, %al
je  .L6
cmpb$3, %al
jle .L57
.L6:
leaq20(%rbx), %rdi
movabsq $-2305966154516004864, %rax
movl$4, 112(%rsp)
movq%rdi, %rdx
shrq$3, %rdx
movzbl  (%rdx,%rax), %edx
movq%rdi, %rax
andl$7, %eax
addl$3, %eax
cmpb%dl, %al
jl  .L7
testb   %dl, %dl
jne .L58
.L7:
movslq  32(%rsp), %r12
movabsq $-2305966154516004864, %rax
movl$5, 116(%rsp)
leaq(%rbx,%r12,4), %rdi
movq%rdi, %rdx
shrq$3, %rdx
movzbl  (%rdx,%rax), %edx
movq%rdi, %rax
andl$7, %eax
addl$3, %eax
cmpb%dl, %al
jl  .L8
testb   %dl, %dl
jne .L59
.L8:
pxor%xmm0, %xmm0
movabsq $-2305966154516004864, %rdx
movl96(%rsp,%r12,4), %eax
movl$0, 16(%rdx,%rbp)
movups  %xmm0, 0(%rbp,%rdx)
addq$160, %rsp
popq%rbx
popq%rbp
popq%r12
ret
.L59:
call__asan_report_load4_noabort
jmp .L8
.L58:
call__asan_report_store4_noabort
jmp .L7
.L57:
call__asan_report_store4_noabort
jmp .L6
.L56:
call__asan_report_store4_noabort
jmp .L5
.L55:
call__asan_report_store4_noabort
jmp .L4
.L54:
call__asan_report_store4_noabort
jmp .L3
.L53:
movq%rbx, %rdi
call

Re: [Intel-gfx] [PATCH] drm/atomic: protect crtc|connector->state with rcu

[Chris Wilson]

On Mon, Mar 20, 2017 at 11:01:59AM +0100, Maarten Lankhorst wrote:
> Op 20-03-17 om 09:59 schreef Daniel Vetter:
> > But my idea was kinda that we'd do the same for probe -> modeset data
> > flows like here for the other way round: Just a bunch of READ_ONCE and
> > maybe lookup the edid with rcu too. That way it's clear to anybody that
> > probe and modeset are entirely not synchronized.
> 
> Though I think it's beneficial to lock them. if it is. I'm not sure there
> are many usecases for parallel modeset vs probe. And if someone does care,
> they can use nonblocking modesets, maybe.
> 
> Legacy userspace probably can't do blocking modeset and probe at the same
> time, so I don't think it will regress.

It can happen - just consider two clients, such as system-logind walking
sysfs. Delaying most modesets would not be an issue, except where the
modeset is being uses to e.g. changes strides before in the middle of a
pageflip sequence.

I would welcome kms_flip flip/set/vblank-vs-probe, and definitely should
add it to legacy-cursor and legacy-plane 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

Re: [Intel-gfx] [PATCH 3/3] [RFC] Revert "drm/i915: use variadic macros and arrays to choose port/pipe based registers"

[Jani Nikula]

On Mon, 20 Mar 2017, Arnd Bergmann  wrote:
> The varargs macro trick in _PIPE3/_PHY3/_PORT3 was meant as an optimization
> to shrink the i915 kernel module by around 1000 bytes.

To be clear, it was not at all intended to be an optimization, nothing
of the sort. The intention was to make it easier and less error prone to
add more parameters to said macros. The text size shring was just a
bonus.

> However, the
> downside is a size regression with CONFIG_KASAN, as I found from stack size
> warnings with gcc-7.0.1:

In his review of the original change, Chris provided this comparison
https://godbolt.org/g/YCK1od

How does CONFIG_KASAN change this? Would be nice to see how the
generated code blows up.


BR,
Jani.


>
> before:
> drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'bxt_ddi_pll_get_hw_state':
> drivers/gpu/drm/i915/intel_dpll_mgr.c:1644:1: error: the frame size of 176 
> bytes is larger than 100 bytes [-Werror=frame-larger-than=]
> drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'bxt_ddi_pll_enable':
> drivers/gpu/drm/i915/intel_dpll_mgr.c:1548:1: error: the frame size of 224 
> bytes is larger than 100 bytes [-Werror=frame-larger-than=]
>
> after:
> drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'bxt_ddi_pll_get_hw_state':
> drivers/gpu/drm/i915/intel_dpll_mgr.c:1644:1: error: the frame size of 1016 
> bytes is larger than 1000 bytes [-Werror=frame-larger-than=]
> drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'bxt_ddi_pll_enable':
> drivers/gpu/drm/i915/intel_dpll_mgr.c:1548:1: error: the frame size of 1960 
> bytes is larger than 1000 bytes [-Werror=frame-larger-than=]
>
> I also checked the module sizes and got
>
> before:
>text  data bss dec hex filename
> 2704592412025   11104 3127721  2fb9a9 drivers/gpu/drm/i915/i915.o
>
> after:
>text  data bss dec hex filename
> 2718538412025   11104 3141667  2ff023 drivers/gpu/drm/i915/i915.o
>
> showing a significant code size growth. This reverts the patch to avoid
> the warning and get back to the better code with CONFIG_KASAN. If someone
> has another idea to avoid the warning while keeping the more efficient
> code for the non-KASAN case, that would obviously be better.
>
> Fixes: ce64645d86ac ("drm/i915: use variadic macros and arrays to choose 
> port/pipe based registers")
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 04c8f69fcc62..aa732eccdeb5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -48,8 +48,6 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>   return !i915_mmio_reg_equal(reg, INVALID_MMIO_REG);
>  }
>  
> -#define _PICK(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
> -
>  #define _PIPE(pipe, a, b) ((a) + (pipe)*((b)-(a)))
>  #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
>  #define _PLANE(plane, a, b) _PIPE(plane, a, b)
> @@ -58,11 +56,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b))
>  #define _PORT(port, a, b) ((a) + (port)*((b)-(a)))
>  #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
> -#define _PIPE3(pipe, ...) _PICK(pipe, __VA_ARGS__)
> +#define _PIPE3(pipe, a, b, c) ((pipe) == PIPE_A ? (a) : \
> +(pipe) == PIPE_B ? (b) : (c))
>  #define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PIPE3(pipe, a, b, c))
> -#define _PORT3(port, ...) _PICK(port, __VA_ARGS__)
> +#define _PORT3(port, a, b, c) ((port) == PORT_A ? (a) : \
> +(port) == PORT_B ? (b) : (c))
>  #define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PORT3(pipe, a, b, c))
> -#define _PHY3(phy, ...) _PICK(phy, __VA_ARGS__)
> +#define _PHY3(phy, a, b, c) ((phy) == DPIO_PHY0 ? (a) : \
> +  (phy) == DPIO_PHY1 ? (b) : (c))
>  #define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
>  
>  #define _MASKED_FIELD(mask, value) ({
>\

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Skip force-wake for uncached mmio flush of GGTT writes

[Chris Wilson]

On Mon, Mar 20, 2017 at 12:02:02PM +0200, Mika Kuoppala wrote:
> Chris Wilson  writes:
> 
> > The trick of using an uncached mmio read to ensure that the GGTT writes
> > are flushed does not require us to do the forcewake dance, so avoid it
> > in the hope of reducing the frequency that we do keep the device forced
> > awake.
> >
> > Signed-off-by: Chris Wilson 
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c | 7 +--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index 5e280e3bfd86..d468300e2f05 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -3094,8 +3094,11 @@ i915_gem_object_flush_gtt_write_domain(struct 
> > drm_i915_gem_object *obj)
> >  * system agents we cannot reproduce this behaviour).
> >  */
> > wmb();
> > -   if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv))
> > -   POSTING_READ(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
> > +   if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
> > +   spin_lock_irq(_priv->uncore.lock);
> > +   POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
> > +   spin_unlock_irq(_priv->uncore.lock);
> > +   }
> >
> 
> Why is it so that the flushing (from gpu side) doesn't need
> the rc6?

It's not GPU, it's GTT. Just the usual thing with our fake pci not
following pci ordering rules.
-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] drm/atomic: protect crtc|connector->state with rcu

[Maarten Lankhorst]

Op 20-03-17 om 09:59 schreef Daniel Vetter:
> On Mon, Mar 20, 2017 at 09:38:52AM +0100, Maarten Lankhorst wrote:
>> Op 20-03-17 om 09:18 schreef Daniel Vetter:
>>> On Fri, Mar 17, 2017 at 05:52:32PM +0100, Maarten Lankhorst wrote:
 Op 16-03-17 om 21:15 schreef Daniel Vetter:
> On Thu, Mar 16, 2017 at 5:09 PM, Maarten Lankhorst
>  wrote:
>> Op 16-03-17 om 16:52 schreef Daniel Vetter:
>>> The vblank code really wants to look at crtc->state without having to
>>> take a ww_mutex. One option might be to take one of the vblank locks
>>> right when assigning crtc->state, which would ensure that the vblank
>>> code doesn't race and access freed memory.
>> I'm not sure this is the right approach for vblank.
> It's not, it's just that I've had to resurrect this patch quickly
> before leaving and accidentally left the vblank stuff in.
>
>> crtc->state might not be the same as the current state in case of a 
>> nonblocking
>> modeset that hasn't even disabled the old crtc yet.
>>> But userspace tends to poke the vblank_ioctl to query the current
>>> vblank timestamp rather often, and going all in with rcu would help a
>>> bit. We're not there yet, but also doesn't really hurt.
>>>
>>> v2: Maarten needs this to make connector properties atomic, so he can
>>> peek at state from the various ->mode_valid hooks.
>>>
>>> Cc: Maarten Lankhorst 
>>> Signed-off-by: Daniel Vetter 
>>> ---
>>>  drivers/gpu/drm/drm_atomic.c| 26 +-
>>>  drivers/gpu/drm/drm_atomic_helper.c |  2 +-
>>>  include/drm/drm_atomic.h|  5 +
>>>  include/drm/drm_connector.h | 13 -
>>>  include/drm/drm_crtc.h  |  9 -
>>>  5 files changed, 43 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>>> index 9b892af7811a..ba11e31ff9ba 100644
>>> --- a/drivers/gpu/drm/drm_atomic.c
>>> +++ b/drivers/gpu/drm/drm_atomic.c
>>> @@ -213,16 +213,10 @@ void drm_atomic_state_clear(struct 
>>> drm_atomic_state *state)
>>>  }
>>>  EXPORT_SYMBOL(drm_atomic_state_clear);
>>>
>>> -/**
>>> - * __drm_atomic_state_free - free all memory for an atomic state
>>> - * @ref: This atomic state to deallocate
>>> - *
>>> - * This frees all memory associated with an atomic state, including 
>>> all the
>>> - * per-object state for planes, crtcs and connectors.
>>> - */
>>> -void __drm_atomic_state_free(struct kref *ref)
>>> +void ___drm_atomic_state_free(struct rcu_head *rhead)
>>>  {
>>> - struct drm_atomic_state *state = container_of(ref, 
>>> typeof(*state), ref);
>>> + struct drm_atomic_state *state =
>>> + container_of(rhead, typeof(*state), rhead);
>>>   struct drm_mode_config *config = >dev->mode_config;
>>>
>>>   drm_atomic_state_clear(state);
>>> @@ -236,6 +230,20 @@ void __drm_atomic_state_free(struct kref *ref)
>>>   kfree(state);
>>>   }
>>>  }
>> whatisRCU.txt:
>> "This function invokes func(head) after a grace period has elapsed.
>> This invocation might happen from either softirq or process context,
>> so the function is not permitted to block."
>>
>> Looking at
>> commit 6f0f02dc56f18760b46dc1bf5b3f7386869d4162
>> Author: Chris Wilson 
>> Date:   Mon Jan 23 21:29:39 2017 +
>>
>> drm/i915: Move atomic state free from out of fence release
>>
>> I would say that drm_atomic_state_free would definitely block..
>>
>> The only thing that makes sense IMO is doing kfree_rcu on the 
>> object_states.
>> But I don't think RCU is the answer here, it won't protect you against 
>> using
>> the wrong crtc state.
>>
>> I think I would try to use the crtc ww_mutex in the vblank code and 
>> serialize it to pending commits, if at all possible.
> Oops. I guess it should have come with "totally untested". In that
> case we need a workter which does a synchronize_rcu() before
> releasing. I don't just want to do a simple kfree_rcu() because by
> that point we've (partially) destroyed the state alreayd (so it's
> already unsafe to access, and special ruels are ugly). And doing it
> here before we release anything in the core would avoid the need for
> drivers to bother with kfree_rcu().
>
> I'll try to respin something less obviously buggy tomorrow :-)
 It will still be buggy tomorrow, since you have no way to know if the 
 current hardware crtc_state is anything like crtc->state.

 :(
>>> Maybe I wasnt' clear, so let me retry:
>>>
>>> - this approach doesn't work for vblank and crtc state. 

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: allocate mock file pointer dynamically

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: allocate mock file pointer 
dynamically
URL   : https://patchwork.freedesktop.org/series/21533/
State : success

== Summary ==

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

Test gem_exec_suspend:
Subgroup basic-s4-devices:
dmesg-warn -> PASS   (fi-bxt-t5700) fdo#100125

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

fi-bdw-5557u total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  
time: 463s
fi-bsw-n3050 total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  
time: 573s
fi-bxt-j4205 total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  
time: 534s
fi-bxt-t5700 total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  
time: 559s
fi-byt-j1900 total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  
time: 499s
fi-byt-n2820 total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  
time: 496s
fi-hsw-4770  total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time: 434s
fi-hsw-4770r total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time: 434s
fi-ilk-650   total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  
time: 433s
fi-ivb-3520m total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 515s
fi-ivb-3770  total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 495s
fi-kbl-7500u total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 478s
fi-skl-6260u total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time: 480s
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: 477s
fi-skl-6770hqtotal:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time: 519s
fi-snb-2520m total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time: 553s
fi-snb-2600  total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  
time: 421s

6aefe8d3baa2d94246dd20ba33c77758b6de9ccc drm-tip: 2017y-03m-20d-07h-21m-17s UTC 
integration manifest
7c8d5c6 Revert "drm/i915: use variadic macros and arrays to choose port/pipe 
based registers"
5530a66 drm/i915: split out check for noncontiguous pfn range
5b90d89 drm/i915: allocate mock file pointer dynamically

== Logs ==

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

Re: [Intel-gfx] [PATCH] drm/i915: Skip force-wake for uncached mmio flush of GGTT writes

[Mika Kuoppala]

Chris Wilson  writes:

> The trick of using an uncached mmio read to ensure that the GGTT writes
> are flushed does not require us to do the forcewake dance, so avoid it
> in the hope of reducing the frequency that we do keep the device forced
> awake.
>
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 5e280e3bfd86..d468300e2f05 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3094,8 +3094,11 @@ i915_gem_object_flush_gtt_write_domain(struct 
> drm_i915_gem_object *obj)
>* system agents we cannot reproduce this behaviour).
>*/
>   wmb();
> - if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv))
> - POSTING_READ(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
> + if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
> + spin_lock_irq(_priv->uncore.lock);
> + POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
> + spin_unlock_irq(_priv->uncore.lock);
> + }
>

Why is it so that the flushing (from gpu side) doesn't need
the rc6?

Also noticed that write domain check, prior to calling this,
in set_to_cpu_domain() is redundant.

-Mika

>   intel_fb_obj_flush(obj, write_origin(obj, I915_GEM_DOMAIN_GTT));
>  
> -- 
> 2.11.0
>
> ___
> 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/uc: Make intel_uc_prepare_fw() static

[Joonas Lahtinen]

On ke, 2017-03-15 at 19:38 +, Michal Wajdeczko wrote:
> There is no need to expose this function as it is called from
> one function only. Also move it up to avoid forward declaration.
> 
> Signed-off-by: Michal Wajdeczko 
> Cc: Arkadiusz Hiler 
> Cc: Joonas Lahtinen 

+ Adding CCs

Looks good to me, 

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/3] drm/i915: allocate mock file pointer dynamically

[Daniel Vetter]

On Mon, Mar 20, 2017 at 10:40:27AM +0100, Arnd Bergmann wrote:
> A struct file is a bit too large to put on the kernel stack in general
> and triggers a warning for low settings of CONFIG_FRAME_WARN:
> 
> drivers/gpu/drm/i915/selftests/mock_drm.c: In function 'mock_file':
> drivers/gpu/drm/i915/selftests/mock_drm.c:46:1: error: the frame size of 1328 
> bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
> drivers/gpu/drm/i915/selftests/mock_drm.c: In function 'mock_file_free':
> drivers/gpu/drm/i915/selftests/mock_drm.c:54:1: error: the frame size of 1312 
> bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
> 
> It's also slightly dangerous to leave a reference to a stack object
> in the drm_file structure after leaving the stack frame.
> This changes the code to just allocate the object dynamically
> and release it when we are done with it.
> 
> Fixes: 66d9cb5d805a ("drm/i915: Mock the GEM device for self-testing")
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/gpu/drm/i915/selftests/mock_drm.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/mock_drm.c 
> b/drivers/gpu/drm/i915/selftests/mock_drm.c
> index 113dec05c7dc..18514065c93d 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_drm.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_drm.c
> @@ -32,15 +32,15 @@ static inline struct inode fake_inode(struct 
> drm_i915_private *i915)
>  struct drm_file *mock_file(struct drm_i915_private *i915)
>  {
>   struct inode inode = fake_inode(i915);
> - struct file filp = {};
> + struct file *filp = kzalloc(sizeof(struct file), GFP_KERNEL);
>   struct drm_file *file;
>   int err;
>  
> - err = drm_open(, );
> + err = drm_open(, filp);

Aside: We should have a FIXME and/or merge David Herrmanns code to allow a
drm_file without a real struct file for kernel-internal use ...
-Daniel

>   if (unlikely(err))
>   return ERR_PTR(err);
>  
> - file = filp.private_data;
> + file = filp->private_data;
>   file->authenticated = true;
>   return file;
>  }
> @@ -48,7 +48,9 @@ struct drm_file *mock_file(struct drm_i915_private *i915)
>  void mock_file_free(struct drm_i915_private *i915, struct drm_file *file)
>  {
>   struct inode inode = fake_inode(i915);
> - struct file filp = { .private_data = file };
> + struct file *filp = file->filp;
>  
> - drm_release(, );
> + drm_release(, filp);
> +
> + kfree(filp);
>  }
> -- 
> 2.9.0
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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] [PATCH 1/7] lib/igt_fb: Let others use igt_get_fb_tile_size

[Daniel Vetter]

On Sat, Mar 18, 2017 at 12:45:20AM +0530, Praveen Paneri wrote:
> This function can be used by igt_draw to get accurate
> tile dimensions for all tile formats.
> 
> Signed-off-by: Praveen Paneri 
> ---
>  lib/igt_fb.c | 2 +-
>  lib/igt_fb.h | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index d2b7e9e..6ecc36b 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -74,7 +74,7 @@ static struct format_desc_struct {
>  #define for_each_format(f)   \
>   for (f = format_desc; f - format_desc < ARRAY_SIZE(format_desc); f++)
>  
> -static void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
> +void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
>unsigned *width_ret, unsigned *height_ret)

Documentation seems missing here.
-Daniel

>  {
>   switch (tiling) {
> diff --git a/lib/igt_fb.h b/lib/igt_fb.h
> index 4a680ce..b178eba 100644
> --- a/lib/igt_fb.h
> +++ b/lib/igt_fb.h
> @@ -94,7 +94,8 @@ enum igt_text_align {
>   align_vcenter   = 0x04,
>   align_hcenter   = 0x08,
>  };
> -
> +void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
> + unsigned *width_ret, unsigned *height_ret);
>  void igt_calc_fb_size(int fd, int width, int height, int bpp, uint64_t 
> tiling,
> unsigned *size_ret, unsigned *stride_ret);
>  unsigned int
> -- 
> 2.7.4
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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

[Intel-gfx] [PATCH 3/3] [RFC] Revert "drm/i915: use variadic macros and arrays to choose port/pipe based registers"

[Arnd Bergmann]

The varargs macro trick in _PIPE3/_PHY3/_PORT3 was meant as an optimization
to shrink the i915 kernel module by around 1000 bytes. However, the
downside is a size regression with CONFIG_KASAN, as I found from stack size
warnings with gcc-7.0.1:

before:
drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'bxt_ddi_pll_get_hw_state':
drivers/gpu/drm/i915/intel_dpll_mgr.c:1644:1: error: the frame size of 176 
bytes is larger than 100 bytes [-Werror=frame-larger-than=]
drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'bxt_ddi_pll_enable':
drivers/gpu/drm/i915/intel_dpll_mgr.c:1548:1: error: the frame size of 224 
bytes is larger than 100 bytes [-Werror=frame-larger-than=]

after:
drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'bxt_ddi_pll_get_hw_state':
drivers/gpu/drm/i915/intel_dpll_mgr.c:1644:1: error: the frame size of 1016 
bytes is larger than 1000 bytes [-Werror=frame-larger-than=]
drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'bxt_ddi_pll_enable':
drivers/gpu/drm/i915/intel_dpll_mgr.c:1548:1: error: the frame size of 1960 
bytes is larger than 1000 bytes [-Werror=frame-larger-than=]

I also checked the module sizes and got

before:
   textdata bss dec hex filename
2704592  412025   11104 3127721  2fb9a9 drivers/gpu/drm/i915/i915.o

after:
   textdata bss dec hex filename
2718538  412025   11104 3141667  2ff023 drivers/gpu/drm/i915/i915.o

showing a significant code size growth. This reverts the patch to avoid
the warning and get back to the better code with CONFIG_KASAN. If someone
has another idea to avoid the warning while keeping the more efficient
code for the non-KASAN case, that would obviously be better.

Fixes: ce64645d86ac ("drm/i915: use variadic macros and arrays to choose 
port/pipe based registers")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/i915/i915_reg.h | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 04c8f69fcc62..aa732eccdeb5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -48,8 +48,6 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
return !i915_mmio_reg_equal(reg, INVALID_MMIO_REG);
 }
 
-#define _PICK(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index])
-
 #define _PIPE(pipe, a, b) ((a) + (pipe)*((b)-(a)))
 #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
 #define _PLANE(plane, a, b) _PIPE(plane, a, b)
@@ -58,11 +56,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b))
 #define _PORT(port, a, b) ((a) + (port)*((b)-(a)))
 #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
-#define _PIPE3(pipe, ...) _PICK(pipe, __VA_ARGS__)
+#define _PIPE3(pipe, a, b, c) ((pipe) == PIPE_A ? (a) : \
+  (pipe) == PIPE_B ? (b) : (c))
 #define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PIPE3(pipe, a, b, c))
-#define _PORT3(port, ...) _PICK(port, __VA_ARGS__)
+#define _PORT3(port, a, b, c) ((port) == PORT_A ? (a) : \
+  (port) == PORT_B ? (b) : (c))
 #define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PORT3(pipe, a, b, c))
-#define _PHY3(phy, ...) _PICK(phy, __VA_ARGS__)
+#define _PHY3(phy, a, b, c) ((phy) == DPIO_PHY0 ? (a) : \
+(phy) == DPIO_PHY1 ? (b) : (c))
 #define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c))
 
 #define _MASKED_FIELD(mask, value) ({ \
-- 
2.9.0

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

[Intel-gfx] [PATCH 2/3] drm/i915: split out check for noncontiguous pfn range

[Arnd Bergmann]

We get a warning with gcc-7 about a pointless comparison when
using a linear memmap:

drivers/gpu/drm/i915/selftests/scatterlist.c: In function 'alloc_table':
drivers/gpu/drm/i915/selftests/scatterlist.c:219:66: error: self-comparison 
always evaluates to false [-Werror=tautological-compare]

Splitting out the comparison into a separate function avoids the warning
and makes it slightly more obvious what happens.

Fixes: 935a2f776aa5 ("drm/i915: Add some selftests for sg_table manipulation")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/i915/selftests/scatterlist.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/selftests/scatterlist.c 
b/drivers/gpu/drm/i915/selftests/scatterlist.c
index eb2cda8e2b9f..c072b0f9180b 100644
--- a/drivers/gpu/drm/i915/selftests/scatterlist.c
+++ b/drivers/gpu/drm/i915/selftests/scatterlist.c
@@ -189,6 +189,11 @@ static unsigned int random(unsigned long n,
return 1 + (prandom_u32_state(rnd) % 1024);
 }
 
+static inline bool page_contiguous(struct page *first, struct page *last,  
unsigned long npages)
+{
+   return first + npages == last;
+}
+
 static int alloc_table(struct pfn_table *pt,
   unsigned long count, unsigned long max,
   npages_fn_t npages_fn,
@@ -216,7 +221,8 @@ static int alloc_table(struct pfn_table *pt,
unsigned long npages = npages_fn(n, count, rnd);
 
/* Nobody expects the Sparse Memmap! */
-   if (pfn_to_page(pfn + npages) != pfn_to_page(pfn) + npages) {
+   if (!page_contiguous(pfn_to_page(pfn),
+pfn_to_page(pfn + npages), npages)) {
sg_free_table(>st);
return -ENOSPC;
}
-- 
2.9.0

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

[Intel-gfx] [PATCH 1/3] drm/i915: allocate mock file pointer dynamically

[Arnd Bergmann]

A struct file is a bit too large to put on the kernel stack in general
and triggers a warning for low settings of CONFIG_FRAME_WARN:

drivers/gpu/drm/i915/selftests/mock_drm.c: In function 'mock_file':
drivers/gpu/drm/i915/selftests/mock_drm.c:46:1: error: the frame size of 1328 
bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
drivers/gpu/drm/i915/selftests/mock_drm.c: In function 'mock_file_free':
drivers/gpu/drm/i915/selftests/mock_drm.c:54:1: error: the frame size of 1312 
bytes is larger than 1280 bytes [-Werror=frame-larger-than=]

It's also slightly dangerous to leave a reference to a stack object
in the drm_file structure after leaving the stack frame.
This changes the code to just allocate the object dynamically
and release it when we are done with it.

Fixes: 66d9cb5d805a ("drm/i915: Mock the GEM device for self-testing")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/i915/selftests/mock_drm.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/mock_drm.c 
b/drivers/gpu/drm/i915/selftests/mock_drm.c
index 113dec05c7dc..18514065c93d 100644
--- a/drivers/gpu/drm/i915/selftests/mock_drm.c
+++ b/drivers/gpu/drm/i915/selftests/mock_drm.c
@@ -32,15 +32,15 @@ static inline struct inode fake_inode(struct 
drm_i915_private *i915)
 struct drm_file *mock_file(struct drm_i915_private *i915)
 {
struct inode inode = fake_inode(i915);
-   struct file filp = {};
+   struct file *filp = kzalloc(sizeof(struct file), GFP_KERNEL);
struct drm_file *file;
int err;
 
-   err = drm_open(, );
+   err = drm_open(, filp);
if (unlikely(err))
return ERR_PTR(err);
 
-   file = filp.private_data;
+   file = filp->private_data;
file->authenticated = true;
return file;
 }
@@ -48,7 +48,9 @@ struct drm_file *mock_file(struct drm_i915_private *i915)
 void mock_file_free(struct drm_i915_private *i915, struct drm_file *file)
 {
struct inode inode = fake_inode(i915);
-   struct file filp = { .private_data = file };
+   struct file *filp = file->filp;
 
-   drm_release(, );
+   drm_release(, filp);
+
+   kfree(filp);
 }
-- 
2.9.0

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

[Intel-gfx] i915 (ivy bridge) + 4.10.3 + gimp = BUG in list_move_tail()

[Jérôme Carretero]

Hi,


After a kernel update from v4.9.10 to v4.10.3, any time I bring out the gimp,
the i915 driver NULL-pointer dereferences something in list_move_tail(),
somewhere in i915_gem_evict_for_vma().

I'm providing the kernel log, if more is needed (say you aren't
aware of this regression) I'm available.

xf-86-video-intel is 860c3664fe79c1fe92095ff345068f1fc7e4e651,
mesa is 11.2.1, xorg-server is 1.19.2, but I don't think it matters.


Regards,

-- 
Jérôme


Mar 19 17:32:11 Vantage kernel: BUG: unable to handle kernel NULL pointer 
dereference at 0088
Mar 19 17:32:11 Vantage kernel: IP: list_move_tail+0xb/0x26
Mar 19 17:32:11 Vantage kernel: PGD 1641b8067 
Mar 19 17:32:11 Vantage kernel: PUD 1506ba067 
Mar 19 17:32:11 Vantage kernel: PMD 0 
Mar 19 17:32:11 Vantage kernel: 
Mar 19 17:32:11 Vantage kernel: Oops: 0002 [#1] PREEMPT SMP
Mar 19 17:32:11 Vantage kernel: Modules linked in: ccm fuse bnep hid_generic 
iTCO_wdt iTCO_vendor_support coretemp intel_rapl iosf_mbi x86_pkg_temp_thermal 
kvm_intel btusb btrtl kvm btbcm iwldvm btintel mac80211 irqbypass dm_mod 
aesni_intel uvcvideo snd_hda_codec_hdmi aes_x86_64 crypto_simd cryptd 
videobuf2_vmalloc g
Mar 19 17:32:11 Vantage kernel: CPU: 2 PID: 5559 Comm: gimp Not tainted 
4.10.3-Vantage-dirty #107
Mar 19 17:32:11 Vantage kernel: Hardware name: LENOVO 2349L64/2349L64, BIOS 
G1ETA5WW (2.65 ) 04/15/2014
Mar 19 17:32:11 Vantage kernel: task: 880171af5400 task.stack: 
c9000a784000
Mar 19 17:32:11 Vantage kernel: RIP: 0010:list_move_tail+0xb/0x26
Mar 19 17:32:11 Vantage kernel: RSP: 0018:c9000a787ac8 EFLAGS: 00010296
Mar 19 17:32:11 Vantage kernel: RAX: 88040b67be60 RBX: 88040b67bcc8 
RCX: 88040c38e620
Mar 19 17:32:11 Vantage kernel: RDX: 0080 RSI: 88040be2df68 
RDI: 88040b67be58
Mar 19 17:32:11 Vantage kernel: RBP: c9000a787ac8 R08: 880171af5400 
R09: 
Mar 19 17:32:11 Vantage kernel: R10:  R11: 7fff 
R12: 88040be2dc10
Mar 19 17:32:11 Vantage kernel: R13:  R14:  
R15: 88016515
Mar 19 17:32:11 Vantage kernel: FS:  7f0e85a28d40() 
GS:88041e28() knlGS:
Mar 19 17:32:11 Vantage kernel: CS:  0010 DS:  ES:  CR0: 
80050033
Mar 19 17:32:11 Vantage kernel: CR2: 0088 CR3: 00015acf5000 
CR4: 001406e0
Mar 19 17:32:11 Vantage kernel: Call Trace:
Mar 19 17:32:11 Vantage kernel:  i915_vma_unbind+0x1d0/0x274
Mar 19 17:32:11 Vantage kernel:  i915_gem_evict_for_vma+0x7d/0x91
Mar 19 17:32:11 Vantage kernel:  __i915_vma_do_pin+0x226/0x376
Mar 19 17:32:11 Vantage kernel:  
i915_gem_execbuffer_reserve_vma.isra.26+0xbc/0x189
Mar 19 17:32:11 Vantage kernel:  i915_gem_execbuffer_reserve.isra.27+0x2ac/0x339
Mar 19 17:32:11 Vantage kernel:  i915_gem_do_execbuffer.isra.32+0x62a/0x1200
Mar 19 17:32:11 Vantage kernel:  ? __radix_tree_lookup+0x2b/0x86
Mar 19 17:32:11 Vantage kernel:  ? find_lock_entry+0x36/0x57
Mar 19 17:32:11 Vantage kernel:  ? balance_dirty_pages_ratelimited+0x1c/0x9a9
Mar 19 17:32:11 Vantage kernel:  ? PageUptodate+0x9/0x17
Mar 19 17:32:11 Vantage kernel:  ? shmem_getpage_gfp+0x11f/0x763
Mar 19 17:32:11 Vantage kernel:  i915_gem_execbuffer2+0x132/0x1b4
Mar 19 17:32:11 Vantage kernel:  drm_ioctl+0x242/0x34a
Mar 19 17:32:11 Vantage kernel:  ? i915_gem_execbuffer+0x229/0x229
Mar 19 17:32:11 Vantage kernel:  ? handle_mm_fault+0x8b5/0xb97
Mar 19 17:32:11 Vantage kernel:  ? vma_merge+0x285/0x2aa
Mar 19 17:32:11 Vantage kernel:  vfs_ioctl+0x13/0x2f
Mar 19 17:32:11 Vantage kernel:  do_vfs_ioctl+0x49c/0x50a
Mar 19 17:32:11 Vantage kernel:  ? recalc_sigpending+0x31/0x41
Mar 19 17:32:11 Vantage kernel:  ? __fget+0x66/0x72
Mar 19 17:32:11 Vantage kernel:  SyS_ioctl+0x52/0x74
Mar 19 17:32:11 Vantage kernel:  entry_SYSCALL_64_fastpath+0x1a/0xa9
Mar 19 17:32:11 Vantage kernel: RIP: 0033:0x7f0e81144107
Mar 19 17:32:11 Vantage kernel: RSP: 002b:7fffe7e180b8 EFLAGS: 0246 
ORIG_RAX: 0010
Mar 19 17:32:11 Vantage kernel: RAX: ffda RBX: 016b3710 
RCX: 7f0e81144107
Mar 19 17:32:11 Vantage kernel: RDX: 7fffe7e18108 RSI: 40406469 
RDI: 000a
Mar 19 17:32:11 Vantage kernel: RBP: 7fffe7e182a0 R08:  
R09: 
Mar 19 17:32:11 Vantage kernel: R10:  R11: 0246 
R12: 7fffe7e180c0
Mar 19 17:32:11 Vantage kernel: R13: 0001 R14: 00a0 
R15: 012dbf10
Mar 19 17:32:11 Vantage kernel: Code: 87 78 10 00 00 00 00 00 00 48 8b 48 08 ff 
70 20 4c 8b 48 18 44 8b 40 10 e8 67 76 c2 ff 5a c9 c3 48 8b 47 08 55 48 8b 17 
48 89 e5 <48> 89 42 08 48 89 10 48 8b 46 08 48 89 7e 08 48 89 37 48 89 47 
Mar 19 17:32:11 Vantage kernel: RIP: list_move_tail+0xb/0x26 RSP: 
c9000a787ac8
Mar 19 17:32:11 Vantage kernel: CR2: 0088
Mar 19 17:32:11 Vantage kernel: ---[ end trace b20a3798f5da98ce ]---
___

Re: [Intel-gfx] [PATCH 2/2] drm/i915/vbt: split out defaults that are set when there is no VBT

[Jani Nikula]

On Fri, 17 Mar 2017, Paulo Zanoni  wrote:
> Em Ter, 2017-03-14 às 11:09 +0200, Jani Nikula escreveu:
>> On Mon, 13 Mar 2017, Manasi Navare  wrote:
>> > 
>> > On Mon, Mar 13, 2017 at 11:55:31AM +0200, Jani Nikula wrote:
>> > > 
>> > > On Sat, 11 Mar 2017, Manasi Navare 
>> > > wrote:
>> > > > 
>> > > > On Fri, Mar 10, 2017 at 03:27:58PM +0200, Jani Nikula wrote:
>> > > > > 
>> > > > > The main thing are the DDI ports. If there's a VBT that says
>> > > > > there are
>> > > > > no outputs, we should trust that, and not have semi-random
>> > > > > defaults. Unfortunately, the defaults have resulted in some
>> > > > > Chromebooks
>> > > > > without VBT to rely on this behaviour, so we split out the
>> > > > > defaults for
>> > > > > the missing VBT case.
>> > > > > 
>> > > > > Cc: Manasi Navare 
>> > > > > Cc: Ville Syrjälä 
>> > > > > Signed-off-by: Jani Nikula 
>> > > > > ---
>> > > > >  drivers/gpu/drm/i915/intel_bios.c | 17 -
>> > > > >  1 file changed, 16 insertions(+), 1 deletion(-)
>> > > > > 
>> > > > > diff --git a/drivers/gpu/drm/i915/intel_bios.c
>> > > > > b/drivers/gpu/drm/i915/intel_bios.c
>> > > > > index 710988d72253..639d45c1dd2e 100644
>> > > > > --- a/drivers/gpu/drm/i915/intel_bios.c
>> > > > > +++ b/drivers/gpu/drm/i915/intel_bios.c
>> > > > > @@ -1341,6 +1341,7 @@ parse_device_mapping(struct
>> > > > > drm_i915_private *dev_priv,
>> > > > >  return;
>> > > > >  }
>> > > > >  
>> > > > > +/* Common defaults which may be overridden by VBT. */
>> > > > >  static void
>> > > > >  init_vbt_defaults(struct drm_i915_private *dev_priv)
>> > > > >  {
>> > > > > @@ -1377,6 +1378,18 @@ init_vbt_defaults(struct
>> > > > > drm_i915_private *dev_priv)
>> > > > >  _priv->vbt.ddi_port_info[port];
>> > > > >  
>> > > > >  info->hdmi_level_shift =
>> > > > > HDMI_LEVEL_SHIFT_UNKNOWN;
>> > > > > +}
>> > > > > +}
>> > > > > +
>> > > > > +/* Defaults to initialize only if there is no VBT. */
>> > > > > +static void
>> > > > > +init_vbt_missing_defaults(struct drm_i915_private *dev_priv)
>> > > > > +{
>> > > > > +enum port port;
>> > > > > +
>> > > > > +for (port = PORT_A; port < I915_MAX_PORTS; port++) {
>> > > > > +struct ddi_vbt_port_info *info =
>> > > > > +_priv->vbt.ddi_port_info[port];
>> > > > >  
>> > > > >  info->supports_dvi = (port != PORT_A && port
>> > > > > != PORT_E);
>> > > > >  info->supports_hdmi = info->supports_dvi;
>> > > > > @@ -1516,8 +1529,10 @@ void intel_bios_init(struct
>> > > > > drm_i915_private *dev_priv)
>> > > > >  parse_ddi_ports(dev_priv, bdb);
>> > > > >  
>> > > > >  out:
>> > > > > -if (!vbt)
>> > > > > +if (!vbt) {
>> > > > >  DRM_INFO("Failed to find VBIOS tables
>> > > > > (VBT)\n");
>> > > > > +init_vbt_missing_defaults(dev_priv);
>> > > > > +}
>> > > > 
>> > > > So in case there is no VBT, this will set supports_DP flag on
>> > > > Port A.
>> > > > What is there is no VBT and there is no eDP on Port A?
>> > > > In this case it will still try to link train on Port A and
>> > > > fail..?
>> > > > I am not sure if this case exists, but just a thought looking
>> > > > at it.
>> > > 
>> > > It's possible the case exists, but the point is that the
>> > > behaviour for
>> > > the no-VBT case remains the same before and after this patch.
>> > > 
>> > > BR,
>> > > Jani.
>> > > 
>> > > 
>> > 
>> > Ok agreed. In that case Reviewed-by: Manasi Navare
>> > 
>> 
>> Pushed to drm-intel-next-queued, thanks for the review.
>> 
>> I really hope there are no machines out there that have a crippled
>> VBT
>> with no child device config. I guess we'll find out...
>
> I have access to this very interesting machine with DDB version 163 and
> a child device size config that's 1 instead of the expected 33.
>
> So what happens here is that since the VBT is supposed to be valid we
> don't end up calling init_vbt_missing_defauilts(). We return early from
> parse_device_mapping(), which means we don't set vbt.child_dev_num,
> which means that parse_ddi_port() returns early. So info->supports_*
> stays false, and intel_ddi_init() fails.
>
> Given your commit message it seems that we should properly be able to
> distinguish between "VBT correctly says that there's no output" and
> "VBT is drunk and should go home" in order to fix this problem.

I'm not sure it's possible to distinguish between the two. I thought
we'd be able to rely on the former. If we have to change
"init_vbt_missing_defaults" to "init_child_dev_missing_defaults", then
we'll never be able to handle the case where vbt correctly states there
are no child devices. :(

BR,
Jani.


> I can confirm that reverting this patch makes display great again^w^w
> work again. So unfortunately I'll have 

Re: [Intel-gfx] [i-g-t 1/1] tests/gem_gtt_hog: Clear the parameters for GEM_CREATE ioctl

[Kamble, Sagar A]

On 3/20/2017 2:25 PM, Chris Wilson wrote:

On Mon, Mar 20, 2017 at 02:16:54PM +0530, Kamble, Sagar A wrote:


On 3/20/2017 1:11 PM, Chris Wilson wrote:

On Mon, Mar 20, 2017 at 11:32:15AM +0530, Sagar Arun Kamble wrote:

Due to garbage data seen by i915, gem_create_ioctl failed for gem obj
created with drmIoctl(GEM_CREATE) without properly initialized
parameters. Can be fixed by calling gem_create helper too.

Whose bug are you working around?
-Chris

Found out now that this is happening due to mismatch in the libdrm headers 
32bit/64bit flags for GEM_CREATE.
Able to resolve by properly using the proper definitions.
Will this fix be still applicable as flags would stay uninitialized with 
current call to drmIoctl?

At the moment, create.pad is never tested ergo it is valid to contain
garbage and as demonstrated that behaviour is already relied upon by
userspace.

How did the headers vary? The defintion is
struct drm_i915_gem_create {
 /**
  * Requested size for the object.
  *
  * The (page-aligned) allocated size for the object will be returned.
  */
 __u64 size;
 /**
  * Returned handle for the object.
  *
  * Object handles are nonzero.
  */
 __u32 handle;
 __u32 pad;
};
which should be 32/64bit safe. Otherwise we need a compat ioctl for the
same reason as not breaking 32bit userspace on 64bit kernel.
-Chris


This behavior is with internal Android kernel where gem_create has extra member as 
"__u64 flags"
added for stolen objects.
With pad value exception and size, handle set properly this patch does not 
apply then.





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

Re: [Intel-gfx] [PATCH] drm/atomic: protect crtc|connector->state with rcu

[Daniel Vetter]

On Mon, Mar 20, 2017 at 09:38:52AM +0100, Maarten Lankhorst wrote:
> Op 20-03-17 om 09:18 schreef Daniel Vetter:
> > On Fri, Mar 17, 2017 at 05:52:32PM +0100, Maarten Lankhorst wrote:
> >> Op 16-03-17 om 21:15 schreef Daniel Vetter:
> >>> On Thu, Mar 16, 2017 at 5:09 PM, Maarten Lankhorst
> >>>  wrote:
>  Op 16-03-17 om 16:52 schreef Daniel Vetter:
> > The vblank code really wants to look at crtc->state without having to
> > take a ww_mutex. One option might be to take one of the vblank locks
> > right when assigning crtc->state, which would ensure that the vblank
> > code doesn't race and access freed memory.
>  I'm not sure this is the right approach for vblank.
> >>> It's not, it's just that I've had to resurrect this patch quickly
> >>> before leaving and accidentally left the vblank stuff in.
> >>>
>  crtc->state might not be the same as the current state in case of a 
>  nonblocking
>  modeset that hasn't even disabled the old crtc yet.
> > But userspace tends to poke the vblank_ioctl to query the current
> > vblank timestamp rather often, and going all in with rcu would help a
> > bit. We're not there yet, but also doesn't really hurt.
> >
> > v2: Maarten needs this to make connector properties atomic, so he can
> > peek at state from the various ->mode_valid hooks.
> >
> > Cc: Maarten Lankhorst 
> > Signed-off-by: Daniel Vetter 
> > ---
> >  drivers/gpu/drm/drm_atomic.c| 26 +-
> >  drivers/gpu/drm/drm_atomic_helper.c |  2 +-
> >  include/drm/drm_atomic.h|  5 +
> >  include/drm/drm_connector.h | 13 -
> >  include/drm/drm_crtc.h  |  9 -
> >  5 files changed, 43 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 9b892af7811a..ba11e31ff9ba 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -213,16 +213,10 @@ void drm_atomic_state_clear(struct 
> > drm_atomic_state *state)
> >  }
> >  EXPORT_SYMBOL(drm_atomic_state_clear);
> >
> > -/**
> > - * __drm_atomic_state_free - free all memory for an atomic state
> > - * @ref: This atomic state to deallocate
> > - *
> > - * This frees all memory associated with an atomic state, including 
> > all the
> > - * per-object state for planes, crtcs and connectors.
> > - */
> > -void __drm_atomic_state_free(struct kref *ref)
> > +void ___drm_atomic_state_free(struct rcu_head *rhead)
> >  {
> > - struct drm_atomic_state *state = container_of(ref, 
> > typeof(*state), ref);
> > + struct drm_atomic_state *state =
> > + container_of(rhead, typeof(*state), rhead);
> >   struct drm_mode_config *config = >dev->mode_config;
> >
> >   drm_atomic_state_clear(state);
> > @@ -236,6 +230,20 @@ void __drm_atomic_state_free(struct kref *ref)
> >   kfree(state);
> >   }
> >  }
>  whatisRCU.txt:
>  "This function invokes func(head) after a grace period has elapsed.
>  This invocation might happen from either softirq or process context,
>  so the function is not permitted to block."
> 
>  Looking at
>  commit 6f0f02dc56f18760b46dc1bf5b3f7386869d4162
>  Author: Chris Wilson 
>  Date:   Mon Jan 23 21:29:39 2017 +
> 
>  drm/i915: Move atomic state free from out of fence release
> 
>  I would say that drm_atomic_state_free would definitely block..
> 
>  The only thing that makes sense IMO is doing kfree_rcu on the 
>  object_states.
>  But I don't think RCU is the answer here, it won't protect you against 
>  using
>  the wrong crtc state.
> 
>  I think I would try to use the crtc ww_mutex in the vblank code and 
>  serialize it to pending commits, if at all possible.
> >>> Oops. I guess it should have come with "totally untested". In that
> >>> case we need a workter which does a synchronize_rcu() before
> >>> releasing. I don't just want to do a simple kfree_rcu() because by
> >>> that point we've (partially) destroyed the state alreayd (so it's
> >>> already unsafe to access, and special ruels are ugly). And doing it
> >>> here before we release anything in the core would avoid the need for
> >>> drivers to bother with kfree_rcu().
> >>>
> >>> I'll try to respin something less obviously buggy tomorrow :-)
> >> It will still be buggy tomorrow, since you have no way to know if the 
> >> current hardware crtc_state is anything like crtc->state.
> >>
> >> :(
> > Maybe I wasnt' clear, so let me retry:
> >
> > - this approach doesn't work for vblank and crtc state. Agreed. I'll
> >   remove all the leftover comments 

Re: [Intel-gfx] [i-g-t 1/1] tests/gem_gtt_hog: Clear the parameters for GEM_CREATE ioctl

[Chris Wilson]

On Mon, Mar 20, 2017 at 02:16:54PM +0530, Kamble, Sagar A wrote:
> 
> 
> On 3/20/2017 1:11 PM, Chris Wilson wrote:
> >On Mon, Mar 20, 2017 at 11:32:15AM +0530, Sagar Arun Kamble wrote:
> >>Due to garbage data seen by i915, gem_create_ioctl failed for gem obj
> >>created with drmIoctl(GEM_CREATE) without properly initialized
> >>parameters. Can be fixed by calling gem_create helper too.
> >Whose bug are you working around?
> >-Chris
> 
> Found out now that this is happening due to mismatch in the libdrm headers 
> 32bit/64bit flags for GEM_CREATE.
> Able to resolve by properly using the proper definitions.
> Will this fix be still applicable as flags would stay uninitialized with 
> current call to drmIoctl?

At the moment, create.pad is never tested ergo it is valid to contain
garbage and as demonstrated that behaviour is already relied upon by
userspace.

How did the headers vary? The defintion is
struct drm_i915_gem_create {
/**
 * Requested size for the object.
 *
 * The (page-aligned) allocated size for the object will be returned.
 */
__u64 size;
/**
 * Returned handle for the object.
 *
 * Object handles are nonzero.
 */
__u32 handle;
__u32 pad;
};
which should be 32/64bit safe. Otherwise we need a compat ioctl for the
same reason as not breaking 32bit userspace on 64bit kernel.
-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 i-g-t v2] tests/kms_atomic: test that TEST_ONLY does not clobber state

[Maarten Lankhorst]

Op 09-03-17 om 10:19 schreef Mika Kahola:
> We need to make sure that TEST_ONLY really only touches the free-standing
> state objects and nothing else. Test approach here is the following:
>
> - Create a config and submit it with TEST_ONLY.
> - do dpms off/on cycle with the current config to reconfigure hw
> - read back all legacy state to make sure none of that is clobbered
>
> v2: use ATOMIC_RELAX_NONE instead of CRTC_RELAX_MODE when checking crtc or
> plane state (Maarten)
> rename subtest and function that executes this test (Maarten)
>
> Signed-off-by: Mika Kahola 
> ---
>  tests/kms_atomic.c | 82 
> ++
>  1 file changed, 82 insertions(+)
>
> diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
> index d6273f4..6375fed 100644
> --- a/tests/kms_atomic.c
> +++ b/tests/kms_atomic.c
> @@ -831,6 +831,25 @@ static uint32_t plane_get_igt_format(struct 
> kms_atomic_plane_state *plane)
>   return ret;
>  }
>  
> +static void
> +set_dpms(int fd, int mode)
> +{
> + int i;
> + drmModeConnector *connector;
> + uint32_t id;
> + drmModeRes *resources = drmModeGetResources(fd);
> +
> + for (i = 0; i < resources->count_connectors; i++) {
> + id = resources->connectors[i];
> +
> + connector = drmModeGetConnectorCurrent(fd, id);
> +
> + kmstest_set_connector_dpms(fd, connector, mode);
> +
> + drmModeFreeConnector(connector);
> + }
> +}
> +
>  static void plane_overlay(struct kms_atomic_crtc_state *crtc,
> struct kms_atomic_plane_state *plane_old)
>  {
> @@ -930,6 +949,57 @@ static void plane_primary(struct kms_atomic_crtc_state 
> *crtc,
>   drmModeAtomicFree(req);
>  }
>  
> +/* test to ensure that DRM_MODE_ATOMIC_TEST_ONLY really only touches the
> + * free-standing state objects and nothing else.
> + */
> +static void test_only(struct kms_atomic_crtc_state *crtc,
> +   struct kms_atomic_plane_state *plane_old)
> +{
> + struct drm_mode_modeinfo *mode = crtc->mode.data;
> + struct kms_atomic_plane_state plane = *plane_old;
> + uint32_t format = plane_get_igt_format();
> + drmModeAtomicReq *req = drmModeAtomicAlloc();
> + struct igt_fb fb;
> + int ret;
> +
> + igt_require(format != 0);
> +
> + plane.src_x = 0;
> + plane.src_y = 0;
> + plane.src_w = mode->hdisplay << 16;
> + plane.src_h = mode->vdisplay << 16;
> + plane.crtc_x = 0;
> + plane.crtc_y = 0;
> + plane.crtc_w = mode->hdisplay;
> + plane.crtc_h = mode->vdisplay;
> + plane.crtc_id = crtc->obj;
> + plane.fb_id = igt_create_pattern_fb(plane.state->desc->fd,
> + plane.crtc_w, plane.crtc_h,
> + format, I915_TILING_NONE, );
> +
> + drmModeAtomicSetCursor(req, 0);
> + crtc_populate_req(crtc, req);
> + plane_populate_req(, req);
> + ret = drmModeAtomicCommit(crtc->state->desc->fd, req,
> +   DRM_MODE_ATOMIC_TEST_ONLY, NULL);
> +
> + igt_assert_eq(ret, 0);
> +
> + /* go through dpms off/on cycle */
> + set_dpms(crtc->state->desc->fd, DRM_MODE_DPMS_OFF);
> + set_dpms(crtc->state->desc->fd, DRM_MODE_DPMS_ON);
Could probably just set the crtc's active property, same effect with less code.

Either way.

Reviewed-by: Maarten Lankhorst 


> + /* check the state */
> + crtc_check_current_state(crtc, plane_old, ATOMIC_RELAX_NONE);
> + plane_check_current_state(plane_old, ATOMIC_RELAX_NONE);
> +
> + /* Re-enable the plane through the legacy CRTC/primary-plane API, and
> +  * verify through atomic. */
> + crtc_commit_legacy(crtc, plane_old, CRTC_RELAX_MODE);
> +
> + drmModeAtomicFree(req);
> +}
> +
>  static void plane_cursor(struct kms_atomic_crtc_state *crtc,
>struct kms_atomic_plane_state *plane_old)
>  {
> @@ -1427,6 +1497,18 @@ igt_main
>   atomic_state_free(scratch);
>   }
>  
> + igt_subtest("test_only") {
> + struct kms_atomic_state *scratch = atomic_state_dup(current);
> + struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
> + struct kms_atomic_plane_state *plane =
> + find_plane(scratch, PLANE_TYPE_PRIMARY, crtc);
> +
> + igt_require(crtc);
> + igt_require(plane);
> + test_only(crtc, plane);
> + atomic_state_free(scratch);
> + }
> +
>   igt_subtest("plane_cursor_legacy") {
>   struct kms_atomic_state *scratch = atomic_state_dup(current);
>   struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);


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

Re: [Intel-gfx] [i-g-t 1/1] tests/gem_gtt_hog: Clear the parameters for GEM_CREATE ioctl

[Kamble, Sagar A]

On 3/20/2017 1:11 PM, Chris Wilson wrote:

On Mon, Mar 20, 2017 at 11:32:15AM +0530, Sagar Arun Kamble wrote:

Due to garbage data seen by i915, gem_create_ioctl failed for gem obj
created with drmIoctl(GEM_CREATE) without properly initialized
parameters. Can be fixed by calling gem_create helper too.

Whose bug are you working around?
-Chris


Found out now that this is happening due to mismatch in the libdrm headers 
32bit/64bit flags for GEM_CREATE.
Able to resolve by properly using the proper definitions.
Will this fix be still applicable as flags would stay uninitialized with 
current call to drmIoctl?





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

Re: [Intel-gfx] [PATCH] drm/atomic: protect crtc|connector->state with rcu

[Maarten Lankhorst]

Op 20-03-17 om 09:18 schreef Daniel Vetter:
> On Fri, Mar 17, 2017 at 05:52:32PM +0100, Maarten Lankhorst wrote:
>> Op 16-03-17 om 21:15 schreef Daniel Vetter:
>>> On Thu, Mar 16, 2017 at 5:09 PM, Maarten Lankhorst
>>>  wrote:
 Op 16-03-17 om 16:52 schreef Daniel Vetter:
> The vblank code really wants to look at crtc->state without having to
> take a ww_mutex. One option might be to take one of the vblank locks
> right when assigning crtc->state, which would ensure that the vblank
> code doesn't race and access freed memory.
 I'm not sure this is the right approach for vblank.
>>> It's not, it's just that I've had to resurrect this patch quickly
>>> before leaving and accidentally left the vblank stuff in.
>>>
 crtc->state might not be the same as the current state in case of a 
 nonblocking
 modeset that hasn't even disabled the old crtc yet.
> But userspace tends to poke the vblank_ioctl to query the current
> vblank timestamp rather often, and going all in with rcu would help a
> bit. We're not there yet, but also doesn't really hurt.
>
> v2: Maarten needs this to make connector properties atomic, so he can
> peek at state from the various ->mode_valid hooks.
>
> Cc: Maarten Lankhorst 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_atomic.c| 26 +-
>  drivers/gpu/drm/drm_atomic_helper.c |  2 +-
>  include/drm/drm_atomic.h|  5 +
>  include/drm/drm_connector.h | 13 -
>  include/drm/drm_crtc.h  |  9 -
>  5 files changed, 43 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 9b892af7811a..ba11e31ff9ba 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -213,16 +213,10 @@ void drm_atomic_state_clear(struct drm_atomic_state 
> *state)
>  }
>  EXPORT_SYMBOL(drm_atomic_state_clear);
>
> -/**
> - * __drm_atomic_state_free - free all memory for an atomic state
> - * @ref: This atomic state to deallocate
> - *
> - * This frees all memory associated with an atomic state, including all 
> the
> - * per-object state for planes, crtcs and connectors.
> - */
> -void __drm_atomic_state_free(struct kref *ref)
> +void ___drm_atomic_state_free(struct rcu_head *rhead)
>  {
> - struct drm_atomic_state *state = container_of(ref, typeof(*state), 
> ref);
> + struct drm_atomic_state *state =
> + container_of(rhead, typeof(*state), rhead);
>   struct drm_mode_config *config = >dev->mode_config;
>
>   drm_atomic_state_clear(state);
> @@ -236,6 +230,20 @@ void __drm_atomic_state_free(struct kref *ref)
>   kfree(state);
>   }
>  }
 whatisRCU.txt:
 "This function invokes func(head) after a grace period has elapsed.
 This invocation might happen from either softirq or process context,
 so the function is not permitted to block."

 Looking at
 commit 6f0f02dc56f18760b46dc1bf5b3f7386869d4162
 Author: Chris Wilson 
 Date:   Mon Jan 23 21:29:39 2017 +

 drm/i915: Move atomic state free from out of fence release

 I would say that drm_atomic_state_free would definitely block..

 The only thing that makes sense IMO is doing kfree_rcu on the 
 object_states.
 But I don't think RCU is the answer here, it won't protect you against 
 using
 the wrong crtc state.

 I think I would try to use the crtc ww_mutex in the vblank code and 
 serialize it to pending commits, if at all possible.
>>> Oops. I guess it should have come with "totally untested". In that
>>> case we need a workter which does a synchronize_rcu() before
>>> releasing. I don't just want to do a simple kfree_rcu() because by
>>> that point we've (partially) destroyed the state alreayd (so it's
>>> already unsafe to access, and special ruels are ugly). And doing it
>>> here before we release anything in the core would avoid the need for
>>> drivers to bother with kfree_rcu().
>>>
>>> I'll try to respin something less obviously buggy tomorrow :-)
>> It will still be buggy tomorrow, since you have no way to know if the 
>> current hardware crtc_state is anything like crtc->state.
>>
>> :(
> Maybe I wasnt' clear, so let me retry:
>
> - this approach doesn't work for vblank and crtc state. Agreed. I'll
>   remove all the leftover comments I've forgotten to remove in a hurry.
>
> - the patch itself is broken, so can't be used for connector->state
>   peeking in mode_valid either. That one I'll fix.
>
> Does that make sense?
> -Daniel

Yes, but I'm still not completely convinced it's required for connector 

Re: [Intel-gfx] [PATCH] drm/atomic: protect crtc|connector->state with rcu

[Daniel Vetter]

On Fri, Mar 17, 2017 at 05:52:32PM +0100, Maarten Lankhorst wrote:
> Op 16-03-17 om 21:15 schreef Daniel Vetter:
> > On Thu, Mar 16, 2017 at 5:09 PM, Maarten Lankhorst
> >  wrote:
> >> Op 16-03-17 om 16:52 schreef Daniel Vetter:
> >>> The vblank code really wants to look at crtc->state without having to
> >>> take a ww_mutex. One option might be to take one of the vblank locks
> >>> right when assigning crtc->state, which would ensure that the vblank
> >>> code doesn't race and access freed memory.
> >> I'm not sure this is the right approach for vblank.
> > It's not, it's just that I've had to resurrect this patch quickly
> > before leaving and accidentally left the vblank stuff in.
> >
> >> crtc->state might not be the same as the current state in case of a 
> >> nonblocking
> >> modeset that hasn't even disabled the old crtc yet.
> >>> But userspace tends to poke the vblank_ioctl to query the current
> >>> vblank timestamp rather often, and going all in with rcu would help a
> >>> bit. We're not there yet, but also doesn't really hurt.
> >>>
> >>> v2: Maarten needs this to make connector properties atomic, so he can
> >>> peek at state from the various ->mode_valid hooks.
> >>>
> >>> Cc: Maarten Lankhorst 
> >>> Signed-off-by: Daniel Vetter 
> >>> ---
> >>>  drivers/gpu/drm/drm_atomic.c| 26 +-
> >>>  drivers/gpu/drm/drm_atomic_helper.c |  2 +-
> >>>  include/drm/drm_atomic.h|  5 +
> >>>  include/drm/drm_connector.h | 13 -
> >>>  include/drm/drm_crtc.h  |  9 -
> >>>  5 files changed, 43 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> >>> index 9b892af7811a..ba11e31ff9ba 100644
> >>> --- a/drivers/gpu/drm/drm_atomic.c
> >>> +++ b/drivers/gpu/drm/drm_atomic.c
> >>> @@ -213,16 +213,10 @@ void drm_atomic_state_clear(struct drm_atomic_state 
> >>> *state)
> >>>  }
> >>>  EXPORT_SYMBOL(drm_atomic_state_clear);
> >>>
> >>> -/**
> >>> - * __drm_atomic_state_free - free all memory for an atomic state
> >>> - * @ref: This atomic state to deallocate
> >>> - *
> >>> - * This frees all memory associated with an atomic state, including all 
> >>> the
> >>> - * per-object state for planes, crtcs and connectors.
> >>> - */
> >>> -void __drm_atomic_state_free(struct kref *ref)
> >>> +void ___drm_atomic_state_free(struct rcu_head *rhead)
> >>>  {
> >>> - struct drm_atomic_state *state = container_of(ref, typeof(*state), 
> >>> ref);
> >>> + struct drm_atomic_state *state =
> >>> + container_of(rhead, typeof(*state), rhead);
> >>>   struct drm_mode_config *config = >dev->mode_config;
> >>>
> >>>   drm_atomic_state_clear(state);
> >>> @@ -236,6 +230,20 @@ void __drm_atomic_state_free(struct kref *ref)
> >>>   kfree(state);
> >>>   }
> >>>  }
> >> whatisRCU.txt:
> >> "This function invokes func(head) after a grace period has elapsed.
> >> This invocation might happen from either softirq or process context,
> >> so the function is not permitted to block."
> >>
> >> Looking at
> >> commit 6f0f02dc56f18760b46dc1bf5b3f7386869d4162
> >> Author: Chris Wilson 
> >> Date:   Mon Jan 23 21:29:39 2017 +
> >>
> >> drm/i915: Move atomic state free from out of fence release
> >>
> >> I would say that drm_atomic_state_free would definitely block..
> >>
> >> The only thing that makes sense IMO is doing kfree_rcu on the 
> >> object_states.
> >> But I don't think RCU is the answer here, it won't protect you against 
> >> using
> >> the wrong crtc state.
> >>
> >> I think I would try to use the crtc ww_mutex in the vblank code and 
> >> serialize it to pending commits, if at all possible.
> > Oops. I guess it should have come with "totally untested". In that
> > case we need a workter which does a synchronize_rcu() before
> > releasing. I don't just want to do a simple kfree_rcu() because by
> > that point we've (partially) destroyed the state alreayd (so it's
> > already unsafe to access, and special ruels are ugly). And doing it
> > here before we release anything in the core would avoid the need for
> > drivers to bother with kfree_rcu().
> >
> > I'll try to respin something less obviously buggy tomorrow :-)
> It will still be buggy tomorrow, since you have no way to know if the current 
> hardware crtc_state is anything like crtc->state.
> 
> :(

Maybe I wasnt' clear, so let me retry:

- this approach doesn't work for vblank and crtc state. Agreed. I'll
  remove all the leftover comments I've forgotten to remove in a hurry.

- the patch itself is broken, so can't be used for connector->state
  peeking in mode_valid either. That one I'll fix.

Does that make sense?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing 

Re: [Intel-gfx] [PATCH] drm/i915/glk: CDCLK calculation changes for glk

[Chauhan, Madhav]

> -Original Message-
> From: Chauhan, Madhav
> Sent: Friday, March 17, 2017 7:11 PM
> To: Nikula, Jani ; Ander Conselvan De Oliveira
> ; intel-gfx@lists.freedesktop.org
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/glk: CDCLK calculation changes for
> glk
> 
> > -Original Message-
> > From: Nikula, Jani
> > Sent: Thursday, March 16, 2017 7:00 PM
> > To: Ander Conselvan De Oliveira ; Chauhan,
> > Madhav ; intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/glk: CDCLK calculation
> > changes for glk
> >
> > On Thu, 16 Mar 2017, Ander Conselvan De Oliveira
> >  wrote:
> > > On Thu, 2017-03-16 at 15:10 +0200, Jani Nikula wrote:
> > >> On Thu, 16 Mar 2017, "Chauhan, Madhav"
> >  wrote:
> > >> > > -Original Message-
> > >> > > From: Nikula, Jani
> > >> > > Sent: Thursday, February 16, 2017 9:03 PM
> > >> > > To: Chauhan, Madhav ; intel-
> > >> > > g...@lists.freedesktop.org
> > >> > > Cc: Conselvan De Oliveira, Ander
> > >> > > ;
> > >> > > Shankar, Uma ; Mukherjee, Indranil
> > >> > > ; Sharma, Shashank
> > >> > > ; Chauhan, Madhav
> > >> > > ; ville.syrj...@linux.intel.com
> > >> > > Subject: Re: [PATCH] drm/i915/glk: CDCLK calculation changes
> > >> > > for glk
> > >> > >
> > >> > > On Thu, 16 Feb 2017, Madhav Chauhan
> > 
> > >> > > wrote:
> > >> > > > As per BSPEC, valid cdclk values for glk are 79.2, 158.4, 316.8 
> > >> > > > Mhz.
> > >> > > > Practically we can achive only 99% of these cdclk values(HW
> > >> > > > team checking on this). So cdclk should be calculated for the
> > >> > > > given pixclk as per that otherwise it may lead to screen
> > >> > > > corruption
> > for some scenarios.
> > >> > > >
> > >> > > > v2: Rebased to new CDLCK code framework
> > >> > > >
> > >> > > > Signed-off-by: Madhav Chauhan 
> > >> > > > ---
> > >> > > >  drivers/gpu/drm/i915/intel_cdclk.c | 4 ++--
> > >> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >> > > >
> > >> > > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> > >> > > > b/drivers/gpu/drm/i915/intel_cdclk.c
> > >> > > > index d643c0c..834df68 100644
> > >> > > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > >> > > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > >> > > > @@ -1071,9 +1071,9 @@ static int bxt_calc_cdclk(int
> > >> > > > max_pixclk)
> > >> > > >
> > >> > > >  static int glk_calc_cdclk(int max_pixclk)  {
> > >> > > > -  if (max_pixclk > 2 * 158400)
> > >> > > > +  if (max_pixclk > DIV_ROUND_UP(2 * 158400 * 99, 100))
> > >> > >
> > >> > > Where do we ensure we don't use pixel clock 312841..316800?
> > >> > > Clearly we shouldn't use that because we can't guarantee it
> > >> > > works,
> > right?
> > >> >
> > >> > Why do we need to ensure that ?? Can you please elaborate more on
> > this?
> > >> > Here we are finding one of  the defined CDCLK value for a pixel
> > >> > clock
> > >>
> > >> I probably had some great idea a month ago when I wrote that, but I
> > >> can no longer remember what it was. :(
> > >
> > > I'm not sure if that is what you meant, but if the hardware can't
> > > handle it,
> > > intel_compute_max_dotclk() needs to take the 99% limitation into
> > > account
> > too.
> > > I.e., max dot clock would be .99 * 2 *  316800 = 627264.
> >
> > Yes, thank you!

Tested this change on drm-tip. Found that 1-2 times MIPI didn't come up (might 
be some error 
during testing) after that it works  fine every time.
Could this change have some sort of impact on MIPI functionality?? Looked at 
code, found nothing
from that front.

> 
> Ok. Will include this change as well along with additional comments for
> explaining 99% usage of cdclk inside glk_calc_cdclk.
> Thanks for review.
> 
> >
> > Jani.
> >
> > >
> > > Ander
> > >
> > >>
> > >> BR,
> > >> Jani.
> > >>
> > >>
> > >> > >
> > >> > > Before we get the spec update to confirm what to do, I think we
> > >> > > need a comment here explaining what's going on.
> > >> >
> > >> > Will add the following comment, if that's fine, will send the
> > >> > rebased
> > patch:
> > >> > "For GLK platform, only 99% of the defined CDCLK value can be
> achieved
> > >> >   So calculate pixel clock on that basis"
> > >> >
> > >> > Regards,
> > >> > Madhav
> > >> > >
> > >> > > BR,
> > >> > > Jani.
> > >> > >
> > >> > > >return 316800;
> > >> > > > -  else if (max_pixclk > 2 * 79200)
> > >> > > > +  else if (max_pixclk > DIV_ROUND_UP(2 * 79200 * 99, 100))
> > >> > > >return 158400;
> > >> > > >else
> > >> > > >return 79200;
> > >> > >
> > >> > > --
> > >> > > Jani Nikula, Intel Open Source Technology Center
> > >>
> > >>
> > > 

Re: [Intel-gfx] linux-next: build failure after merge of the drm tree

[Daniel Vetter]

On Mon, Mar 20, 2017 at 9:03 AM, Daniel Vetter  wrote:
> On Mon, Mar 20, 2017 at 1:51 AM, Stephen Rothwell  
> wrote:
>> This cherry picking of fixes from new development back to Linus' tree
>> can be a real pain when so many other changes happen in the same files.
>
> One possible fix for this would be if you reuse our rerere cache. The
> only reason we don't go insane with all the drm conflicts is that we
> completely distributed conflict resolution. Developers push a patch,
> script tells them there's a conflict, they resolve it, maintainers
> never even notice.We only notice when we double-check the merge
> resolution when rerere re-applies it for the real backmerge :-) The
> merge order in drm-tip should also match what you have in linux-next,
> so you should be able to entirely reuse them.
>
> Anyway, if you trust us enough to scoop up random git rerere caches
> (or at least use them to double-check your own), they're all public:
>
> git://anongit.freedesktop.org/drm-tip rerere-cache
>
> Or
>
> https://cgit.freedesktop.org/drm-tip/tree/rr-cache?h=rerere-cache
>
> Yes we should probably gc them, but disk space is cheap.

And the merge order, in case you want to check that. We can adjust it ofc:

https://cgit.freedesktop.org/drm-tip/tree/nightly.conf?h=rerere-cache

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] linux-next: build failure after merge of the drm tree

[Daniel Vetter]

On Mon, Mar 20, 2017 at 1:51 AM, Stephen Rothwell  wrote:
> This cherry picking of fixes from new development back to Linus' tree
> can be a real pain when so many other changes happen in the same files.

One possible fix for this would be if you reuse our rerere cache. The
only reason we don't go insane with all the drm conflicts is that we
completely distributed conflict resolution. Developers push a patch,
script tells them there's a conflict, they resolve it, maintainers
never even notice.We only notice when we double-check the merge
resolution when rerere re-applies it for the real backmerge :-) The
merge order in drm-tip should also match what you have in linux-next,
so you should be able to entirely reuse them.

Anyway, if you trust us enough to scoop up random git rerere caches
(or at least use them to double-check your own), they're all public:

git://anongit.freedesktop.org/drm-tip rerere-cache

Or

https://cgit.freedesktop.org/drm-tip/tree/rr-cache?h=rerere-cache

Yes we should probably gc them, but disk space is cheap.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 1/2] lib: Open debugfs files for the given DRM device

[Tomeu Vizoso]

When opening a DRM debugfs file, locate the right path based on the
given DRM device FD.

This is needed so, in setups with more than one DRM device, any
operations on debugfs files affect the expected DRM device.

v2: rebased and fixed new API additions

Signed-off-by: Tomeu Vizoso 
Reviewed-by: Robert Foss 
---
 benchmarks/gem_latency.c  |   2 +-
 lib/drmtest.c |   2 +-
 lib/igt_aux.c |  10 +--
 lib/igt_aux.h |   4 +-
 lib/igt_debugfs.c | 185 ++
 lib/igt_debugfs.h |  30 +++
 lib/igt_gt.c  |  14 +--
 lib/igt_gt.h  |   4 +-
 lib/igt_kms.c |  10 +--
 lib/igt_kms.h |   4 +-
 lib/intel_io.h|   2 +-
 lib/intel_mmio.c  |   4 +-
 lib/intel_os.c|   9 +-
 tests/drv_hangman.c   |   4 +-
 tests/drv_missed_irq.c|  35 
 tests/drv_suspend.c   |   8 +-
 tests/gem_concurrent_all.c|   2 +-
 tests/gem_eio.c   |   2 +-
 tests/gem_exec_latency.c  |   2 +-
 tests/gem_exec_parse.c|   2 +-
 tests/gem_exec_whisper.c  |   6 +-
 tests/gem_mocs_settings.c |  14 +--
 tests/gem_persistent_relocs.c |   2 +-
 tests/gem_ppgtt.c |  10 +--
 tests/gem_reloc_vs_gpu.c  |   2 +-
 tests/gem_render_linear_blits.c   |   2 +-
 tests/gem_render_tiled_blits.c|   2 +-
 tests/gem_seqno_wrap.c|  24 ++---
 tests/gem_tiled_swapping.c|   9 +-
 tests/gem_workarounds.c   |   4 +-
 tests/kms_atomic_transition.c |   2 +-
 tests/kms_ccs.c   |   4 +-
 tests/kms_chv_cursor_fail.c   |   4 +-
 tests/kms_crtc_background_color.c |   4 +-
 tests/kms_cursor_crc.c|   4 +-
 tests/kms_cursor_legacy.c |   4 +-
 tests/kms_draw_crc.c  |   2 +-
 tests/kms_fbc_crc.c   |   8 +-
 tests/kms_fbcon_fbt.c |  44 -
 tests/kms_flip_tiling.c   |   8 +-
 tests/kms_frontbuffer_tracking.c  |  26 +++---
 tests/kms_invalid_dotclock.c  |   6 +-
 tests/kms_mmap_write_crc.c|   4 +-
 tests/kms_mmio_vs_cs_flip.c   |   6 +-
 tests/kms_pipe_color.c|   5 +-
 tests/kms_pipe_crc_basic.c|  12 +--
 tests/kms_plane.c |   4 +-
 tests/kms_plane_lowres.c  |  14 ++-
 tests/kms_plane_multiple.c|   4 +-
 tests/kms_plane_scaling.c |   4 +-
 tests/kms_psr_sink_crc.c  |   6 +-
 tests/kms_pwrite_crc.c|   4 +-
 tests/kms_rotation_crc.c  |   4 +-
 tests/kms_sink_crc_basic.c|   8 +-
 tests/kms_universal_plane.c   |  12 +--
 tests/perf.c  |  16 ++--
 tests/pm_lpsp.c   |   2 +-
 tests/pm_rpm.c|   6 +-
 tests/pm_sseu.c   |  28 +++---
 tests/prime_mmap_kms.c|   2 +-
 tools/intel_display_crc.c |   2 +-
 tools/intel_display_poller.c  |   8 +-
 tools/intel_dp_compliance.c   |   6 +-
 tools/intel_forcewaked.c  |  10 ++-
 tools/intel_gpu_top.c |   8 +-
 tools/intel_guc_logger.c  |  10 ++-
 tools/intel_infoframes.c  |   7 +-
 tools/intel_l3_parity.c   |   2 +-
 tools/intel_panel_fitter.c|   9 +-
 tools/intel_perf_counters.c   |   2 +-
 tools/intel_reg.c |  12 ++-
 tools/intel_watermark.c   |  23 +++--
 72 files changed, 425 insertions(+), 341 deletions(-)

diff --git a/benchmarks/gem_latency.c b/benchmarks/gem_latency.c
index 7be78f20358f..fc875f1acb0a 100644
--- a/benchmarks/gem_latency.c
+++ b/benchmarks/gem_latency.c
@@ -470,7 +470,7 @@ static int run(int seconds,
if (gen < 6)
return IGT_EXIT_SKIP; /* Needs BCS timestamp */
 
-   intel_register_access_init(intel_get_pci_device(), false);
+   intel_register_access_init(intel_get_pci_device(), false, fd);
 
if (gen == 6)
timestamp_reg = REG(RCS_TIMESTAMP);
diff --git a/lib/drmtest.c b/lib/drmtest.c
index 065ab1198e79..35f71c0d4f45 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -183,7 +183,7 @@ void gem_quiescent_gpu(int fd)
gem_sync(fd, obj.handle);
gem_close(fd, obj.handle);
 
-   igt_drop_caches_set(DROP_RETIRE | DROP_FREED);
+   igt_drop_caches_set(fd, DROP_RETIRE | DROP_FREED);
 }
 
 /**
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 7ee279a3cc33..1222806c94a3 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -350,10 +350,10 @@ void igt_stop_signal_helper(void)
 }
 
 static struct igt_helper_process shrink_helper;
-static void __attribute__((noreturn)) shrink_helper_process(pid_t pid)
+static void __attribute__((noreturn)) shrink_helper_process(int fd, pid_t pid)
 {
while (1) {
-   

[Intel-gfx] [PATCH i-g-t 2/2] kms_pipe_crc_basic: Skip sequence tests if the source doesn't provide frame numbers

[Tomeu Vizoso]

Some frame sources such as sinks aren't able to provide meaningful frame
numbers, so in those cases just skip the TEST_SEQUENCE tests.

v2: Rebased

Signed-off-by: Tomeu Vizoso 
---
 tests/kms_pipe_crc_basic.c | 28 ++--
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index 49e856abbf4c..5a01fc6c2777 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -89,7 +89,7 @@ static void test_bad_source(data_t *data)
 #define TEST_SEQUENCE (1<<0)
 #define TEST_NONBLOCK (1<<1)
 
-static void
+static bool
 test_read_crc_for_output(data_t *data, int pipe, igt_output_t *output,
 unsigned flags)
 {
@@ -167,9 +167,23 @@ test_read_crc_for_output(data_t *data, int pipe, 
igt_output_t *output,
for (j = 0; j < (n_crcs - 1); j++)
igt_assert_crc_equal([j], [j + 1]);
 
-   if (flags & TEST_SEQUENCE)
-   for (j = 0; j < (n_crcs - 1); j++)
-   igt_assert_eq(crcs[j].frame + 1, crcs[j + 
1].frame);
+   if (flags & TEST_SEQUENCE) {
+   if (!crcs[0].has_valid_frame) {
+   igt_info("Skipping connector, its 'auto' source 
doesn't give us frame numbers.\n");
+
+   free(crcs);
+   igt_remove_fb(data->drm_fd, >fb);
+   igt_plane_set_fb(primary, NULL);
+
+   igt_output_set_pipe(output, PIPE_ANY);
+
+   return false;
+   } else {
+   for (j = 0; j < (n_crcs - 1); j++)
+   igt_assert_eq(crcs[j].frame + 1,
+   crcs[j + 1].frame);
+   }
+   }
 
free(crcs);
igt_remove_fb(data->drm_fd, >fb);
@@ -177,6 +191,8 @@ test_read_crc_for_output(data_t *data, int pipe, 
igt_output_t *output,
 
igt_output_set_pipe(output, PIPE_ANY);
}
+
+   return true;
 }
 
 static void test_read_crc(data_t *data, int pipe, unsigned flags)
@@ -193,8 +209,8 @@ static void test_read_crc(data_t *data, int pipe, unsigned 
flags)
 igt_subtest_name(), igt_output_name(output),
 kmstest_pipe_name(pipe));
 
-   test_read_crc_for_output(data, pipe, output, flags);
-   valid_connectors ++;
+   if (test_read_crc_for_output(data, pipe, output, flags))
+   valid_connectors ++;
}
 
igt_require_f(valid_connectors, "No connector found for pipe %i\n", 
pipe);
-- 
2.9.3

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

[Intel-gfx] [PATCH 1/2] dim: Update drm-tip in update-next

[Daniel Vetter]

The integration branch moved!

Signed-off-by: Daniel Vetter 
---
 dim | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/dim b/dim
index 64edcda83084..d8b190efe2de 100755
--- a/dim
+++ b/dim
@@ -1291,13 +1291,14 @@ function dim_update_next
remote=`url_to_remote $drm_tip_ssh`
 
git pull --ff-only
+   git fetch drm-tip
 
if ! git branch --merged $remote/drm-tip | grep -q drm-intel-fixes ; 
then
-   echo "drm-intel-fixes not merged into -nigthly, please update!"
+   echo "drm-intel-fixes not merged into drm-tip, please update!"
exit 2
fi
if ! git branch --merged $remote/drm-tip | grep -q 
drm-intel-next-queued ; then
-   echo "drm-intel-next-queued not merged into -nigthly, please 
update!"
+   echo "drm-intel-next-queued not merged into drm-tip, please 
update!"
exit 2
fi
 
-- 
2.11.0

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

[Intel-gfx] [PATCH 2/2] dim: Docs for Ville's tools

[Daniel Vetter]

Cc: Ville Syrjälä 
Signed-off-by: Daniel Vetter 
---
 dim.rst | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/dim.rst b/dim.rst
index a778701c7335..1bec3838bace 100644
--- a/dim.rst
+++ b/dim.rst
@@ -182,6 +182,22 @@ apply-queued|aq [*git am arguments*]
 Applies a patch to -fixes, -next-fixes or -next-queued respectively, complains
 if it's not the right branch. Additional arguments are passed to git am.
 
+extract-tags *branch* [*git-rangeish*]
+--
+
+extract-queued [*git-rangeish*]
+---
+
+extract-fixes [*git-rangeish*]
+--
+
+extract-next-fixes [*git-rangeish*]
+---
+
+This extracts various tags (eg. Reviwed-by:) from emails and applies them to 
the
+top commit on the given branch. You can give the comamnd a rangeish to add the
+tags from the same email to multiple already applied patches.
+
 magic-patch|mp [-a]
 ---
 Apply a patch using patch and then wiggle in any conflicts. When passing the
@@ -189,6 +205,20 @@ option -a automatically changes the working directory into 
the git repository
 used by the last previous branch-specific command. This is useful with the
 per-branch workdir model.
 
+add-link *branch*
+-
+
+add-link-queued
+---
+
+add-link-fixes
+--
+
+add-link-next-fixes
+---
+
+This command adds the Link: tag (for patches that failed to apply directly).
+
 magic-rebase-resolve|mrr
 
 Tries to resolve a rebase conflict by first resetting the tree
-- 
2.11.0

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

Re: [Intel-gfx] [i-g-t 1/1] tests/gem_gtt_hog: Clear the parameters for GEM_CREATE ioctl

[Chris Wilson]

On Mon, Mar 20, 2017 at 11:32:15AM +0530, Sagar Arun Kamble wrote:
> Due to garbage data seen by i915, gem_create_ioctl failed for gem obj
> created with drmIoctl(GEM_CREATE) without properly initialized
> parameters. Can be fixed by calling gem_create helper too.

Whose bug are you working around?
-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] Updated drm-intel-testing

[Daniel Vetter]

Hi all,

More in i915 for 4.12:

- designware i2c fixes from Hans de Goede, in a topic branch shared
  with other subsystems (maybe, they didn't confirm, but requested the
  pull)
- drop drm_panel usage from the intel dsi vbt panel (Jani)
- vblank evasion improvements and tracing (Maarten and Ville)
- clarify spinlock irq semantics again a bit (Tvrtko)
- new ->pwrite backend hook (right now just for shmem pageche writes),
  from Chris
- more planar/ccs work from Ville
- hotplug safe connector iterators everywhere
- userptr fixes (Chris)
- selftests for cache coloring eviction (Matthew Auld)
- extend debugfs drop_caches interface for shrinker testing (Chris)
- baytrail "the rps kills the machine" fix (Chris)
- use new atomic state iterators, a lot (Maarten)
- refactor guc/huc code some (Arkadiusz Hiler)
- tighten breadcrumbs rbtree a bit (Chris)
- improve wrap-around and time handling in rps residency counters
  (Mika)
- split reset-in-progress in two flags, backoff and handoff (Chris)
- other misc reset improvements from a few people
- bunch of vgpu interaction fixes with recent code changes
- misc stuff all over, as usual

Happy testing!

Cheers, 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

[Intel-gfx] [i-g-t 1/1] tests/gem_gtt_hog: Clear the parameters for GEM_CREATE ioctl

[Sagar Arun Kamble]

Due to garbage data seen by i915, gem_create_ioctl failed for gem obj
created with drmIoctl(GEM_CREATE) without properly initialized
parameters. Can be fixed by calling gem_create helper too.

Cc: Chris Wilson 
Cc: Lukasz Kalamarz 
Cc: Radoslaw Szwichtenberg 
Signed-off-by: Sagar Arun Kamble 
---
 tests/gem_gtt_hog.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/gem_gtt_hog.c b/tests/gem_gtt_hog.c
index a3dbfad..0696bdc 100644
--- a/tests/gem_gtt_hog.c
+++ b/tests/gem_gtt_hog.c
@@ -93,6 +93,7 @@ static void busy(data_t *data, uint32_t handle, int size, int 
loops)
gem_exec[0].handle = handle;
gem_exec[0].flags = EXEC_OBJECT_NEEDS_FENCE;
 
+   memset(, 0, sizeof(create));
create.handle = 0;
create.size = 4096;
drmIoctl(data->fd, DRM_IOCTL_I915_GEM_CREATE, );
-- 
1.9.1

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