Re: [Intel-gfx] [PATCH v3 0/2] Handle unsupported configuration with IF-ID
On Fri, Jun 30, 2017 at 05:40:58PM +0530, Mahesh Kumar wrote: > Gen9+ Interlace fetch mode doesn't support few plane configurations & pipe > scaling. > - Y-tile > - 90/270 rotation > - pipe/plane scaling > - 420 planar formats Do we have igt testcases that try to exercise all this? When fixing bugs, pls make sure you don't fix the bugs, but also make sure we have solid coverage. Given that this escaped for years, it's very likely our coverage is _really_ bad and needs to be improve a lot for testing interlaced modes ... Thanks, Daniel > > Changes since V2: > - Address review comments from ville > > Mahesh Kumar (2): > drm/i915/skl+: Check for supported plane configuration in Interlace > mode > drm/i915/skl+: Scaling not supported in IF-ID Interlace mode > > drivers/gpu/drm/i915/intel_atomic_plane.c | 15 +++ > drivers/gpu/drm/i915/intel_display.c | 15 +++ > 2 files changed, 30 insertions(+) > > -- > 2.13.0 > > ___ > 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] Updated drm-intel-testing
Hi all, 2nd round of 4.14 features: - prep for deferred fbdev setup - refactor fixed 16.16 computations and skl+ wm code (Mahesh Kumar) - more cnl paches (Rodrigo, Imre et al) - tighten context cleanup and handling (Chris Wilson) - fix interlaced handling on skl+ (Mahesh Kumar) - small bits 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
Re: [Intel-gfx] [i-g-t] many testcases skiped
On Mon, Jul 17, 2017 at 12:28:00PM +0800, Hailin Gu wrote: > Hi~ all, > > Does the test program have any hardware or version limitations? > > > I found that most testcase could run on a machine with intel(R) Core(TM) > i7-2600K CPU @ 3.40GHz. > > But can't run on Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz or Intel(R) > Core(TM) i3-2120 CPU @ 3.30GHz. Hey, The hardware limitation is that you should have integradted graphics. E5-2669 v3 does not have one according to ark.intel.com. Error message seems to be pretty clear, stating: "No known gpu found". As of the i3 - it probably should work. Can you share complete logs from that machinge to see what goes wrong there? -- Cheers, Arek > Thank you > > *From:*Hailin Gu [mailto:hailinx...@intel.com] > *Sent:* Tuesday, July 11, 2017 7:51 PM > *To:* intel-gfx@lists.freedesktop.org > *Cc:* Li, ZhijianX ; Li, Philip > *Subject:* [i-g-t] many testcases skiped > > Hi~ all, > > I want to run the testcases of igt, but there are some problems during that. > Lots of testcase started with gem and kms skiped. > > Thanks for your helping. > > I make the piglit and igt by following steps > > $cd piglit > > $cmake . > > $male > > $make install > > $cd intel-gpu-tools > > $./autogen > > $make > > All of these steps are successful. And I write the path of igt in > piglit.conf. running the tests by calling > > "./piglit run igt -t gem_sync /tmp" > > But all the case shows skip. Here is the output. > > root@lkp-skl-sp1 /lkp/benchmarks/piglit# ./bin/piglit run igt -t > gem_sync /tmp > [46/46] skip: 46 > > Further, I try to run the test case directly, by running the test program in > igt. All the case skiped with following tips. > > root@lkp-skl-sp1 /lkp/benchmarks/piglit/lib/piglit/bin/igt/tests# > ./gem_sync > IGT-Version: 1.19-g4258cc8e (x86_64) (Linux: 4.9.0 x86_64) > Test requirement not met in function drm_open_driver, file > drmtest.c:359: > Test requirement: !(fd<0) > No known gpu found > Last errno: 2, No such file or directory > > my environment: > > - Linux lkp-skl-sp1 4.9.0 > > - cpu skylake i7-6700 > > May I have something ignored? Or What kind of equipment can meet the test > requirements? > > > > Thank you > > Gu > > ___ > 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] [i-g-t] many testcases skiped
On Mon, Jul 17, 2017 at 11:14:03AM +0300, Arkadiusz Hiler wrote: > On Mon, Jul 17, 2017 at 12:28:00PM +0800, Hailin Gu wrote: > > Hi~ all, > > > > Does the test program have any hardware or version limitations? > > > > > > I found that most testcase could run on a machine with intel(R) Core(TM) > > i7-2600K CPU @ 3.40GHz. > > > > But can't run on Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz or Intel(R) > > Core(TM) i3-2120 CPU @ 3.30GHz. > > Hey, > > The hardware limitation is that you should have integradted graphics. And by "limitation" I've meant "requirement". At least when it comes to Intel GPUs. > E5-2669 v3 does not have one according to ark.intel.com. > > Error message seems to be pretty clear, stating: "No known gpu found". > > As of the i3 - it probably should work. Can you share complete logs from > that machinge to see what goes wrong there? > > -- > Cheers, > Arek > > > Thank you > > > > *From:*Hailin Gu [mailto:hailinx...@intel.com] > > *Sent:* Tuesday, July 11, 2017 7:51 PM > > *To:* intel-gfx@lists.freedesktop.org > > *Cc:* Li, ZhijianX ; Li, Philip > > *Subject:* [i-g-t] many testcases skiped > > > > Hi~ all, > > > > I want to run the testcases of igt, but there are some problems during that. > > Lots of testcase started with gem and kms skiped. > > > > Thanks for your helping. > > > > I make the piglit and igt by following steps > > > > $cd piglit > > > > $cmake . > > > > $male > > > > $make install > > > > $cd intel-gpu-tools > > > > $./autogen > > > > $make > > > > All of these steps are successful. And I write the path of igt in > > piglit.conf. running the tests by calling > > > > "./piglit run igt -t gem_sync /tmp" > > > > But all the case shows skip. Here is the output. > > > > root@lkp-skl-sp1 /lkp/benchmarks/piglit# ./bin/piglit run igt -t > > gem_sync /tmp > > [46/46] skip: 46 > > > > Further, I try to run the test case directly, by running the test program in > > igt. All the case skiped with following tips. > > > > root@lkp-skl-sp1 /lkp/benchmarks/piglit/lib/piglit/bin/igt/tests# > > ./gem_sync > > IGT-Version: 1.19-g4258cc8e (x86_64) (Linux: 4.9.0 x86_64) > > Test requirement not met in function drm_open_driver, file > > drmtest.c:359: > > Test requirement: !(fd<0) > > No known gpu found > > Last errno: 2, No such file or directory > > > > my environment: > > > > - Linux lkp-skl-sp1 4.9.0 > > > > - cpu skylake i7-6700 > > > > May I have something ignored? Or What kind of equipment can meet the test > > requirements? > > > > > > > > Thank you > > > > Gu > > > > > ___ > > 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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 3/3] tests/gem_reset_stats: Enforce full chip reset mode before run
On Thu, Jul 06, 2017 at 10:29:41AM -0700, Michel Thierry wrote: > On 06/07/17 04:12, Arkadiusz Hiler wrote: > > On Tue, Jun 20, 2017 at 11:25:02AM -0700, Michel Thierry wrote: > > > Platforms with per-engine reset enabled (i915.reset=2) are unlikely to > > > perform a full chip reset, keeping the reset_count unmodified. In order > > > to keep the expectations of this test, enforce that full GPU reset is > > > enabled (i915.reset=1). > > > > > > Later on, we can expand the reset_stats ioctl to also return the number > > > of per-engine resets and use reset_count + reset_engine_count when > > > checking for the updated reset count. > > > > > > Signed-off-by: Michel Thierry > > > > This no longer applies due to changes in the context. It would be nice > > if you would send rebased version as well :-) > > Hi Arek, > > I think the v2 of these patches still apply cleanly, > > https://patchwork.freedesktop.org/patch/164248/ and > https://patchwork.freedesktop.org/patch/164249/ Since you've said that you have a consensus with Antionio, and the patches LGTM. Acked-by: Arkadiusz Hiler Pushed, thanks! ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 3/4] drm/i915/execlists: Split insert_request()
In the next patch we will want to reinsert a request not at the end of the priority queue, but at the front. Here we split insert_request() into two, the first function retrieves the priority list (for reuse for unsubmit later) and a wrapper function to insert at the end of that list and to schedule the tasklet if we were first. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_lrc.c | 35 +++ 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 8ab0c4b76c98..d227480b3a26 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -292,10 +292,10 @@ uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx, return ctx->engine[engine->id].lrc_desc; } -static bool -insert_request(struct intel_engine_cs *engine, - struct i915_priotree *pt, - int prio) +static struct i915_priolist * +lookup_priolist(struct intel_engine_cs *engine, + struct i915_priotree *pt, + int prio) { struct i915_priolist *p; struct rb_node **parent, *rb; @@ -317,8 +317,7 @@ insert_request(struct intel_engine_cs *engine, parent = &rb->rb_right; first = false; } else { - list_add_tail(&pt->link, &p->requests); - return false; + return p; } } @@ -344,16 +343,14 @@ insert_request(struct intel_engine_cs *engine, } p->priority = prio; + INIT_LIST_HEAD(&p->requests); rb_link_node(&p->node, rb, parent); rb_insert_color(&p->node, &engine->execlist_queue); - INIT_LIST_HEAD(&p->requests); - list_add_tail(&pt->link, &p->requests); - if (first) engine->execlist_first = &p->node; - return first; + return ptr_pack_bits(p, first, 1); } static inline void @@ -712,6 +709,17 @@ static void intel_lrc_irq_handler(unsigned long data) intel_uncore_forcewake_put(dev_priv, engine->fw_domains); } +static void insert_request(struct intel_engine_cs *engine, + struct i915_priotree *pt, + int prio) +{ + struct i915_priolist *p = lookup_priolist(engine, pt, prio); + + list_add_tail(&pt->link, &ptr_mask_bits(p, 1)->requests); + if (ptr_unmask_bits(p, 1) && execlists_elsp_ready(engine)) + tasklet_hi_schedule(&engine->irq_tasklet); +} + static void execlists_submit_request(struct drm_i915_gem_request *request) { struct intel_engine_cs *engine = request->engine; @@ -720,12 +728,7 @@ static void execlists_submit_request(struct drm_i915_gem_request *request) /* Will be called from irq-context when using foreign fences. */ spin_lock_irqsave(&engine->timeline->lock, flags); - if (insert_request(engine, - &request->priotree, - request->priotree.priority)) { - if (execlists_elsp_ready(engine)) - tasklet_hi_schedule(&engine->irq_tasklet); - } + insert_request(engine, &request->priotree, request->priotree.priority); GEM_BUG_ON(!engine->execlist_first); GEM_BUG_ON(list_empty(&request->priotree.link)); -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 1/4] drm/i915/execlists: Move insert_request()
Move insert_request() earlier to avoid a forward declaration in a later patch. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_lrc.c | 128 +++ 1 file changed, 64 insertions(+), 64 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index ad61d1998fb7..1b2f0e3d383a 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -292,6 +292,70 @@ uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx, return ctx->engine[engine->id].lrc_desc; } +static bool +insert_request(struct intel_engine_cs *engine, + struct i915_priotree *pt, + int prio) +{ + struct i915_priolist *p; + struct rb_node **parent, *rb; + bool first = true; + + if (unlikely(engine->no_priolist)) + prio = I915_PRIORITY_NORMAL; + +find_priolist: + /* most positive priority is scheduled first, equal priorities fifo */ + rb = NULL; + parent = &engine->execlist_queue.rb_node; + while (*parent) { + rb = *parent; + p = rb_entry(rb, typeof(*p), node); + if (prio > p->priority) { + parent = &rb->rb_left; + } else if (prio < p->priority) { + parent = &rb->rb_right; + first = false; + } else { + list_add_tail(&pt->link, &p->requests); + return false; + } + } + + if (prio == I915_PRIORITY_NORMAL) { + p = &engine->default_priolist; + } else { + p = kmem_cache_alloc(engine->i915->priorities, GFP_ATOMIC); + /* Convert an allocation failure to a priority bump */ + if (unlikely(!p)) { + prio = I915_PRIORITY_NORMAL; /* recurses just once */ + + /* To maintain ordering with all rendering, after an +* allocation failure we have to disable all scheduling. +* Requests will then be executed in fifo, and schedule +* will ensure that dependencies are emitted in fifo. +* There will be still some reordering with existing +* requests, so if userspace lied about their +* dependencies that reordering may be visible. +*/ + engine->no_priolist = true; + goto find_priolist; + } + } + + p->priority = prio; + rb_link_node(&p->node, rb, parent); + rb_insert_color(&p->node, &engine->execlist_queue); + + INIT_LIST_HEAD(&p->requests); + list_add_tail(&pt->link, &p->requests); + + if (first) + engine->execlist_first = &p->node; + + return first; +} + static inline void execlists_context_status_change(struct drm_i915_gem_request *rq, unsigned long status) @@ -649,70 +713,6 @@ static void intel_lrc_irq_handler(unsigned long data) intel_uncore_forcewake_put(dev_priv, engine->fw_domains); } -static bool -insert_request(struct intel_engine_cs *engine, - struct i915_priotree *pt, - int prio) -{ - struct i915_priolist *p; - struct rb_node **parent, *rb; - bool first = true; - - if (unlikely(engine->no_priolist)) - prio = I915_PRIORITY_NORMAL; - -find_priolist: - /* most positive priority is scheduled first, equal priorities fifo */ - rb = NULL; - parent = &engine->execlist_queue.rb_node; - while (*parent) { - rb = *parent; - p = rb_entry(rb, typeof(*p), node); - if (prio > p->priority) { - parent = &rb->rb_left; - } else if (prio < p->priority) { - parent = &rb->rb_right; - first = false; - } else { - list_add_tail(&pt->link, &p->requests); - return false; - } - } - - if (prio == I915_PRIORITY_NORMAL) { - p = &engine->default_priolist; - } else { - p = kmem_cache_alloc(engine->i915->priorities, GFP_ATOMIC); - /* Convert an allocation failure to a priority bump */ - if (unlikely(!p)) { - prio = I915_PRIORITY_NORMAL; /* recurses just once */ - - /* To maintain ordering with all rendering, after an -* allocation failure we have to disable all scheduling. -* Requests will then be executed in fifo, and schedule -* will ensure that dependencies are emitted in fifo. -* There will be still some reordering with existing -* reques
[Intel-gfx] [RFC 4/4] drm/i915/execlists: Preemption!
When we write to ELSP, it triggers a context preemption at the earliest arbitration point (3DPRIMITIVE, some PIPECONTROLs, a few other operations and the explicit MI_ARB_CHECK). If this is to the same context, it triggers a LITE_RESTORE where the RING_TAIL is merely updated (used currently to chain requests from the same context together, avoiding bubbles). However, if it is to a different, a full context-switch is performed and it will start to execute the new context saving the image of the old for later execution. Previously we avoided preemption by only submitting a new context when the old was idle. But now we wish embrace it, and if the new request has a higher priority than the currently executing request, we write to the ELSP regardless, thus triggering preemption. In the context-switch interrupt handler, we therefore need to check whether the old context was completed or whether we just switched to the new context preemptively. In the deqeueu function (responsible for deciding who executes next), we need to take note of when we will cause a preemption and move all the preempted requests back onto the execution list. After that we can proceed as normal. The current heuristic for deciding when to preempt are only if the new request is of higher priority, and has the privileged priority of greater than 0. Signed-off-by: Chris Wilson Cc: Michal Winiarski Cc: Tvrtko Ursulin Cc: Arkadiusz Hiler Cc: Mika Kuoppala Cc: Ben Widawsky --- drivers/gpu/drm/i915/intel_lrc.c| 146 drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- 2 files changed, 110 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index d227480b3a26..fe037bb9644c 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -397,9 +397,9 @@ static u64 execlists_update_context(struct drm_i915_gem_request *rq) return ce->lrc_desc; } -static void execlists_submit_ports(struct intel_engine_cs *engine) +static void execlists_submit_ports(struct intel_engine_cs *engine, + struct execlist_port *port) { - struct execlist_port *port = engine->execlist_port; u32 __iomem *elsp = engine->i915->regs + i915_mmio_reg_offset(RING_ELSP(engine)); unsigned int n; @@ -456,24 +456,21 @@ static void port_assign(struct execlist_port *port, port_set(port, port_pack(i915_gem_request_get(rq), port_count(port))); } +static void unwind_wa_tail(struct drm_i915_gem_request *rq) +{ + rq->tail = intel_ring_wrap(rq->ring, + rq->wa_tail - WA_TAIL_DWORDS*sizeof(u32)); + assert_ring_tail_valid(rq->ring, rq->tail); +} + static void execlists_dequeue(struct intel_engine_cs *engine) { - struct drm_i915_gem_request *last; - struct execlist_port *port = engine->execlist_port; + struct execlist_port *ports = engine->execlist_port; + struct execlist_port *port = ports; + struct drm_i915_gem_request *last = port_request(port); struct rb_node *rb; bool submit = false; - - last = port_request(port); - if (last) - /* WaIdleLiteRestore:bdw,skl -* Apply the wa NOOPs to prevent ring:HEAD == req:TAIL -* as we resubmit the request. See gen8_emit_breadcrumb() -* for where we prepare the padding after the end of the -* request. -*/ - last->tail = last->wa_tail; - - GEM_BUG_ON(port_isset(&port[1])); + bool once = last; /* Hardware submission is through 2 ports. Conceptually each port * has a (RING_START, RING_HEAD, RING_TAIL) tuple. RING_START is @@ -503,6 +500,49 @@ static void execlists_dequeue(struct intel_engine_cs *engine) struct i915_priolist *p = rb_entry(rb, typeof(*p), node); struct drm_i915_gem_request *rq, *rn; + if (once) { + if (port_count(&port[0]) > 1) + goto done; + + if (p->priority > max(last->priotree.priority, 0)) { + list_for_each_entry_safe_reverse(rq, rn, + &engine->timeline->requests, +link) { + struct i915_priolist *p; + + if (i915_gem_request_completed(rq)) + break; + + __i915_gem_request_unsubmit(rq); + unwind_wa_tail(rq); + + p = lookup_priolist(engine, + &rq->priotree, + rq->pr
[Intel-gfx] [RFC 2/4] drm/i915/execlists: Keep request->priority for its lifetime
With preemption, we will want to "unsubmit" a request, taking it back from the hw and returning it to the priority sorted execution list. In order to know where to insert it into that list, we need to remember its adjust priority (which may change even as it was being executed). Signed-off-by: Chris Wilson Cc: Michal Winiarski --- drivers/gpu/drm/i915/intel_lrc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 1b2f0e3d383a..8ab0c4b76c98 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -552,8 +552,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine) } INIT_LIST_HEAD(&rq->priotree.link); - rq->priotree.priority = INT_MAX; - __i915_gem_request_submit(rq); trace_i915_gem_request_in(rq, port_index(port, engine)); last = rq; @@ -687,6 +685,7 @@ static void intel_lrc_irq_handler(unsigned long data) execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT); trace_i915_gem_request_out(rq); + rq->priotree.priority = INT_MAX; i915_gem_request_put(rq); port[0] = port[1]; -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [RFC,1/4] drm/i915/execlists: Move insert_request()
== Series Details == Series: series starting with [RFC,1/4] drm/i915/execlists: Move insert_request() URL : https://patchwork.freedesktop.org/series/27382/ State : success == Summary == Series 27382v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/27382/revisions/1/mbox/ Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-b: pass -> DMESG-WARN (fi-byt-j1900) fdo#101705 +1 fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:450s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:430s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:355s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:537s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:510s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:488s fi-byt-n2820 total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:484s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:595s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:435s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:418s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:424s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:487s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:476s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:465s fi-kbl-7560u total:279 pass:268 dwarn:1 dfail:0 fail:0 skip:10 time:573s fi-kbl-r total:279 pass:260 dwarn:1 dfail:0 fail:0 skip:18 time:581s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:574s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:457s fi-skl-6700hqtotal:279 pass:262 dwarn:0 dfail:0 fail:0 skip:17 time:589s fi-skl-6700k total:279 pass:257 dwarn:4 dfail:0 fail:0 skip:18 time:466s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:483s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:435s fi-skl-x1585ltotal:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:478s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:543s fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:408s 0ea81ff8a340aa1f2bc863f4c8e631542e47c98e drm-tip: 2017y-07m-17d-07h-18m-34s UTC integration manifest 79664bf drm/i915/execlists: Split insert_request() 94f9ee5 drm/i915/execlists: Keep request->priority for its lifetime b207421 drm/i915/execlists: Move insert_request() == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5205/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Consistently use enum pipe for PCH transcoders
On Fri, Jul 14, 2017 at 06:04:03PM -0700, Matthias Kaehlcke wrote: > The current code uses two different enum types for PCH transcoders and > performs implicit conversions between the two types. This is error prone > and causes clang to raise warnings like this: > > drivers/gpu/drm/i915/intel_dp.c:3546:51: warning: implicit conversion > from enumeration type 'enum pipe' to different enumeration type > 'enum transcoder' [-Wenum-conversion] > intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false); > > Consistently use the type enum pipe for PCH transcoders. > > Signed-off-by: Matthias Kaehlcke Thanks for respinning. Unfortunately it doesn't apply cleanly to drm-intel-next-queued, and I don't have a clang setup ready to confirm I didn't screw up anything. Can you pls rebase? Thanks a lot. -Daniel > --- > drivers/gpu/drm/i915/i915_irq.c| 10 +- > drivers/gpu/drm/i915/intel_display.c | 24 ++-- > drivers/gpu/drm/i915/intel_drv.h | 6 +++--- > drivers/gpu/drm/i915/intel_fifo_underrun.c | 6 +++--- > 4 files changed, 21 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 190f6aa5d15e..7960d2170750 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2132,10 +2132,10 @@ static void ibx_irq_handler(struct drm_i915_private > *dev_priv, u32 pch_iir) > DRM_DEBUG_DRIVER("PCH transcoder CRC error interrupt\n"); > > if (pch_iir & SDE_TRANSA_FIFO_UNDER) > - intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A); > + intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A); > > if (pch_iir & SDE_TRANSB_FIFO_UNDER) > - intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B); > + intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B); > } > > static void ivb_err_int_handler(struct drm_i915_private *dev_priv) > @@ -2169,13 +2169,13 @@ static void cpt_serr_int_handler(struct > drm_i915_private *dev_priv) > DRM_ERROR("PCH poison interrupt\n"); > > if (serr_int & SERR_INT_TRANS_A_FIFO_UNDERRUN) > - intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A); > + intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A); > > if (serr_int & SERR_INT_TRANS_B_FIFO_UNDERRUN) > - intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B); > + intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B); > > if (serr_int & SERR_INT_TRANS_C_FIFO_UNDERRUN) > - intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_C); > + intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_C); > > I915_WRITE(SERR_INT, serr_int); > } > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 9106ea32b048..21a8fea46ad9 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -1782,7 +1782,7 @@ static void lpt_enable_pch_transcoder(struct > drm_i915_private *dev_priv, > > /* FDI must be feeding us bits for PCH ports */ > assert_fdi_tx_enabled(dev_priv, (enum pipe) cpu_transcoder); > - assert_fdi_rx_enabled(dev_priv, TRANSCODER_A); > + assert_fdi_rx_enabled(dev_priv, PIPE_A); > > /* Workaround: set timing override bit. */ > val = I915_READ(TRANS_CHICKEN2(PIPE_A)); > @@ -1858,16 +1858,16 @@ void lpt_disable_pch_transcoder(struct > drm_i915_private *dev_priv) > I915_WRITE(TRANS_CHICKEN2(PIPE_A), val); > } > > -enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc) > +enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc) > { > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > WARN_ON(!crtc->config->has_pch_encoder); > > if (HAS_PCH_LPT(dev_priv)) > - return TRANSCODER_A; > + return PIPE_A; > else > - return (enum transcoder) crtc->pipe; > + return crtc->pipe; > } > > /** > @@ -1906,7 +1906,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc) > if (crtc->config->has_pch_encoder) { > /* if driving the PCH, we need FDI enabled */ > assert_fdi_rx_pll_enabled(dev_priv, > - (enum pipe) > intel_crtc_pch_transcoder(crtc)); > + > intel_crtc_pch_transcoder(crtc)); > assert_fdi_tx_pll_enabled(dev_priv, > (enum pipe) cpu_transcoder); > } > @@ -4573,7 +4573,7 @@ static void lpt_pch_enable(const struct > intel_crtc_state *crtc_state) > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > > - assert_pch_transcoder_dis
Re: [Intel-gfx] [PATCH v2 i-g-t] igt/perf: add tests to verify create/destroy userspace configs
On 14/07/17 19:21, Matthew Auld wrote: On 14 July 2017 at 18:57, Lionel Landwerlin wrote: On 14/07/17 18:09, Matthew Auld wrote: On 13 July 2017 at 12:16, Lionel Landwerlin wrote: v2: Add tests regarding removing configs (Matthew) Add tests regarding adding/removing configs without permissions (Matthew) Signed-off-by: Lionel Landwerlin --- tests/perf.c | 201 +++ 1 file changed, 201 insertions(+) I still think it would be nice to have at least have some flex tests, especially if we consider the issue with not reserving enough dwords, even if it's a pita... Well the current test were testing exactly that, and we didn't see any errors. Because in most cases, the back of the batchbuffer will just get rewritten by the next batch. Assuming we don't reserve enough dwords then intel_ring_advance() should catch this, it checks the advanced cs against the number of reserved dwords from intel_ring_begin(), so we should be hitting the GEM_BUG_ON(), unless I'm glossing over something. Do you have DEBUG_GEM enabled? Ah, that must be why... Apart from testing the if (reg >= xxx && reg =< yyy), I don't know that we can test anything interesting here :( I was thinking flex configs with n_regs < max_flex, and also n_regs > max_flex. Ok, will add n_regs > max_flex. n_regs < max_flex should be covered as we currently don't have any flex reg. diff --git a/tests/perf.c b/tests/perf.c index db28ba1f..47caf1c6 100644 --- a/tests/perf.c +++ b/tests/perf.c @@ -146,6 +146,36 @@ enum drm_i915_perf_record_type { }; #endif /* !DRM_I915_PERF_OPEN */ +#ifndef DRM_IOCTL_I915_PERF_ADD_CONFIG + +#define DRM_I915_PERF_ADD_CONFIG 0x37 +#define DRM_I915_PERF_REMOVE_CONFIG0x38 + +#define DRM_IOCTL_I915_PERF_ADD_CONFIG DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_PERF_ADD_CONFIG, struct drm_i915_perf_oa_config) +#define DRM_IOCTL_I915_PERF_REMOVE_CONFIG DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_PERF_REMOVE_CONFIG, __u64) + +/** + * Structure to upload perf dynamic configuration into the kernel. + */ +struct drm_i915_perf_oa_config { + /** String formatted like "%08x-%04x-%04x-%04x-%012x" */ + __u64 uuid; + + __u32 n_mux_regs; + __u32 pad0; + __u64 mux_regs; + + __u32 n_boolean_regs; + __u32 pad1; + __u64 boolean_regs; + + __u32 n_flex_regs; + __u32 pad2; + __u64 flex_regs; +}; + +#endif /* !DRM_IOCTL_I915_PERF_ADD_CONFIG */ + struct accumulator { #define MAX_RAW_OA_COUNTERS 62 enum drm_i915_oa_format format; @@ -4001,6 +4031,168 @@ test_rc6_disable(void) igt_assert_neq(n_events_end - n_events_start, 0); } +static void +test_invalid_userspace_config_create(void) +{ + struct drm_i915_perf_oa_config config; + const char *uuid = "01234567-0123-0123-0123-0123456789ab"; + const char *invalid_uuid = "blablabla-wrong"; + uint32_t mux_regs[] = { 0x9888 /* NOA_WRITE */, 0x0 }; + uint32_t invalid_mux_regs[] = { 0x12345678 /* invalid register */, 0x0 }; + + memset(&config, 0, sizeof(config)); + + /* invalid uuid */ + config.uuid = to_user_pointer(invalid_uuid); + config.n_mux_regs = 1; + config.mux_regs = to_user_pointer(mux_regs); + config.n_boolean_regs = 0; + config.n_flex_regs = 0; + + do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config, EINVAL); + + /* invalid mux_regs */ + config.uuid = to_user_pointer(uuid); + config.n_mux_regs = 1; + config.mux_regs = to_user_pointer(invalid_mux_regs); + config.n_boolean_regs = 0; + config.n_flex_regs = 0; + + do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config, EINVAL); + + /* empty config */ + config.uuid = to_user_pointer(uuid); + config.n_mux_regs = 0; + config.mux_regs = to_user_pointer(mux_regs); + config.n_boolean_regs = 0; + config.n_flex_regs = 0; + + do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config, EINVAL); + + /* empty config with null pointers */ + config.uuid = to_user_pointer(uuid); + config.n_mux_regs = 1; + config.mux_regs = to_user_pointer(NULL); + config.n_boolean_regs = 2; + config.boolean_regs = to_user_pointer(NULL); + config.n_flex_regs = 3; + config.flex_regs = to_user_pointer(NULL); + + do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config, EINVAL); +} + +static void +test_invalid_userspace_config_remove(void) +{ + struct drm_i915_perf_oa_config config; + const char *uuid = "01234567-0123-0123-0123-0123456789ab"; + uint32_t mux_regs[] = { 0x9888 /* NOA_WRITE */, 0x0 }; + uint64_t config_id, wrong_config_id = 9; + char path[512]; + int ret; + + snprintf(path, sizeof(path), "/sys/class/drm/card%d/metrics/%s/id", card, uuid); + + /* Destroy previous configuration if present */ +
[Intel-gfx] [PATCH 14/15] drm/i915: Disable per-engine reset for Broxton
Triggering a GPU reset for one engine affects another, notably corrupting the context status buffer (CSB) effectively losing track of inflight requests. Adding a few printks: diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index ad41836fa5e5..a969456bc0fa 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1953,6 +1953,7 @@ int i915_reset_engine(struct intel_engine_cs *engine) goto out; } + pr_err("Resetting %s\n", engine->name); ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine)); if (ret) { /* If we fail here, we expect to fallback to a global reset */ diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 716e5c9ea222..a72bc35d0870 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -355,6 +355,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN); port_set(&port[n], port_pack(rq, count)); desc = execlists_update_context(rq); + pr_err("%s: in (rq=%x) ctx=%d\n", engine->name, rq->global_seqno, upper_32_bits(desc)); GEM_DEBUG_EXEC(port[n].context_id = upper_32_bits(desc)); } else { GEM_BUG_ON(!n); @@ -594,9 +595,23 @@ static void intel_lrc_irq_handler(unsigned long data) if (!(status & GEN8_CTX_STATUS_COMPLETED_MASK)) continue; + pr_err("%s: out CSB (%x head=%d, tail=%d), ctx=%d, rq=%d\n", + engine->name, + readl(csb_mmio), + head, tail, + readl(buf+2*head+1), + port->context_id); + /* Check the context/desc id for this event matches */ - GEM_DEBUG_BUG_ON(readl(buf + 2 * head + 1) != -port->context_id); + if (readl(buf + 2 * head + 1) != port->context_id) { + pr_err("%s: BUG CSB (%x head=%d, tail=%d), ctx=%d, rq=%d\n", + engine->name, + readl(csb_mmio), + head, tail, + readl(buf+2*head+1), + port->context_id); + BUG(); + } rq = port_unpack(port, &count); GEM_BUG_ON(count == 0); Results in: [ 6423.006602] Resetting rcs0 [ 6423.009080] rcs0: in (rq=fe70) ctx=1 [ 6423.009216] rcs0: in (rq=fe6f) ctx=3 [ 6423.009542] rcs0: out CSB (2 head=1, tail=2), ctx=3, rq=3 [ 6423.009619] Resetting bcs0 [ 6423.009980] rcs0: BUG CSB (0 head=1, tail=2), ctx=0, rq=3 Note that this bug may be affect all machines and not just Broxton, Broxton is just the first machine on which I have confirmed this bug. Fixes: 142bc7d99bcf ("drm/i915: Modify error handler for per engine hang recovery") Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Michel Thierry --- drivers/gpu/drm/i915/i915_pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index a1e6b696bcfa..09d97e0990b7 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -398,6 +398,7 @@ static const struct intel_device_info intel_broxton_info = { GEN9_LP_FEATURES, .platform = INTEL_BROXTON, .ddb_size = 512, + .has_reset_engine = false, }; static const struct intel_device_info intel_geminilake_info = { -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/15] drm/i915: Check the execlist queue for pending requests before declaring idle
Including a check against the execlist queue before calling the engine idle and passing hangcheck. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_engine_cs.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index cba120f3d89d..fbac94557ffa 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1407,6 +1407,10 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine) if (port_request(&engine->execlist_port[0])) return false; + /* ELSP is empty, but there are ready requests? */ + if (READ_ONCE(engine->execlist_first)) + return false; + /* Ring stopped? */ if (!ring_is_idle(engine)) return false; -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/15] drm/i915: Assert that machine is wedged for nop_submit_request
We should only ever do nop_submit_request when the machine is wedged, so assert it is so. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ca7a56ff3904..5517373c1bea 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3089,6 +3089,7 @@ void i915_gem_reset_finish(struct drm_i915_private *dev_priv) static void nop_submit_request(struct drm_i915_gem_request *request) { + GEM_BUG_ON(!i915_terminally_wedged(&request->i915->gpu_error)); dma_fence_set_error(&request->fence, -EIO); i915_gem_request_submit(request); intel_engine_init_global_seqno(request->engine, request->global_seqno); -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/15] drm/i915: Wake up waiters after setting the WEDGED bit
After setting the WEDGED bit, make sure that we do wake up waiters as they may not be waiting for a request completion yet, just for its execution. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 40e94b4ef532..ca7a56ff3904 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3157,10 +3157,12 @@ static int __i915_gem_set_wedged_BKL(void *data) struct intel_engine_cs *engine; enum intel_engine_id id; - set_bit(I915_WEDGED, &i915->gpu_error.flags); for_each_engine(engine, i915, id) engine_set_wedged(engine); + set_bit(I915_WEDGED, &i915->gpu_error.flags); + wake_up_all(&i915->gpu_error.reset_queue); + return 0; } -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/15] drm/i915: Clear execlist port[] before updating seqno on wedging
When we wedge the device, we clear out the in-flight requests and advance the breadcrumb to indicate they are complete. However, the breadcrumb advance includes an assert that the engine is idle, so that advancement needs to be the last step to ensure we pass our own sanity checks. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 580b5042f4f7..40e94b4ef532 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3114,13 +3114,6 @@ static void engine_set_wedged(struct intel_engine_cs *engine) dma_fence_set_error(&request->fence, -EIO); spin_unlock_irqrestore(&engine->timeline->lock, flags); - /* Mark all pending requests as complete so that any concurrent -* (lockless) lookup doesn't try and wait upon the request as we -* reset it. -*/ - intel_engine_init_global_seqno(engine, - intel_engine_last_submit(engine)); - /* * Clear the execlists queue up before freeing the requests, as those * are the ones that keep the context and ringbuffer backing objects @@ -3149,6 +3142,13 @@ static void engine_set_wedged(struct intel_engine_cs *engine) */ clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted); } + + /* Mark all pending requests as complete so that any concurrent +* (lockless) lookup doesn't try and wait upon the request as we +* reset it. +*/ + intel_engine_init_global_seqno(engine, + intel_engine_last_submit(engine)); } static int __i915_gem_set_wedged_BKL(void *data) -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/15] drm/i915: Check execlist/ring status during hangcheck
Before we declare an engine as idle, check if there are any pending execlist context-switches and if the ring itself reports as idle. Otherwise, we may be left in a situation where we miss a crucial execlist event (or something more sinister) yet the requests complete. Since the seqno write happens, we believe the engine to be truly idle. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_hangcheck.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_hangcheck.c b/drivers/gpu/drm/i915/intel_hangcheck.c index 9b0ece427bdc..d9d87d96fb69 100644 --- a/drivers/gpu/drm/i915/intel_hangcheck.c +++ b/drivers/gpu/drm/i915/intel_hangcheck.c @@ -324,7 +324,7 @@ hangcheck_get_action(struct intel_engine_cs *engine, if (engine->hangcheck.seqno != hc->seqno) return ENGINE_ACTIVE_SEQNO; - if (i915_seqno_passed(hc->seqno, intel_engine_last_submit(engine))) + if (intel_engine_is_idle(engine)) return ENGINE_IDLE; return engine_stuck(engine, hc->acthd); -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/15] drm/i915: Clear engine irq posted following a reset
When the GPU is reset, we want to discard all pending notifications as either we have manually completed them, or they are no longer applicable. Make sure we do reset the engine->irq_posted prior to re-enabling the engine (e.g. the interrupt tasklets) in i915_gem_reset_finish_engine(). Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 5517373c1bea..19511020f06e 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3027,6 +3027,8 @@ static bool i915_gem_reset_request(struct drm_i915_gem_request *request) void i915_gem_reset_engine(struct intel_engine_cs *engine, struct drm_i915_gem_request *request) { + engine->irq_posted = 0; + if (request && i915_gem_reset_request(request)) { DRM_DEBUG_DRIVER("resetting %s to restart from tail of request 0x%x\n", engine->name, request->global_seqno); -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/15] drm/i915: Report execlists irq bit in debugfs
As part of the knowing whether there is outstanding data in the CSB, also check whether there is an outstanding IRQ notification. Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Tvrtko Ursulin Reviewed-by: Mika Kuoppala --- drivers/gpu/drm/i915/i915_debugfs.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 1130e3c83c88..5b09bd4d0483 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3395,10 +3395,12 @@ static int i915_engine_info(struct seq_file *m, void *unused) ptr = I915_READ(RING_CONTEXT_STATUS_PTR(engine)); read = GEN8_CSB_READ_PTR(ptr); write = GEN8_CSB_WRITE_PTR(ptr); - seq_printf(m, "\tExeclist CSB read %d [%d cached], write %d [%d from hws]\n", + seq_printf(m, "\tExeclist CSB read %d [%d cached], write %d [%d from hws], interrupt posted? %s\n", read, engine->csb_head, write, - intel_read_status_page(engine, intel_hws_csb_write_index(engine->i915))); + intel_read_status_page(engine, intel_hws_csb_write_index(engine->i915)), + yesno(test_bit(ENGINE_IRQ_EXECLIST, + &engine->irq_posted))); if (read >= GEN8_CSB_ENTRIES) read = 0; if (write >= GEN8_CSB_ENTRIES) -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/15] drm/i915: Move idle checks before intel_engine_init_global_seqno()
intel_engine_init_globa_seqno() may be called from an uncontrolled set-wedged path where we have given up waiting for broken hw and declare it defunct. Along that path, any sanity checks that the hw is idle before we adjust its state will expectedly fail, so we simply cannot. Instead of asserting inside init_global_seqno, we move them to the normal caller reset_all_global_seqno() as it handles runtime seqno wraparound. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem_request.c | 4 drivers/gpu/drm/i915/intel_engine_cs.c | 3 --- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 483af8921060..d93a185c0f0a 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -213,6 +213,10 @@ static int reset_all_global_seqno(struct drm_i915_private *i915, u32 seqno) cond_resched(); } + /* Check we are idle before we fiddle with hw state! */ + GEM_BUG_ON(!intel_engine_is_idle(engine)); + GEM_BUG_ON(i915_gem_active_isset(&engine->timeline->last_request)); + /* Finally reset hw state */ intel_engine_init_global_seqno(engine, seqno); tl->seqno = seqno; diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index fbac94557ffa..19297e6aab41 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -337,9 +337,6 @@ void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno) { struct drm_i915_private *dev_priv = engine->i915; - GEM_BUG_ON(!intel_engine_is_idle(engine)); - GEM_BUG_ON(i915_gem_active_isset(&engine->timeline->last_request)); - /* Our semaphore implementation is strictly monotonic (i.e. we proceed * so long as the semaphore value in the register/page is greater * than the sync value), so whenever we reset the seqno, -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/15] drm/i915: Serialize per-engine resets against new requests
We rely on disabling the execlists (by stopping the tasklet) to prevent new requests from submitting to the engine ELSP before we are ready. However, we re-enable the engine before we call init_hw which gives userspace the opportunity to subit a new request which is then overwritten by init_hw -- but not before the HW may have started executing. The subsequent out-of-order CSB is detected by our sanity checks in intel_lrc_irq_handler(). Fixes: a1ef70e14453 ("drm/i915: Add support for per engine reset recovery") Signed-off-by: Chris Wilson Cc: Michel Thierry Cc: Mika Kuoppala --- drivers/gpu/drm/i915/i915_drv.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 901c3ff61527..bc121a46ed9a 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1955,6 +1955,12 @@ int i915_reset_engine(struct intel_engine_cs *engine) } ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine)); + if (ret) { + /* If we fail here, we expect to fallback to a global reset */ + DRM_DEBUG_DRIVER("Failed to reset %s, ret=%d\n", +engine->name, ret); + goto out; + } /* * The request that caused the hang is stuck on elsp, we know the @@ -1963,15 +1969,6 @@ int i915_reset_engine(struct intel_engine_cs *engine) */ i915_gem_reset_engine(engine, active_request); - i915_gem_reset_finish_engine(engine); - - if (ret) { - /* If we fail here, we expect to fallback to a global reset */ - DRM_DEBUG_DRIVER("Failed to reset %s, ret=%d\n", -engine->name, ret); - goto out; - } - /* * The engine and its registers (and workarounds in case of render) * have been reset to their default values. Follow the init_ring @@ -1983,6 +1980,7 @@ int i915_reset_engine(struct intel_engine_cs *engine) error->reset_engine_count[engine->id]++; out: + i915_gem_reset_finish_engine(engine); return ret; } -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 13/15] drm/i915: Emit a user level message when resetting the GPU (or engine)
Although a banned context will be told to -EIO off if they try to submit more requests, we have a discrepancy between whole device resets and per-engine resets where we report the GPU reset but not the engine resets. This leaves a bit of mystery as to why the context was banned, and also reduces awareness overall of when a GPU (engine) reset occurs with its possible side-effects. Signed-off-by: Chris Wilson Cc: Michel Thierry Cc: Mika Kuoppala --- drivers/gpu/drm/i915/i915_drv.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index bc121a46ed9a..4b62fd012877 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1865,9 +1865,10 @@ void i915_reset(struct drm_i915_private *dev_priv) if (!i915_gem_unset_wedged(dev_priv)) goto wakeup; + dev_notice(dev_priv->drm.dev, + "Resetting chip after gpu hang\n"); error->reset_count++; - pr_notice("drm/i915: Resetting chip after gpu hang\n"); disable_irq(dev_priv->drm.irq); ret = i915_gem_reset_prepare(dev_priv); if (ret) { @@ -1945,7 +1946,9 @@ int i915_reset_engine(struct intel_engine_cs *engine) GEM_BUG_ON(!test_bit(I915_RESET_ENGINE + engine->id, &error->flags)); - DRM_DEBUG_DRIVER("resetting %s\n", engine->name); + dev_notice(engine->i915->drm.dev, + "Resetting %s after gpu hang\n", engine->name); + error->reset_engine_count[engine->id]++; active_request = i915_gem_reset_prepare_engine(engine); if (IS_ERR(active_request)) { @@ -1978,7 +1981,6 @@ int i915_reset_engine(struct intel_engine_cs *engine) if (ret) goto out; - error->reset_engine_count[engine->id]++; out: i915_gem_reset_finish_engine(engine); return ret; -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/15] drm/i915: Flush the execlist ports if idle
When doing a GPU reset, the CSB register will be trashed and we will lose any context-switch notifications that happened since the tasklet was disabled. If we find that all requests on this engine were completed, we want to make sure that the ELSP tracker is similarly empty so that we do not feed back in the completed requests upon recovering from the reset. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Mika Kuoppala --- drivers/gpu/drm/i915/intel_lrc.c | 36 ++-- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 3c83f2dd6798..ad61d1998fb7 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1327,6 +1327,31 @@ static void reset_common_ring(struct intel_engine_cs *engine, { struct execlist_port *port = engine->execlist_port; struct intel_context *ce; + unsigned int n; + + /* +* Catch up with any missed context-switch interrupts. +* +* Ideally we would just read the remaining CSB entries now that we +* know the gpu is idle. However, the CSB registers are sometimes^W +* often trashed across a GPU reset! Instead we have to rely on +* guessing the missed context-switch events by looking at what +* requests were completed. +*/ + if (!request) { + for (n = 0; n < ARRAY_SIZE(engine->execlist_port); n++) + i915_gem_request_put(port_request(&port[n])); + memset(engine->execlist_port, 0, sizeof(engine->execlist_port)); + return; + } + + if (request->ctx != port_request(port)->ctx) { + i915_gem_request_put(port_request(port)); + port[0] = port[1]; + memset(&port[1], 0, sizeof(port[1])); + } + + GEM_BUG_ON(request->ctx != port_request(port)->ctx); /* If the request was innocent, we leave the request in the ELSP * and will try to replay it on restarting. The context image may @@ -1338,7 +1363,7 @@ static void reset_common_ring(struct intel_engine_cs *engine, * and have to at least restore the RING register in the context * image back to the expected values to skip over the guilty request. */ - if (!request || request->fence.error != -EIO) + if (request->fence.error != -EIO) return; /* We want a simple context + ring to execute the breadcrumb update. @@ -1360,15 +1385,6 @@ static void reset_common_ring(struct intel_engine_cs *engine, request->ring->head = request->postfix; intel_ring_update_space(request->ring); - /* Catch up with any missed context-switch interrupts */ - if (request->ctx != port_request(port)->ctx) { - i915_gem_request_put(port_request(port)); - port[0] = port[1]; - memset(&port[1], 0, sizeof(port[1])); - } - - GEM_BUG_ON(request->ctx != port_request(port)->ctx); - /* Reset WaIdleLiteRestore:bdw,skl as well */ request->tail = intel_ring_wrap(request->ring, -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 12/15] drm/i915: Make i915_gem_context_mark_guilty() safe for unlocked updates
Since we make call i915_gem_context_mark_guilty() concurrently when resetting different engines in parallel, we need to make sure that our updates are safe for the unlocked access. Signed-off-by: Chris Wilson Cc: Michel Thierry Cc: Mika Kuoppala --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_gem.c | 32 ++-- drivers/gpu/drm/i915/i915_gem_context.c | 6 +++--- drivers/gpu/drm/i915/i915_gem_context.h | 6 +++--- drivers/gpu/drm/i915/i915_gem_request.c | 3 +-- drivers/gpu/drm/i915/i915_gpu_error.c | 8 6 files changed, 30 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b04347035a37..98705432ca59 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -602,7 +602,7 @@ struct drm_i915_file_private { * to limit the badly behaving clients access to gpu. */ #define I915_MAX_CLIENT_CONTEXT_BANS 3 - int context_bans; + atomic_t context_bans; }; /* Used by dp and fdi links */ diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 19511020f06e..82569f2a34ed 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2803,34 +2803,38 @@ i915_gem_object_pwrite_gtt(struct drm_i915_gem_object *obj, return 0; } -static bool ban_context(const struct i915_gem_context *ctx) +static bool ban_context(const struct i915_gem_context *ctx, + unsigned int score) { return (i915_gem_context_is_bannable(ctx) && - ctx->ban_score >= CONTEXT_SCORE_BAN_THRESHOLD); + score >= CONTEXT_SCORE_BAN_THRESHOLD); } static void i915_gem_context_mark_guilty(struct i915_gem_context *ctx) { - ctx->guilty_count++; - ctx->ban_score += CONTEXT_SCORE_GUILTY; - if (ban_context(ctx)) - i915_gem_context_set_banned(ctx); + unsigned int score; + bool banned; - DRM_DEBUG_DRIVER("context %s marked guilty (score %d) banned? %s\n", -ctx->name, ctx->ban_score, -yesno(i915_gem_context_is_banned(ctx))); + atomic_inc(&ctx->guilty_count); - if (!i915_gem_context_is_banned(ctx) || IS_ERR_OR_NULL(ctx->file_priv)) + score = atomic_add_return(CONTEXT_SCORE_GUILTY, &ctx->ban_score); + banned = ban_context(ctx, score); + DRM_DEBUG_DRIVER("context %s marked guilty (score %d) banned? %s\n", +ctx->name, score, yesno(banned)); + if (!banned) return; - ctx->file_priv->context_bans++; - DRM_DEBUG_DRIVER("client %s has had %d context banned\n", -ctx->name, ctx->file_priv->context_bans); + i915_gem_context_set_banned(ctx); + if (!IS_ERR_OR_NULL(ctx->file_priv)) { + atomic_inc(&ctx->file_priv->context_bans); + DRM_DEBUG_DRIVER("client %s has had %d context banned\n", +ctx->name, atomic_read(&ctx->file_priv->context_bans)); + } } static void i915_gem_context_mark_innocent(struct i915_gem_context *ctx) { - ctx->active_count++; + atomic_inc(&ctx->active_count); } struct drm_i915_gem_request * diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 1a87d04e7937..ed91ac8ca832 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -977,7 +977,7 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv) static bool client_is_banned(struct drm_i915_file_private *file_priv) { - return file_priv->context_bans > I915_MAX_CLIENT_CONTEXT_BANS; + return atomic_read(&file_priv->context_bans) > I915_MAX_CLIENT_CONTEXT_BANS; } int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, @@ -1179,8 +1179,8 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev, else args->reset_count = 0; - args->batch_active = READ_ONCE(ctx->guilty_count); - args->batch_pending = READ_ONCE(ctx->active_count); + args->batch_active = atomic_read(&ctx->guilty_count); + args->batch_pending = atomic_read(&ctx->active_count); ret = 0; out: diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h index 04320f80f9f4..2d02918a449e 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.h +++ b/drivers/gpu/drm/i915/i915_gem_context.h @@ -191,17 +191,17 @@ struct i915_gem_context { u32 desc_template; /** guilty_count: How many times this context has caused a GPU hang. */ - unsigned int guilty_count; + atomic_t guilty_count; /** * @active_count: How many times this context was active during a GPU * hang, but did not cause it. */ - unsigned int active_count; + atomic_
[Intel-gfx] [PATCH 02/15] drm/i915: Reset context image on engines after triggering the reset
We try to fixup the context image after the reset to ensure that there are no more pending writes from the hw that may conflict and to fixup any that were in flight. Fixes: a1ef70e14453 ("drm/i915: Add support for per engine reset recovery") Signed-off-by: Chris Wilson Cc: Michel Thierry Cc: Tvrtko Ursulin Cc: Mika Kuoppala Reviewed-by: Mika Kuoppala --- drivers/gpu/drm/i915/i915_drv.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index acd0c8b86f9d..901c3ff61527 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1954,6 +1954,8 @@ int i915_reset_engine(struct intel_engine_cs *engine) goto out; } + ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine)); + /* * The request that caused the hang is stuck on elsp, we know the * active request and can drop it, adjust head to skip the offending @@ -1961,9 +1963,6 @@ int i915_reset_engine(struct intel_engine_cs *engine) */ i915_gem_reset_engine(engine, active_request); - /* Finally, reset just this engine. */ - ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine)); - i915_gem_reset_finish_engine(engine); if (ret) { -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 15/15] drm/i915/selftests: Exercise independence of per-engine resets
If all goes well, resetting one engine should not affect the operation of any others. So to test this, we setup a continuous stream of requests onto to each of the "innocent" engines whilst constantly resetting our target engine. Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Michel Thierry --- drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 165 +++ drivers/gpu/drm/i915/selftests/mock_context.c| 8 ++ drivers/gpu/drm/i915/selftests/mock_context.h| 3 + 3 files changed, 176 insertions(+) diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c index 7096c3911cd3..dbfcb31ba9f4 100644 --- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c +++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c @@ -22,8 +22,13 @@ * */ +#include + #include "../i915_selftest.h" +#include "mock_context.h" +#include "mock_drm.h" + struct hang { struct drm_i915_private *i915; struct drm_i915_gem_object *hws; @@ -372,6 +377,165 @@ static int igt_reset_engine(void *arg) return err; } +static int active_engine(void *data) +{ + struct intel_engine_cs *engine = data; + struct drm_i915_gem_request *rq[2] = {}; + struct i915_gem_context *ctx[2]; + struct drm_file *file; + unsigned long count = 0; + int err = 0; + + file = mock_file(engine->i915); + if (IS_ERR(file)) + return PTR_ERR(file); + + mutex_lock(&engine->i915->drm.struct_mutex); + ctx[0] = live_context(engine->i915, file); + mutex_unlock(&engine->i915->drm.struct_mutex); + if (IS_ERR(ctx[0])) { + err = PTR_ERR(ctx[0]); + goto err_file; + } + + mutex_lock(&engine->i915->drm.struct_mutex); + ctx[1] = live_context(engine->i915, file); + mutex_unlock(&engine->i915->drm.struct_mutex); + if (IS_ERR(ctx[1])) { + err = PTR_ERR(ctx[1]); + i915_gem_context_put(ctx[0]); + goto err_file; + } + + while (!kthread_should_stop()) { + unsigned int idx = count++ & 1; + struct drm_i915_gem_request *old = rq[idx]; + struct drm_i915_gem_request *new; + + mutex_lock(&engine->i915->drm.struct_mutex); + new = i915_gem_request_alloc(engine, ctx[idx]); + if (IS_ERR(new)) { + mutex_unlock(&engine->i915->drm.struct_mutex); + err = PTR_ERR(new); + break; + } + + rq[idx] = i915_gem_request_get(new); + i915_add_request(new); + mutex_unlock(&engine->i915->drm.struct_mutex); + + if (old) { + i915_wait_request(old, 0, MAX_SCHEDULE_TIMEOUT); + i915_gem_request_put(old); + } + } + + for (count = 0; count < ARRAY_SIZE(rq); count++) + i915_gem_request_put(rq[count]); + +err_file: + mock_file_free(engine->i915, file); + return err; +} + +static int igt_reset_active_engines(void *arg) +{ + struct drm_i915_private *i915 = arg; + struct intel_engine_cs *engine, *active; + enum intel_engine_id id, tmp; + int err = 0; + + /* Check that issuing a reset on one engine does not interfere +* with any other engine. +*/ + + if (!intel_has_reset_engine(i915)) + return 0; + + for_each_engine(engine, i915, id) { + struct task_struct *threads[I915_NUM_ENGINES]; + unsigned long resets[I915_NUM_ENGINES]; + unsigned long global = i915_reset_count(&i915->gpu_error); + IGT_TIMEOUT(end_time); + + memset(threads, 0, sizeof(threads)); + for_each_engine(active, i915, tmp) { + struct task_struct *tsk; + + if (active == engine) + continue; + + resets[tmp] = i915_reset_engine_count(&i915->gpu_error, + active); + + tsk = kthread_run(active_engine, active, + "igt/%s", active->name); + if (IS_ERR(tsk)) { + err = PTR_ERR(tsk); + goto unwind; + } + + threads[tmp] = tsk; + get_task_struct(tsk); + + } + + set_bit(I915_RESET_ENGINE + engine->id, &i915->gpu_error.flags); + do { + err = i915_reset_engine(engine); + if (err) { + pr_err("i915_reset_engine(%s) failed, err=%d\n", + engine->name, err); + break; +
Re: [Intel-gfx] [PATCH] drm/atomic-helper: Fix leak in disable_all
Op 15-07-17 om 11:31 schreef Daniel Vetter: > The legacy plane->fb pointer is refcounted by calling > drm_atomic_clean_old_fb(). > > In practice this isn't a real problem because: > - The caller in the i915 gpu reset code restores the original state > again, which means the plane->fb pointer won't change, hence can't > leak. > - Drivers using drm_atomic_helper_shutdown call the fbdev cleanup > first, and that usually cleans up the fb through > drm_remove_framebuffer, which does this correctly. > - Without fbdev the only framebuffers are from userspace, and those > get cleaned up (again using drm_remove_framebuffer) befor the driver > can even be unloaded. > > But in i915 I've switched the cleanup sequence around so that the > _shutdown() calls happens after the drm_remove_framebuffer(), which is > how I discovered this issue. > > v2: My analysis why the current code was ok for gpu reset and > suspend/resume was correct, but then I totally failed to realize that > we better keep this symmetric. Thanksfully CI noticed that for > balance, a refcounting bug must exist at 2 places if previously there > was no issue ... > > v3: Don't be lazy and compute the plane_mask in > commit_duplicated_state properly too, instead of just using ~0U. > > Cc: martin.pe...@free.fr > Cc: ch...@chris-wilson.co.uk > Acked-by: Dave Airlie (v1) > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_atomic_helper.c | 18 -- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index b07fc30372d3..545328a9a769 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2726,6 +2726,7 @@ int drm_atomic_helper_disable_all(struct drm_device > *dev, > struct drm_plane *plane; > struct drm_crtc_state *crtc_state; > struct drm_crtc *crtc; > + unsigned plane_mask = 0; > int ret, i; > > state = drm_atomic_state_alloc(dev); > @@ -2768,10 +2769,14 @@ int drm_atomic_helper_disable_all(struct drm_device > *dev, > goto free; > > drm_atomic_set_fb_for_plane(plane_state, NULL); > + plane_mask |= BIT(drm_plane_index(plane)); > + plane->old_fb = plane->fb; > } > > ret = drm_atomic_commit(state); > free: > + if (plane_mask) > + drm_atomic_clean_old_fb(dev, plane_mask, ret); > drm_atomic_state_put(state); > return ret; > } > @@ -2902,11 +2907,16 @@ int drm_atomic_helper_commit_duplicated_state(struct > drm_atomic_state *state, > struct drm_connector_state *new_conn_state; > struct drm_crtc *crtc; > struct drm_crtc_state *new_crtc_state; > + unsigned plane_mask = 0; > + struct drm_device *dev = state->dev; > + int ret; > > state->acquire_ctx = ctx; > > - for_each_new_plane_in_state(state, plane, new_plane_state, i) > + for_each_new_plane_in_state(state, plane, new_plane_state, i) { > + plane_mask |= BIT(drm_plane_index(plane)); We should really add a drm_plane_mask/drm_connector_mask at some point, to complement drm_crtc_mask. > state->planes[i].old_state = plane->state; > + } > > for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) > state->crtcs[i].old_state = crtc->state; > @@ -2914,7 +2924,11 @@ int drm_atomic_helper_commit_duplicated_state(struct > drm_atomic_state *state, > for_each_new_connector_in_state(state, connector, new_conn_state, i) > state->connectors[i].old_state = connector->state; > > - return drm_atomic_commit(state); > + ret = drm_atomic_commit(state); > + if (plane_mask) > + drm_atomic_clean_old_fb(dev, plane_mask, ret); Kill the if () part, and make it unconditional? On second thought, I should have done the same in drm_framebuffer.c > + return ret; > } > EXPORT_SYMBOL(drm_atomic_helper_commit_duplicated_state); > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 4/4] drm/i915/execlists: Preemption!
Quoting Chris Wilson (2017-07-17 09:42:35) > @@ -503,6 +500,49 @@ static void execlists_dequeue(struct intel_engine_cs > *engine) > struct i915_priolist *p = rb_entry(rb, typeof(*p), node); > struct drm_i915_gem_request *rq, *rn; > > + if (once) { > + if (port_count(&port[0]) > 1) > + goto done; > + > + if (p->priority > max(last->priotree.priority, 0)) { > + list_for_each_entry_safe_reverse(rq, rn, > + > &engine->timeline->requests, > +link) { > + struct i915_priolist *p; > + > + if (i915_gem_request_completed(rq)) > + break; > + > + __i915_gem_request_unsubmit(rq); > + unwind_wa_tail(rq); Fwiw, this is the flaw in this approach. As we decide to move the request back to the execution queue, it may complete. If we try to reexecute it, its ring->tail will be less than RING_HEAD, telling the hw to execute everything after it again. Michal's approach was to use a preemptive switch to a dummy context and then once hw knew the hw wasn't executing any of the other requests, he would unsubmit them and recompute the desired order. I've yet to see another solution for a cheaper barrier between hw/sw, as otherwise we must deliberately insert a stall to do preemption. :| -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] drivers/gpu/drm/i915/intel_pm.c:4467: bad comparison ?
On Mon, 17 Jul 2017, David Binderman wrote: > Hello there, Hello. No need to include LKML for stuff like this. But Cc'd the folks from the broken commit. > drivers/gpu/drm/i915/intel_pm.c:4467]: (warning) Comparison of a boolean > expression with an integer other than 0 or 1. > > Source code is > > else if ((ddb_allocation && ddb_allocation / > fixed_16_16_to_u32_round_up(plane_blocks_per_line)) >= 1) Broken by commit d555cb5827d603244969e08444340e3db78c8a37 Author: Kumar, Mahesh Date: Wed May 17 17:28:29 2017 +0530 drm/i915/skl+: use linetime latency if ddb size is not available The broken code has since been removed by bb9d85f6e9de ("drm/i915/skl: New ddb allocation algorithm") but restored by 9a30a26122c3 ("Revert "drm/i915/skl: New ddb allocation algorithm""). *sigh*. Mahesh et al, please figure it out. 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] ✗ Fi.CI.BAT: warning for RFC: duct-tape for the gpu reset vs. modeset deadlock
Op 15-07-17 om 15:45 schreef Daniel Vetter: > On Sat, Jul 15, 2017 at 3:23 PM, Daniel Vetter wrote: >> On Sat, Jul 15, 2017 at 2:00 PM, Patchwork >> wrote: >>> == Series Details == >>> >>> Series: RFC: duct-tape for the gpu reset vs. modeset deadlock >>> URL : https://patchwork.freedesktop.org/series/27345/ >>> State : warning >>> >>> == Summary == >>> >>> Series 27345v1 RFC: duct-tape for the gpu reset vs. modeset deadlock >>> https://patchwork.freedesktop.org/api/1.0/series/27345/revisions/1/mbox/ >>> >>> Test drv_hangman: >>> Subgroup error-state-basic: >>> pass -> DMESG-WARN (fi-blb-e6850) >>> pass -> DMESG-WARN (fi-pnv-d510) >>> Test gem_busy: >>> Subgroup basic-hang-default: >>> pass -> DMESG-WARN (fi-blb-e6850) >>> pass -> DMESG-WARN (fi-pnv-d510) >>> Test gem_exec_fence: >>> Subgroup await-hang-default: >>> pass -> DMESG-WARN (fi-blb-e6850) >>> pass -> DMESG-WARN (fi-pnv-d510) >> Looks like I wreaked something on gpus where the hw reset kills the >> display. I'll check it out. > Ok, simplest solution seems to be to just not do the special casing > between real hw reset and fake reset, and unconditionally shut down > all the CRTC. Probably a lot more than just the power domain tracking > will get out of whack. I wasn't really sold on that idea either way, > so I'll drop both "drm/i915: only disable outputs in simulated gpu > resets" and "drm/atomic-helper: add sync_all helper for gpu reset" > from the series. They're not needed really anyway. > -Daniel Agreed, sync_all helper should have been done by the disable_all call. The thing I didn't like was that you didn't shut down the GPU unconditionally, but with that fixed series looks good. :) I like patch 6/7, it makes everything more clear. I'm not sure if calling prepare_to_wait twice is allowed, but don't see why not. So with those taken out and feedback addressed. Reviewed-by: Maarten Lankhorst ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] drivers/gpu/drm/i915/intel_pm.c:4467: bad comparison ?
Hi, On Monday 17 July 2017 03:22 PM, Jani Nikula wrote: On Mon, 17 Jul 2017, David Binderman wrote: Hello there, Hello. No need to include LKML for stuff like this. But Cc'd the folks from the broken commit. drivers/gpu/drm/i915/intel_pm.c:4467]: (warning) Comparison of a boolean expression with an integer other than 0 or 1. Source code is else if ((ddb_allocation && ddb_allocation / fixed_16_16_to_u32_round_up(plane_blocks_per_line)) >= 1) ddb_allocation being integer was intentional. Other than that code has improper parentheses as well. intention was if ddb_allocation is not 0 & (ddb_allocation / plane_blocks_per_line >= 1) then execute the condition. it should have been else if (ddb_allocation && (ddb_allocation / fixed_16_16_to_u32_round_up(plane_blocks_per_line) >= 1)) will post a fix. thanks. -Mahesh Broken by commit d555cb5827d603244969e08444340e3db78c8a37 Author: Kumar, Mahesh Date: Wed May 17 17:28:29 2017 +0530 drm/i915/skl+: use linetime latency if ddb size is not available The broken code has since been removed by bb9d85f6e9de ("drm/i915/skl: New ddb allocation algorithm") but restored by 9a30a26122c3 ("Revert "drm/i915/skl: New ddb allocation algorithm""). *sigh*. Mahesh et al, please figure it out. BR, Jani. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [i-g-t] many testcases skiped
Hi~ Hiler Thank for your reply. I'm sorry for didn't notice the integradted graphics. But there is still some cases could not pass and shown requirement not met, running on i7-4770. I wonder if these skiped cases only about the cpu? So different processor must have different test result. Or can we do something to meet the requirements and let all case pass. Logs show below. /igt/tests# ./pm_backlight IGT-Version: 1.19-g4258cc8e (x86_64) (Linux: 4.10.0 x86_64) Test requirement not met in function __real_main154, file pm_backlight.c:163: Test requirement: !(backlight_read(&old, "brightness")) Last errno: 2, No such file or directory Subtest basic-brightness: SKIP Subtest bad-brightness: SKIP Subtest fade: SKIP /igt/tests# ./kms_force_connector_basic IGT-Version: 1.19-g4258cc8e (x86_64) (Linux: 4.10.0 x86_64) Test requirement not met in function main, file kms_force_connector_basic.c:111: Test requirement: !(vga_connector->connection == DRM_MODE_CONNECTED) Last errno: 2, No such file or directory Subtest force-load-detect: SKIP Subtest force-connector-state: SKIP Subtest force-edid: SKIP Subtest prune-stale-modes: SKIP /igt/tests# ./gem_exec_basic IGT-Version: 1.19-g4258cc8e (x86_64) (Linux: 4.10.0 x86_64) Subtest basic-default: SUCCESS (0.000s) Subtest readonly-default: SUCCESS (0.000s) Subtest gtt-default: SUCCESS (0.000s) Subtest basic-render: SUCCESS (0.000s) Subtest readonly-render: SUCCESS (0.000s) Subtest gtt-render: SUCCESS (0.000s) Subtest basic-bsd: SUCCESS (0.000s) Subtest readonly-bsd: SUCCESS (0.000s) Subtest gtt-bsd: SUCCESS (0.000s) Test requirement not met in function gem_require_ring, file ioctl_wrappers.c:1591: Test requirement: gem_has_ring(fd, ring) Subtest basic-bsd1: SKIP (0.000s) Test requirement not met in function gem_require_ring, file ioctl_wrappers.c:1591: Test requirement: gem_has_ring(fd, ring) Subtest readonly-bsd1: SKIP (0.000s) Test requirement not met in function gem_require_ring, file ioctl_wrappers.c:1591: Test requirement: gem_has_ring(fd, ring) Subtest gtt-bsd1: SKIP (0.000s) Test requirement not met in function gem_require_ring, file ioctl_wrappers.c:1591: Test requirement: gem_has_ring(fd, ring) Subtest basic-bsd2: SKIP (0.000s) Test requirement not met in function gem_require_ring, file ioctl_wrappers.c:1591: Test requirement: gem_has_ring(fd, ring) Subtest readonly-bsd2: SKIP (0.000s) Test requirement not met in function gem_require_ring, file ioctl_wrappers.c:1591: Test requirement: gem_has_ring(fd, ring) Subtest gtt-bsd2: SKIP (0.000s) Subtest basic-blt: SUCCESS (0.000s) Subtest readonly-blt: SUCCESS (0.000s) Subtest gtt-blt: SUCCESS (0.000s) Subtest basic-vebox: SUCCESS (0.000s) Subtest readonly-vebox: SUCCESS (0.000s) Subtest gtt-vebox: SUCCESS (0.000s) /igt/tests#./kms_sink_crc_basic IGT-Version: 1.19-g4258cc8e (x86_64) (Linux: 4.10.0 x86_64) no eDP with CRC support found SKIP (0.328s) Thank you Gu > -Original Message- > From: Hiler, Arkadiusz > Sent: Monday, July 17, 2017 4:30 PM > To: Gu, HailinX > Cc: intel-gfx@lists.freedesktop.org; Li, ZhijianX ; Li, > Philip > Subject: Re: [Intel-gfx] [i-g-t] many testcases skiped > > On Mon, Jul 17, 2017 at 11:14:03AM +0300, Arkadiusz Hiler wrote: > > On Mon, Jul 17, 2017 at 12:28:00PM +0800, Hailin Gu wrote: > > > Hi~ all, > > > > > > Does the test program have any hardware or version limitations? > > > > > > > > > I found that most testcase could run on a machine with intel(R) > > > Core(TM) i7-2600K CPU @ 3.40GHz. > > > > > > But can't run on Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz or > > > Intel(R) > > > Core(TM) i3-2120 CPU @ 3.30GHz. > > > > Hey, > > > > The hardware limitation is that you should have integradted graphics. > > And by "limitation" I've meant "requirement". At least when it comes to Intel > GPUs. > > > E5-2669 v3 does not have one according to ark.intel.com. > > > > Error message seems to be pretty clear, stating: "No known gpu found". > > > > As of the i3 - it probably should work. Can you share complete logs > > from that machinge to see what goes wrong there? > > > > -- > > Cheers, > > Arek > > > > > Thank you > > > > > > *From:*Hailin Gu [mailto:hailinx...@intel.com] > > > *Sent:* Tuesday, July 11, 2017 7:51 PM > > > *To:* intel-gfx@lists.freedesktop.org > > > *Cc:* Li, ZhijianX ; Li, Philip > > > > > > *Subject:* [i-g-t] many testcases skiped > > > > > > Hi~ all, > > > > > > I want to run the testcases of igt, but there are some problems during > that. > > > Lots of testcase started with gem and kms skiped. > > > > > > Thanks for your helping. > > > > > > I make the piglit and igt by following steps > > > > > > $cd piglit > > > > > > $cmake . > > > > > > $male > > > > > > $make install > > > > > > $cd intel-gpu-tools > > > > > > $./autogen > > > > > > $make > > > > > > All of these steps are successful. And I write the path of igt in > > > piglit.conf. running the tests by calling > > > > > > "./piglit run igt -t gem_sync /tmp" > >
Re: [Intel-gfx] drivers/gpu/drm/i915/intel_pm.c:4467: bad comparison ?
Op 17-07-17 om 12:32 schreef Mahesh Kumar: > Hi, > > > On Monday 17 July 2017 03:22 PM, Jani Nikula wrote: >> On Mon, 17 Jul 2017, David Binderman wrote: >>> Hello there, >> Hello. No need to include LKML for stuff like this. But Cc'd the folks >> from the broken commit. >> >>> drivers/gpu/drm/i915/intel_pm.c:4467]: (warning) Comparison of a boolean >>> expression with an integer other than 0 or 1. >>> >>> Source code is >>> >>> else if ((ddb_allocation && ddb_allocation / >>> fixed_16_16_to_u32_round_up(plane_blocks_per_line)) >= 1) > ddb_allocation being integer was intentional. > Other than that code has improper parentheses as well. > intention was if ddb_allocation is not 0 & (ddb_allocation / > plane_blocks_per_line >= 1) then execute the condition. > it should have been > else if (ddb_allocation && (ddb_allocation / > fixed_16_16_to_u32_round_up(plane_blocks_per_line) >= 1)) > > will post a fix. > > thanks. > > -Mahesh >> Broken by >> >> commit d555cb5827d603244969e08444340e3db78c8a37 >> Author: Kumar, Mahesh >> Date: Wed May 17 17:28:29 2017 +0530 >> >> drm/i915/skl+: use linetime latency if ddb size is not available >> >> The broken code has since been removed by bb9d85f6e9de ("drm/i915/skl: >> New ddb allocation algorithm") but restored by 9a30a26122c3 ("Revert >> "drm/i915/skl: New ddb allocation algorithm""). *sigh*. >> >> Mahesh et al, please figure it out. >> >> >> BR, >> Jani. >> >> > Would this work? diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 78b9acfc64c0..b9b3d8d45016 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4681,8 +4681,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, if ((cpp * cstate->base.adjusted_mode.crtc_htotal / 512 < 1) && (plane_bytes_per_line / 512 < 1)) selected_result = method2; - else if ((ddb_allocation && ddb_allocation / - fixed_16_16_to_u32_round_up(plane_blocks_per_line)) >= 1) + else if (ddb_allocation >= +fixed_16_16_to_u32_round_up(plane_blocks_per_line)) selected_result = min_fixed_16_16(method1, method2); else if (latency >= linetime_us) selected_result = min_fixed_16_16(method1, method2); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] drivers/gpu/drm/i915/intel_pm.c:4467: bad comparison ?
Hi, On Monday 17 July 2017 04:01 PM, Maarten Lankhorst wrote: Op 17-07-17 om 12:32 schreef Mahesh Kumar: Hi, On Monday 17 July 2017 03:22 PM, Jani Nikula wrote: On Mon, 17 Jul 2017, David Binderman wrote: Hello there, Hello. No need to include LKML for stuff like this. But Cc'd the folks from the broken commit. drivers/gpu/drm/i915/intel_pm.c:4467]: (warning) Comparison of a boolean expression with an integer other than 0 or 1. Source code is else if ((ddb_allocation && ddb_allocation / fixed_16_16_to_u32_round_up(plane_blocks_per_line)) >= 1) ddb_allocation being integer was intentional. Other than that code has improper parentheses as well. intention was if ddb_allocation is not 0 & (ddb_allocation / plane_blocks_per_line >= 1) then execute the condition. it should have been else if (ddb_allocation && (ddb_allocation / fixed_16_16_to_u32_round_up(plane_blocks_per_line) >= 1)) will post a fix. thanks. -Mahesh Broken by commit d555cb5827d603244969e08444340e3db78c8a37 Author: Kumar, Mahesh Date: Wed May 17 17:28:29 2017 +0530 drm/i915/skl+: use linetime latency if ddb size is not available The broken code has since been removed by bb9d85f6e9de ("drm/i915/skl: New ddb allocation algorithm") but restored by 9a30a26122c3 ("Revert "drm/i915/skl: New ddb allocation algorithm""). *sigh*. Mahesh et al, please figure it out. BR, Jani. Would this work? diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 78b9acfc64c0..b9b3d8d45016 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4681,8 +4681,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, if ((cpp * cstate->base.adjusted_mode.crtc_htotal / 512 < 1) && (plane_bytes_per_line / 512 < 1)) selected_result = method2; - else if ((ddb_allocation && ddb_allocation / - fixed_16_16_to_u32_round_up(plane_blocks_per_line)) >= 1) + else if (ddb_allocation >= +fixed_16_16_to_u32_round_up(plane_blocks_per_line)) yes, this would even simplify the condition :) -Mahesh selected_result = min_fixed_16_16(method1, method2); else if (latency >= linetime_us) selected_result = min_fixed_16_16(method1, method2); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Don't complain too much about struct_mutex.
On Sat, Jul 15, 2017 at 11:53:28AM +0200, Daniel Vetter wrote: > A more complete solution would be to do the mutex_init in the drm core > only for legacy drivers, plus add it to each modern driver that still > needs it, which would also give each its own lockdep key. Trying to do > that dynamically doesn't work, because lockdep requires it's keys to > be statically allocated. Would something like the below work? Its not pretty, but would give each user of drm_dev_init() its own key. diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 37b8ad3e30d8..06cc32012728 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -478,9 +478,10 @@ static void drm_fs_inode_free(struct inode *inode) * RETURNS: * 0 on success, or error code on failure. */ -int drm_dev_init(struct drm_device *dev, -struct drm_driver *driver, -struct device *parent) +int __drm_dev_init(struct drm_device *dev, + struct drm_driver *driver, + struct device *parent, + struct lock_class_key *key) { int ret; @@ -496,7 +497,7 @@ int drm_dev_init(struct drm_device *dev, spin_lock_init(&dev->buf_lock); spin_lock_init(&dev->event_lock); - mutex_init(&dev->struct_mutex); + __mutex_init(&dev->struct_mutex, "&dev->struct_mutex", key); mutex_init(&dev->filelist_mutex); mutex_init(&dev->ctxlist_mutex); mutex_init(&dev->master_mutex); diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 53b98321df9b..cda11e97024e 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -531,9 +531,17 @@ void drm_printk(const char *level, unsigned int category, const char *format, ...); extern unsigned int drm_debug; -int drm_dev_init(struct drm_device *dev, -struct drm_driver *driver, -struct device *parent); +int __drm_dev_init(struct drm_device *dev, + struct drm_driver *driver, + struct device *parent, + struct lock_class_key *key); + +#define drm_dev_init(_dev, _driver, _parent) \ +({ \ + static struct lock_class_key __key; \ + __drm_dev_init((_dev), (_driver), (_parent), &__key); \ +}) + void drm_dev_fini(struct drm_device *dev); struct drm_device *drm_dev_alloc(struct drm_driver *driver, ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v10] vfio: ABI for mdev display dma-buf operation
Hi, > No need of flag here. If vGPU driver is not loaded in the guest, > there > is no surface being managed by vGPU, in that case this size will be > zero. Ok, we certainly have the same situation with intel. When the guest driver is not loaded (yet) there is no valid surface. We should cleanly define what the ioctl should do in that case, so all drivers behave the same way. I'd suggest that all fields defining the surface (drm_format, width, height, stride, size) should be set to zero in that case. cheers, Gerd ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix bad comparison in skl_compute_plane_wm.
ddb_allocation && ddb_allocation / blocks_per_line >= 1 is the same as ddb_allocation >= blocks_per_line, so use the latter to simplify this. This fixes the following compiler warning: drivers/gpu/drm/i915/intel_pm.c:4467]: (warning) Comparison of a boolean expression with an integer other than 0 or 1. Signed-off-by: Maarten Lankhorst Fixes: d555cb5827d6 ("drm/i915/skl+: use linetime latency if ddb size is not available") Cc: "Mahesh Kumar" Reported-by: David Binderman Cc: David Binderman Cc: # v4.13-rc1+ --- drivers/gpu/drm/i915/intel_pm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 78b9acfc64c0..b9b3d8d45016 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4681,8 +4681,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, if ((cpp * cstate->base.adjusted_mode.crtc_htotal / 512 < 1) && (plane_bytes_per_line / 512 < 1)) selected_result = method2; - else if ((ddb_allocation && ddb_allocation / - fixed_16_16_to_u32_round_up(plane_blocks_per_line)) >= 1) + else if (ddb_allocation >= +fixed_16_16_to_u32_round_up(plane_blocks_per_line)) selected_result = min_fixed_16_16(method1, method2); else if (latency >= linetime_us) selected_result = min_fixed_16_16(method1, method2); -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Fix error checking/locking in perf/lookup_context()
On Fri, Jul 14, 2017 at 03:28:47PM +, Patchwork wrote: > == Series Details == > > Series: series starting with [1/2] drm/i915: Fix error checking/locking in > perf/lookup_context() > URL : https://patchwork.freedesktop.org/series/27309/ > State : success I pushed both patches to -dinq, thanks for the review. --Imre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Eliminate dead code in intel_sanitize_enable_ppgtt()
On Wed, Jun 01, 2016 at 06:04:40PM +0100, Chris Wilson wrote: > On Wed, Jun 01, 2016 at 05:01:13PM +0100, Damien Lespiau wrote: > > We exit early if has_aliasing_ppgtt is 0, so towards the end of the > > function has_aliasing_ppgtt can only be 1. > > > > Also: > > > > if (foo) > > return 1; > > else > > return 0; > > > > when foo is already a bool is really just: > > > > return foo; > > > > Signed-off-by: Damien Lespiau > > --- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > > b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index 4668477..6aae4b8 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -161,7 +161,7 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private > > *dev_priv, > > if (INTEL_GEN(dev_priv) >= 8 && i915.enable_execlists) > > return has_full_48bit_ppgtt ? 3 : 2; > > else > > - return has_aliasing_ppgtt ? 1 : 0; > > + return has_aliasing_ppgtt; > > /* full-ppgtt doesn't yet work reliably in legacy ringbuffer mode */ > if (!i915.enable_execlists) > return 1; > > return has_full_48bit_ppgtt ? 3 : 2; This fell through the cracks. We'd need it to improve the output from static checkers. --Imre > > -- > 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 mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix bad comparison in skl_compute_plane_wm.
Reviewed-by: Mahesh Kumar On Monday 17 July 2017 04:43 PM, Maarten Lankhorst wrote: ddb_allocation && ddb_allocation / blocks_per_line >= 1 is the same as ddb_allocation >= blocks_per_line, so use the latter to simplify this. This fixes the following compiler warning: drivers/gpu/drm/i915/intel_pm.c:4467]: (warning) Comparison of a boolean expression with an integer other than 0 or 1. Signed-off-by: Maarten Lankhorst Fixes: d555cb5827d6 ("drm/i915/skl+: use linetime latency if ddb size is not available") Cc: "Mahesh Kumar" Reported-by: David Binderman Cc: David Binderman Cc: # v4.13-rc1+ --- drivers/gpu/drm/i915/intel_pm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 78b9acfc64c0..b9b3d8d45016 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4681,8 +4681,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, if ((cpp * cstate->base.adjusted_mode.crtc_htotal / 512 < 1) && (plane_bytes_per_line / 512 < 1)) selected_result = method2; - else if ((ddb_allocation && ddb_allocation / - fixed_16_16_to_u32_round_up(plane_blocks_per_line)) >= 1) + else if (ddb_allocation >= +fixed_16_16_to_u32_round_up(plane_blocks_per_line)) selected_result = min_fixed_16_16(method1, method2); else if (latency >= linetime_us) selected_result = min_fixed_16_16(method1, method2); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix bad comparison in skl_compute_plane_wm, v2.
ddb_allocation && ddb_allocation / blocks_per_line >= 1 is the same as ddb_allocation >= blocks_per_line, so use the latter to simplify this. This fixes the following compiler warning: drivers/gpu/drm/i915/intel_pm.c:4467]: (warning) Comparison of a boolean expression with an integer other than 0 or 1. Changes since v1: - Rebase, was missing the changes to the macro names. Signed-off-by: Maarten Lankhorst Fixes: d555cb5827d6 ("drm/i915/skl+: use linetime latency if ddb size is not available") Cc: "Mahesh Kumar" Reported-by: David Binderman Cc: David Binderman Cc: # v4.13-rc1+ --- drivers/gpu/drm/i915/intel_pm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index e5a2c21f17a4..2fe14880c123 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4677,8 +4677,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, if ((cpp * cstate->base.adjusted_mode.crtc_htotal / 512 < 1) && (plane_bytes_per_line / 512 < 1)) selected_result = method2; - else if ((ddb_allocation && ddb_allocation / - fixed16_to_u32_round_up(plane_blocks_per_line)) >= 1) + else if (ddb_allocation >= +fixed16_to_u32_round_up(plane_blocks_per_line)) selected_result = min_fixed16(method1, method2); else if (latency >= linetime_us) selected_result = min_fixed16(method1, method2); -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/atomic-helper: add sync_all helper for gpu reset
Daniel Vetter writes: > i915 needs this. > > Cc: Maarten Lankhorst > Cc: Ville Syrjälä > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_atomic_helper.c | 38 > + > include/drm/drm_atomic_helper.h | 1 + > 2 files changed, 39 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index cea610161af5..545328a9a769 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1763,6 +1763,44 @@ static struct drm_crtc_commit > *preceeding_commit(struct drm_crtc *crtc) > } > > /** > + * drm_atomic_helper_sync_all - synchronize with all pending nonblocking > commits > + * @dev: DRM device > + * > + * This function synchronizes with all pending atomic commits using the > + * nonblocking helpers (see drm_atomic_helper_setup_commit()) up to the point > + * where all hardware commits are completed (as signalled using > + * drm_atomic_helper_commit_hw_done(). > + * > + * This is useful you need to synchronize with all nonblocking comits, but do s/comits/commits > + * not want to or cannot do a full atomic commit touching all CRTC. All CRTC > + * looks must be held to prevent a concurrent thread from queuing yet another s/looks/locks > + * update. Drivers can use this for e.g. their gpu reset handling, when a gpu > + * reset also affects the display block. > + */ > +void drm_atomic_helper_sync_all(struct drm_device *dev) > +{ > + struct drm_crtc *crtc; > + struct drm_crtc_commit *commit; > + > + drm_for_each_crtc(crtc, dev) { > + spin_lock(&crtc->commit_lock); > + commit = list_first_entry_or_null(&crtc->commit_list, > + struct drm_crtc_commit, commit_entry); > + if (commit) > + drm_crtc_commit_get(commit); > + spin_unlock(&crtc->commit_lock); > + > + if (!commit) > + continue; > + > + wait_for_completion(&commit->hw_done); > + > + drm_crtc_commit_put(commit); > + } > +} > +EXPORT_SYMBOL(drm_atomic_helper_sync_all); Reviewed-by: Mika Kuoppala > + > +/** > * drm_atomic_helper_wait_for_dependencies - wait for required preceeding > commits > * @old_state: atomic state object with old state structures > * > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h > index ed90d0c5ba09..95bef7c3632f 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -92,6 +92,7 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state > *state, > /* nonblocking commit helpers */ > int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, > bool nonblock); > +void drm_atomic_helper_sync_all(struct drm_device *dev); > void drm_atomic_helper_wait_for_dependencies(struct drm_atomic_state *state); > void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *state); > void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *state); > -- > 2.13.2 > > ___ > 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
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Fix bad comparison in skl_compute_plane_wm. (rev2)
== Series Details == Series: drm/i915: Fix bad comparison in skl_compute_plane_wm. (rev2) URL : https://patchwork.freedesktop.org/series/27395/ State : success == Summary == Series 27395v2 drm/i915: Fix bad comparison in skl_compute_plane_wm. https://patchwork.freedesktop.org/api/1.0/series/27395/revisions/2/mbox/ Test gem_exec_suspend: Subgroup basic-s4-devices: dmesg-warn -> PASS (fi-kbl-7560u) k.org#196399 Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: fail -> PASS (fi-snb-2600) fdo#100215 Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-a: dmesg-warn -> PASS (fi-pnv-d510) fdo#101597 +1 Subgroup suspend-read-crc-pipe-b: pass -> DMESG-WARN (fi-byt-j1900) fdo#101705 +1 k.org#196399 https://bugzilla.kernel.org/show_bug.cgi?id=196399 fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597 fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:439s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:441s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:356s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:530s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:510s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:490s fi-byt-n2820 total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:485s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:597s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:436s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:412s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:423s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:494s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:473s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:463s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:573s fi-kbl-r total:279 pass:260 dwarn:1 dfail:0 fail:0 skip:18 time:585s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:574s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:458s fi-skl-6700hqtotal:279 pass:262 dwarn:0 dfail:0 fail:0 skip:17 time:592s fi-skl-6700k total:279 pass:257 dwarn:4 dfail:0 fail:0 skip:18 time:464s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:479s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:434s fi-skl-x1585ltotal:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:472s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:538s fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:405s d93246177cf41d1731dae587923544b7a28659a6 drm-tip: 2017y-07m-17d-11h-27m-05s UTC integration manifest db7a1c6 drm/i915: Fix bad comparison in skl_compute_plane_wm, v2. == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5208/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 4/4] drm/i915/execlists: Preemption!
Quoting Chris Wilson (2017-07-17 10:46:23) > Quoting Chris Wilson (2017-07-17 09:42:35) > > @@ -503,6 +500,49 @@ static void execlists_dequeue(struct intel_engine_cs > > *engine) > > struct i915_priolist *p = rb_entry(rb, typeof(*p), node); > > struct drm_i915_gem_request *rq, *rn; > > > > + if (once) { > > + if (port_count(&port[0]) > 1) > > + goto done; > > + > > + if (p->priority > max(last->priotree.priority, 0)) { > > + list_for_each_entry_safe_reverse(rq, rn, > > + > > &engine->timeline->requests, > > +link) { > > + struct i915_priolist *p; > > + > > + if (i915_gem_request_completed(rq)) > > + break; > > + > > + __i915_gem_request_unsubmit(rq); > > + unwind_wa_tail(rq); > > Fwiw, this is the flaw in this approach. As we decide to move the > request back to the execution queue, it may complete. If we try to > reexecute it, its ring->tail will be less than RING_HEAD, telling the hw > to execute everything after it again. > > Michal's approach was to use a preemptive switch to a dummy context and > then once hw knew the hw wasn't executing any of the other requests, he > would unsubmit them and recompute the desired order. I've yet to see > another solution for a cheaper barrier between hw/sw, as otherwise we > must deliberately insert a stall to do preemption. :| First thought to closing the race is to ensure that the gpu doesn't advance across a breadcrumb as we are processing the list: diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index c712d01f92ab..18a08c701604 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -412,7 +412,12 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define MI_SEMAPHORE_TARGET(engine) ((engine)<<15) #define MI_SEMAPHORE_WAIT MI_INSTR(0x1c, 2) /* GEN8+ */ #define MI_SEMAPHORE_POLL(1<<15) +#define MI_SEMAPHORE_SAD_GT_SDD (0<<12) #define MI_SEMAPHORE_SAD_GTE_SDD (1<<12) +#define MI_SEMAPHORE_SAD_LT_SDD (2<<12) +#define MI_SEMAPHORE_SAD_LTE_SDD (3<<12) +#define MI_SEMAPHORE_SAD_EQ_SDD (4<<12) +#define MI_SEMAPHORE_SAD_NEQ_SDD (5<<12) #define MI_STORE_DWORD_IMM MI_INSTR(0x20, 1) #define MI_STORE_DWORD_IMM_GEN4MI_INSTR(0x20, 2) #define MI_MEM_VIRTUAL (1 << 22) /* 945,g33,965 */ diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index fe037bb9644c..c84e485cb323 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -505,6 +505,12 @@ static void execlists_dequeue(struct intel_engine_cs *engine) goto done; if (p->priority > max(last->priotree.priority, 0)) { + u32 *gpu_sema = &engine->status_page.page_addr[I915_GEM_HWS_INDEX + 2]; + /* Suspend updates from the gpu */ + WRITE_ONCE(*gpu_sema, 1); + readl(engine->i915->regs + + i915_mmio_reg_offset(RING_ACTHD(engine->mmio_base))); + list_for_each_entry_safe_reverse(rq, rn, &engine->timeline->requests, link) { @@ -613,11 +619,16 @@ static void execlists_dequeue(struct intel_engine_cs *engine) static void switch_to_preempt(struct intel_engine_cs *engine) { + u32 *gpu_sema = &engine->status_page.page_addr[I915_GEM_HWS_INDEX + 2]; + memcpy(engine->execlist_port, engine->execlist_preempt, sizeof(engine->execlist_preempt)); memset(engine->execlist_preempt, 0, sizeof(engine->execlist_preempt)); + + /* Restart updates from the gpu */ + WRITE_ONCE(*gpu_sema, 0); } /* @@ -1667,6 +1678,14 @@ static void gen8_emit_breadcrumb(struct drm_i915_gem_request *request, u32 *cs) /* w/a: bit 5 needs to be zero for MI_FLUSH_DW address. */ BUILD_BUG_ON(I915_GEM_HWS_INDEX_ADDR & (1 << 5)); + *cs++ = MI_SEMAPHORE_WAIT | + MI_SEMAPHORE_POLL | + MI_SEMAPHORE_GLOBAL_GTT | + MI_SEMAPHORE_SAD_EQ_SDD; + *cs++ = 0; /* continue if zero (preempt == 0) */ + *cs++ = intel_hws_seqno_address(request->engine) + 8; + *cs++ = 0; + *cs++ = (MI_FLUSH_DW + 1) | MI_FLUSH_DW_OP_STOREDW; *cs++ = intel_hws_seqno_address(request
Re: [Intel-gfx] [i-g-t] many testcases skiped
On Mon, Jul 17, 2017 at 12:30:30PM +0200, Gu, HailinX wrote: > Hi~ Hiler > Thank for your reply. > I'm sorry for didn't notice the integradted graphics. > But there is still some cases could not pass and shown requirement not met, > running on i7-4770. > I wonder if these skiped cases only about the cpu? So different processor > must have different test result. > Or can we do something to meet the requirements and let all case pass. > > Logs show below. > > /igt/tests# ./pm_backlight > IGT-Version: 1.19-g4258cc8e (x86_64) (Linux: 4.10.0 x86_64) > Test requirement not met in function __real_main154, file pm_backlight.c:163: > Test requirement: !(backlight_read(&old, "brightness")) > Last errno: 2, No such file or directory > Subtest basic-brightness: SKIP > Subtest bad-brightness: SKIP > Subtest fade: SKIP Could not read backlight values from sysfs. Probably is not supported on your hardware. > /igt/tests# ./kms_force_connector_basic > IGT-Version: 1.19-g4258cc8e (x86_64) (Linux: 4.10.0 x86_64) > Test requirement not met in function main, file > kms_force_connector_basic.c:111: > Test requirement: !(vga_connector->connection == DRM_MODE_CONNECTED) > Last errno: 2, No such file or directory > Subtest force-load-detect: SKIP > Subtest force-connector-state: SKIP > Subtest force-edid: SKIP > Subtest prune-stale-modes: SKIP Seems like VGA connector is not connected. So the tests skips. > /igt/tests# ./gem_exec_basic > IGT-Version: 1.19-g4258cc8e (x86_64) (Linux: 4.10.0 x86_64) > Subtest basic-default: SUCCESS (0.000s) > Subtest readonly-default: SUCCESS (0.000s) > Subtest gtt-default: SUCCESS (0.000s) > Subtest basic-render: SUCCESS (0.000s) > Subtest readonly-render: SUCCESS (0.000s) > Subtest gtt-render: SUCCESS (0.000s) > Subtest basic-bsd: SUCCESS (0.000s) > Subtest readonly-bsd: SUCCESS (0.000s) > Subtest gtt-bsd: SUCCESS (0.000s) > Test requirement not met in function gem_require_ring, file > ioctl_wrappers.c:1591: > Test requirement: gem_has_ring(fd, ring) > Subtest basic-bsd1: SKIP (0.000s) > Test requirement not met in function gem_require_ring, file > ioctl_wrappers.c:1591: > Test requirement: gem_has_ring(fd, ring) > Subtest readonly-bsd1: SKIP (0.000s) > Test requirement not met in function gem_require_ring, file > ioctl_wrappers.c:1591: > Test requirement: gem_has_ring(fd, ring) > Subtest gtt-bsd1: SKIP (0.000s) > Test requirement not met in function gem_require_ring, file > ioctl_wrappers.c:1591: > Test requirement: gem_has_ring(fd, ring) > Subtest basic-bsd2: SKIP (0.000s) > Test requirement not met in function gem_require_ring, file > ioctl_wrappers.c:1591: > Test requirement: gem_has_ring(fd, ring) > Subtest readonly-bsd2: SKIP (0.000s) > Test requirement not met in function gem_require_ring, file > ioctl_wrappers.c:1591: > Test requirement: gem_has_ring(fd, ring) > Subtest gtt-bsd2: SKIP (0.000s) You have only BSD ring. No BSD1 nor BSD2, therefore the skips. > Subtest basic-blt: SUCCESS (0.000s) > Subtest readonly-blt: SUCCESS (0.000s) > Subtest gtt-blt: SUCCESS (0.000s) > Subtest basic-vebox: SUCCESS (0.000s) > Subtest readonly-vebox: SUCCESS (0.000s) > Subtest gtt-vebox: SUCCESS (0.000s) > > /igt/tests#./kms_sink_crc_basic > IGT-Version: 1.19-g4258cc8e (x86_64) (Linux: 4.10.0 x86_64) > no eDP with CRC support found > SKIP (0.328s) Quite likely you do not have Embedded Display Port on a desktop machine. > Thank you > Gu SKIPs are expected when there is no support for particular feature in the hardware or other conditions are not met. If you do not find and error self-explanatory then bring it to the mailing list (or read the code). -- Cheers, Arek > > > -Original Message- > > From: Hiler, Arkadiusz > > Sent: Monday, July 17, 2017 4:30 PM > > To: Gu, HailinX > > Cc: intel-gfx@lists.freedesktop.org; Li, ZhijianX ; > > Li, > > Philip > > Subject: Re: [Intel-gfx] [i-g-t] many testcases skiped > > > > On Mon, Jul 17, 2017 at 11:14:03AM +0300, Arkadiusz Hiler wrote: > > > On Mon, Jul 17, 2017 at 12:28:00PM +0800, Hailin Gu wrote: > > > > Hi~ all, > > > > > > > > Does the test program have any hardware or version limitations? > > > > > > > > > > > > I found that most testcase could run on a machine with intel(R) > > > > Core(TM) i7-2600K CPU @ 3.40GHz. > > > > > > > > But can't run on Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz or > > > > Intel(R) > > > > Core(TM) i3-2120 CPU @ 3.30GHz. > > > > > > Hey, > > > > > > The hardware limitation is that you should have integradted graphics. > > > > And by "limitation" I've meant "requirement". At least when it comes to > > Intel > > GPUs. > > > > > E5-2669 v3 does not have one according to ark.intel.com. > > > > > > Error message seems to be pretty clear, stating: "No known gpu found". > > > > > > As of the i3 - it probably should work. Can you share complete logs > > > from that machinge to see what goes wrong there? > > > > > > -- > > > Cheers, > > > Arek > > > > > > > Thank you > > > > > > > > *From:*Hailin Gu [m
Re: [Intel-gfx] [PATCH v2 7/7] igt/kms_fbc_crc.c : Add Y-tile tests
On Friday 14 July 2017 07:55 PM, Paulo Zanoni wrote: Em Sex, 2017-07-14 às 19:25 +0530, Praveen Paneri escreveu: Hi Paulo, On Thursday 13 July 2017 02:31 AM, Paulo Zanoni wrote: Em Sex, 2017-04-28 às 20:07 +0530, Praveen Paneri escreveu: Now that we have support for Y-tiled buffers, add another iteration of tests for Y-tiled buffers. Have you tested this on platforms that don't support Y-tiled buffers? I Unfortunately I haven't... don't see a check for that, so I wonder if we'll just fail some assertion or correctly hit some igt_skip() call I couldn't find. ...but I will add the check as you have mentioned More below. Signed-off-by: Praveen Paneri --- tests/kms_fbc_crc.c | 71 + 1 file changed, 39 insertions(+), 32 deletions(-) diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c index 7964e05..cdf04c1 100644 --- a/tests/kms_fbc_crc.c +++ b/tests/kms_fbc_crc.c @@ -318,12 +318,10 @@ static void prepare_crtc(data_t *data) igt_output_set_pipe(output, data->pipe); } -static void create_fbs(data_t *data, bool tiled, struct igt_fb *fbs) +static void create_fbs(data_t *data, uint64_t tiling, struct igt_fb *fbs) { int rc; drmModeModeInfo *mode = igt_output_get_mode(data- output); - uint64_t tiling = tiled ? LOCAL_I915_FORMAT_MOD_X_TILED : - LOCAL_DRM_FORMAT_MOD_NONE; rc = igt_create_color_fb(data->drm_fd, mode->hdisplay, mode- vdisplay, DRM_FORMAT_XRGB, tiling, @@ -344,8 +342,8 @@ static void get_ref_crcs(data_t *data) struct igt_fb fbs[4]; int i; - create_fbs(data, false, &fbs[0]); - create_fbs(data, false, &fbs[2]); + create_fbs(data, LOCAL_DRM_FORMAT_MOD_NONE, &fbs[0]); + create_fbs(data, LOCAL_DRM_FORMAT_MOD_NONE, &fbs[2]); fill_mmap_gtt(data, fbs[2].gem_handle, 0xff); fill_mmap_gtt(data, fbs[3].gem_handle, 0xff); @@ -366,7 +364,7 @@ static void get_ref_crcs(data_t *data) igt_remove_fb(data->drm_fd, &fbs[i]); } -static bool prepare_test(data_t *data, enum test_mode test_mode) +static bool prepare_test(data_t *data, enum test_mode test_mode, uint64_t tiling) { igt_display_t *display = &data->display; igt_output_t *output = data->output; @@ -374,7 +372,7 @@ static bool prepare_test(data_t *data, enum test_mode test_mode) data->primary = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_PRIMARY); - create_fbs(data, true, data->fb); + create_fbs(data, tiling, data->fb); igt_pipe_crc_free(data->pipe_crc); data->pipe_crc = NULL; @@ -484,32 +482,41 @@ static void run_test(data_t *data, enum test_mode mode) reset_display(data); - for_each_pipe_with_valid_output(display, data->pipe, data- output) { - prepare_crtc(data); - - igt_info("Beginning %s on pipe %s, connector %s\n", - igt_subtest_name(), - kmstest_pipe_name(data->pipe), - igt_output_name(data->output)); - - if (!prepare_test(data, mode)) { - igt_info("%s on pipe %s, connector %s: SKIPPED\n", - igt_subtest_name(), - kmstest_pipe_name(data->pipe), - igt_output_name(data- output)); - continue; + for (int tiling = I915_TILING_X; +tiling <= I915_TILING_Y; tiling++) { What I don't understand is why this part of the code chooses to go with the tiling constants (I915_TILING_) only to later convert them to modifiers with igt_fb_tiling_to_mod(). If this loop iterated over the modifiers directly we wouldn't need that. The rest of the code only cares about the modifiers. I chose to loop over tiling constants as they are in a simple arithmetic order. anyhow I will just change that. Just put the two local_format stuff in an array and iterate over it. Also as mentioned above can I just add a check to skip Y-tiling tests for older platforms? igt_skip_on(intel_gen(intel_get_drm_devid(drm_fd)) < 9 && tiling == I915_TILING_Y); You can't do this here because the same subtest tests X tiling. Okay! what if I just skip the loop for Y tiling with some message? Perhaps we could make Y tiling be a separate subtest? I'm not a huge fan of single tests that do tons of stuff. Will it be possible in just one subtest. I thought we will have to duplicate all the subtests for Y-tile case? Plz suggest Thanks, Praveen + for_each_pipe_with_valid_output(display, + data->pipe, data- output) { + prepare_crtc(data); + + igt_info("Beginning %s on pipe %s, connector " + "%s, %s-tiled\n", +
[Intel-gfx] [PATCH xf86-video-intel] legacy/i810: remove unused numVisualConfigs
From: Emil Velikov Cc: Chris Wilson Signed-off-by: Emil Velikov --- src/legacy/i810/i810.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/legacy/i810/i810.h b/src/legacy/i810/i810.h index de250ab8..347188c9 100644 --- a/src/legacy/i810/i810.h +++ b/src/legacy/i810/i810.h @@ -218,7 +218,6 @@ typedef struct _I810Rec { int LockHeld; DRIInfoPtr pDRIInfo; int drmSubFD; - int numVisualConfigs; unsigned long dcacheHandle; unsigned long backHandle; unsigned long zHandle; -- 2.13.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 0/3] Add support for loadable OA configs
Hi all, Here is a v4 of this series. It is mostly prompted by one of Imre's series [1] that attracted my attention at how we do locking in the perf part of the driver. I believe up to now it was fine due to the fact that we had all configs always present in kernel space. With this series, configs can now go away. So locking needs some reworking, in particular dev_priv->perf.exclusive_stream needs to be locked. This series also introduce a new lock for adding/modifying/removing configs. Cheers, [1] : https://patchwork.freedesktop.org/series/27309/ Lionel Landwerlin (3): drm/i915/perf: fix flex eu registers programming drm/i915/perf: prune OA configs drm/i915: Implement I915_PERF_ADD/REMOVE_CONFIG interface drivers/gpu/drm/i915/i915_drv.c |2 + drivers/gpu/drm/i915/i915_drv.h | 92 +- drivers/gpu/drm/i915/i915_oa_bdw.c| 5360 + drivers/gpu/drm/i915/i915_oa_bdw.h|8 +- drivers/gpu/drm/i915/i915_oa_bxt.c| 2623 +--- drivers/gpu/drm/i915/i915_oa_bxt.h|8 +- drivers/gpu/drm/i915/i915_oa_chv.c| 2806 + drivers/gpu/drm/i915/i915_oa_chv.h|8 +- drivers/gpu/drm/i915/i915_oa_glk.c| 2535 +--- drivers/gpu/drm/i915/i915_oa_glk.h|8 +- drivers/gpu/drm/i915/i915_oa_hsw.c| 764 + drivers/gpu/drm/i915/i915_oa_hsw.h|8 +- drivers/gpu/drm/i915/i915_oa_kblgt2.c | 2971 +- drivers/gpu/drm/i915/i915_oa_kblgt2.h |8 +- drivers/gpu/drm/i915/i915_oa_kblgt3.c | 3020 +-- drivers/gpu/drm/i915/i915_oa_kblgt3.h |8 +- drivers/gpu/drm/i915/i915_oa_sklgt2.c | 3458 + drivers/gpu/drm/i915/i915_oa_sklgt2.h |8 +- drivers/gpu/drm/i915/i915_oa_sklgt3.c | 3019 +-- drivers/gpu/drm/i915/i915_oa_sklgt3.h |8 +- drivers/gpu/drm/i915/i915_oa_sklgt4.c | 3073 +-- drivers/gpu/drm/i915/i915_oa_sklgt4.h |8 +- drivers/gpu/drm/i915/i915_perf.c | 706 +++-- drivers/gpu/drm/i915/i915_reg.h |2 + include/uapi/drm/i915_drm.h | 24 + 25 files changed, 973 insertions(+), 29562 deletions(-) -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 1/3] drm/i915/perf: fix flex eu registers programming
We were reserving fewer dwords in the ring than necessary. Indeed we're always writing all registers once, so discard the actual number of registers given by the user and just program the whitelisted ones once. Fixes: 19f81df2859e ("drm/i915/perf: Add OA unit support for Gen 8+") Reported-by: Matthew Auld Signed-off-by: Lionel Landwerlin Reviewed-by: Matthew Auld Cc: # v4.12+ --- drivers/gpu/drm/i915/i915_perf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 96682fd86f82..df78bfa9e574 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1601,11 +1601,11 @@ static int gen8_emit_oa_config(struct drm_i915_gem_request *req) u32 *cs; int i; - cs = intel_ring_begin(req, n_flex_regs * 2 + 4); + cs = intel_ring_begin(req, ARRAY_SIZE(flex_mmio) * 2 + 4); if (IS_ERR(cs)) return PTR_ERR(cs); - *cs++ = MI_LOAD_REGISTER_IMM(n_flex_regs + 1); + *cs++ = MI_LOAD_REGISTER_IMM(ARRAY_SIZE(flex_mmio) + 1); *cs++ = i915_mmio_reg_offset(GEN8_OACTXCONTROL); *cs++ = (dev_priv->perf.oa.period_exponent << GEN8_OA_TIMER_PERIOD_SHIFT) | -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 3/3] drm/i915: Implement I915_PERF_ADD/REMOVE_CONFIG interface
The motivation behind this new interface is expose at runtime the creation of new OA configs which can be used as part of the i915 perf open interface. This will enable the kernel to learn new configs which may be experimental, or otherwise not part of the core set currently available through the i915 perf interface. This will relieve the kernel from holding all the possible configs, so we can leave only the test configs here. v2: Drop DRM_ERROR for userspace errors (Matthew) Add padding to userspace structure (Matthew) s/guid/uuid/ (Matthew) v3: Use u32 instead of int to iterate through registers (Matthew) v4: Lock access to dynamic config list (Lionel) Signed-off-by: Matthew Auld Signed-off-by: Lionel Landwerlin Signed-off-by: Andrzej Datczuk --- drivers/gpu/drm/i915/i915_drv.c | 2 + drivers/gpu/drm/i915/i915_drv.h | 48 + drivers/gpu/drm/i915/i915_perf.c | 413 +-- drivers/gpu/drm/i915/i915_reg.h | 2 + include/uapi/drm/i915_drm.h | 24 +++ 5 files changed, 475 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index d310d8245dca..a3d339670ec1 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2730,6 +2730,8 @@ static const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), }; static struct drm_driver driver = { diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2b824f8875c4..607484737f3d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1917,6 +1917,9 @@ struct i915_oa_config { struct attribute_group sysfs_metric; struct attribute *attrs[2]; struct device_attribute sysfs_metric_id; + + /* Only updated while holding dev_priv->perf.metrics_lock. */ + bool in_use; }; struct i915_perf_stream; @@ -2043,6 +2046,25 @@ struct i915_perf_stream { */ struct i915_oa_ops { /** +* @is_valid_b_counter_reg: Validates register's address for +* programming boolean counters for a particular platform. +*/ + bool (*is_valid_b_counter_reg)(struct drm_i915_private *dev_priv, + u32 addr); + + /** +* @is_valid_mux_reg: Validates register's address for programming mux +* for a particular platform. +*/ + bool (*is_valid_mux_reg)(struct drm_i915_private *dev_priv, u32 addr); + + /** +* @is_valid_flex_reg: Validates register's address for programming +* flex EU filtering for a particular platform. +*/ + bool (*is_valid_flex_reg)(struct drm_i915_private *dev_priv, u32 addr); + + /** * @init_oa_buffer: Resets the head and tail pointers of the * circular buffer for periodic OA reports. * @@ -2430,10 +2452,32 @@ struct drm_i915_private { struct kobject *metrics_kobj; struct ctl_table_header *sysctl_header; + /* +* Lock associated with adding/modifying/removing OA configs +* in dev_priv->perf.metrics_idr. +*/ + struct mutex metrics_lock; + + /* +* List of dynamic configurations, you need to hold +* dev_priv->perf.metrics_lock to access it. +*/ + struct idr metrics_idr; + + /* +* Lock associated with anything below within this structure +* except exclusive_stream. +*/ struct mutex lock; struct list_head streams; struct { + /* +* The stream currently using the OA unit. If accessed +* outside a syscall associated to its file +* descriptor, you need to hold +* dev_priv->drm.struct_mutex. +*/ struct i915_perf_stream *exclusive_stream; u32 specific_ctx_id; @@ -3599,6 +3643,10 @@ i915_gem_context_lookup_timeline(struct i915_gem_context *ctx, int i915_perf_open_ioctl(struct drm_device *dev, void *data, struct drm_file *file); +int i915_perf_add_config_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); +int i915_perf_remove_config_ioc
[Intel-gfx] [PATCH v3 i-g-t] igt/perf: add tests to verify create/destroy userspace configs
v2: Add tests regarding removing configs (Matthew) Add tests regarding adding/removing configs without permissions (Matthew) v3: Add some flex registers (Matthew) Signed-off-by: Lionel Landwerlin --- tests/perf.c | 207 +++ 1 file changed, 207 insertions(+) diff --git a/tests/perf.c b/tests/perf.c index db28ba1f..e341262f 100644 --- a/tests/perf.c +++ b/tests/perf.c @@ -146,6 +146,36 @@ enum drm_i915_perf_record_type { }; #endif /* !DRM_I915_PERF_OPEN */ +#ifndef DRM_IOCTL_I915_PERF_ADD_CONFIG + +#define DRM_I915_PERF_ADD_CONFIG 0x37 +#define DRM_I915_PERF_REMOVE_CONFIG0x38 + +#define DRM_IOCTL_I915_PERF_ADD_CONFIG DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_PERF_ADD_CONFIG, struct drm_i915_perf_oa_config) +#define DRM_IOCTL_I915_PERF_REMOVE_CONFIG DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_PERF_REMOVE_CONFIG, __u64) + +/** + * Structure to upload perf dynamic configuration into the kernel. + */ +struct drm_i915_perf_oa_config { + /** String formatted like "%08x-%04x-%04x-%04x-%012x" */ + __u64 uuid; + + __u32 n_mux_regs; + __u32 pad0; + __u64 mux_regs; + + __u32 n_boolean_regs; + __u32 pad1; + __u64 boolean_regs; + + __u32 n_flex_regs; + __u32 pad2; + __u64 flex_regs; +}; + +#endif /* !DRM_IOCTL_I915_PERF_ADD_CONFIG */ + struct accumulator { #define MAX_RAW_OA_COUNTERS 62 enum drm_i915_oa_format format; @@ -4001,6 +4031,174 @@ test_rc6_disable(void) igt_assert_neq(n_events_end - n_events_start, 0); } +static void +test_invalid_userspace_config_create(void) +{ + struct drm_i915_perf_oa_config config; + const char *uuid = "01234567-0123-0123-0123-0123456789ab"; + const char *invalid_uuid = "blablabla-wrong"; + uint32_t mux_regs[] = { 0x9888 /* NOA_WRITE */, 0x0 }; + uint32_t invalid_mux_regs[] = { 0x12345678 /* invalid register */, 0x0 }; + + memset(&config, 0, sizeof(config)); + + /* invalid uuid */ + config.uuid = to_user_pointer(invalid_uuid); + config.n_mux_regs = 1; + config.mux_regs = to_user_pointer(mux_regs); + config.n_boolean_regs = 0; + config.n_flex_regs = 0; + + do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config, EINVAL); + + /* invalid mux_regs */ + config.uuid = to_user_pointer(uuid); + config.n_mux_regs = 1; + config.mux_regs = to_user_pointer(invalid_mux_regs); + config.n_boolean_regs = 0; + config.n_flex_regs = 0; + + do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config, EINVAL); + + /* empty config */ + config.uuid = to_user_pointer(uuid); + config.n_mux_regs = 0; + config.mux_regs = to_user_pointer(mux_regs); + config.n_boolean_regs = 0; + config.n_flex_regs = 0; + + do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config, EINVAL); + + /* empty config with null pointers */ + config.uuid = to_user_pointer(uuid); + config.n_mux_regs = 1; + config.mux_regs = to_user_pointer(NULL); + config.n_boolean_regs = 2; + config.boolean_regs = to_user_pointer(NULL); + config.n_flex_regs = 3; + config.flex_regs = to_user_pointer(NULL); + + do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config, EINVAL); +} + +static void +test_invalid_userspace_config_remove(void) +{ + struct drm_i915_perf_oa_config config; + const char *uuid = "01234567-0123-0123-0123-0123456789ab"; + uint32_t mux_regs[] = { 0x9888 /* NOA_WRITE */, 0x0 }; + uint64_t config_id, wrong_config_id = 9; + char path[512]; + int ret; + + snprintf(path, sizeof(path), "/sys/class/drm/card%d/metrics/%s/id", card, uuid); + + /* Destroy previous configuration if present */ + if (try_read_u64_file(path, &config_id)) + igt_assert(igt_ioctl(drm_fd, DRM_IOCTL_I915_PERF_REMOVE_CONFIG, &config_id) == 0); + + memset(&config, 0, sizeof(config)); + + config.uuid = to_user_pointer(uuid); + + config.n_mux_regs = 1; + config.mux_regs = to_user_pointer(mux_regs); + config.n_boolean_regs = 0; + config.n_flex_regs = 0; + + ret = igt_ioctl(drm_fd, DRM_IOCTL_I915_PERF_ADD_CONFIG, &config); + igt_assert(ret > 0); + config_id = ret; + + /* Removing configs without permissions should fail. */ + igt_fork(child, 1) { + igt_drop_root(); + + do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_REMOVE_CONFIG, &config_id, EACCES); + } + igt_waitchildren(); + + /* Removing invalid config ID should fail. */ + do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_REMOVE_CONFIG, &wrong_config_id, EINVAL); + + igt_assert(igt_ioctl(drm_fd, DRM_IOCTL_I915_PERF_REMOVE_CONFIG, &config_id) == 0); +} + +static void +test_create_destroy_userspace_config(void) +{ + struct drm_i915_perf_oa_config config; +
Re: [Intel-gfx] [PATCH i-g-t 1/7] igt: lib/igt_crc: Split out CRC functionality
On Thu, Jul 06, 2017 at 05:14:18PM +0100, Liviu Dudau wrote: > From: Brian Starkey > > Separate out the CRC code for better compartmentalisation. Should ease > the addition of more/different CRC sources in the future. > > Signed-off-by: Brian Starkey > Signed-off-by: Liviu Dudau > > --- > lib/Makefile.sources | 2 + > lib/igt_chamelium.h | 1 + > lib/igt_crc.c | 563 > ++ > lib/igt_crc.h | 125 + > lib/igt_debugfs.c | 547 > lib/igt_debugfs.h | 81 -- > tests/chamelium.c | 1 + > tests/kms_atomic_transition.c | 1 + > tests/kms_ccs.c | 1 + > tests/kms_chv_cursor_fail.c | 1 + > tests/kms_crtc_background_color.c | 1 + > tests/kms_cursor_crc.c| 1 + > tests/kms_cursor_legacy.c | 1 + > tests/kms_draw_crc.c | 1 + > tests/kms_fbc_crc.c | 1 + > tests/kms_flip_tiling.c | 1 + > tests/kms_frontbuffer_tracking.c | 1 + > tests/kms_mmap_write_crc.c| 1 + > tests/kms_mmio_vs_cs_flip.c | 1 + > tests/kms_pipe_color.c| 1 + > tests/kms_pipe_crc_basic.c| 1 + > tests/kms_plane.c | 1 + > tests/kms_plane_lowres.c | 1 + > tests/kms_plane_multiple.c| 1 + > tests/kms_plane_scaling.c | 1 + > tests/kms_pwrite_crc.c| 1 + > tests/kms_rotation_crc.c | 1 + > tests/kms_universal_plane.c | 1 + > tools/intel_display_crc.c | 1 + > 29 files changed, 714 insertions(+), 628 deletions(-) > create mode 100644 lib/igt_crc.c > create mode 100644 lib/igt_crc.h > > diff --git a/lib/Makefile.sources b/lib/Makefile.sources > index 53fdb54c..cfba15c9 100644 > --- a/lib/Makefile.sources > +++ b/lib/Makefile.sources > @@ -11,6 +11,8 @@ lib_source_list = \ > igt_debugfs.h \ > igt_aux.c \ > igt_aux.h \ > + igt_crc.c \ > + igt_crc.h \ > igt_edid_template.h \ > igt_gt.c\ > igt_gt.h\ > diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h > index 81322ad2..ea5abc2e 100644 > --- a/lib/igt_chamelium.h > +++ b/lib/igt_chamelium.h > @@ -31,6 +31,7 @@ > #endif > > #include "igt.h" > +#include "igt_crc.h" > #include > > struct chamelium; > diff --git a/lib/igt_crc.c b/lib/igt_crc.c > new file mode 100644 > index ..91a0b5a8 > --- /dev/null > +++ b/lib/igt_crc.c > @@ -0,0 +1,563 @@ > +/* > + * Copyright © 2013 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > DEALINGS > + * IN THE SOFTWARE. > + * > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "igt_aux.h" > +#include "igt_crc.h" > +#include "igt_core.h" > +#include "igt_debugfs.h" > +#include "igt_kms.h" > + > +/** > + * igt_assert_crc_equal: > + * @a: first pipe CRC value > + * @b: second pipe CRC value > + * > + * Compares two CRC values and fails the testcase if they don't match with > + * igt_fail(). Note that due to CRC collisions CRC based testcase can only > + * assert that CRCs match, never that they are different. Otherwise there > might > + * be random testcase failures when different screen contents end up with the > + * same CRC by chance. > + */ > +void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) > +{ > + int i; > + > + for (i = 0; i < a->n_words; i++) > + igt_assert_eq_u32(a->crc[i], b->crc[i]); > +} > + > +/** > + * igt_crc_to_string: > + * @crc: pipe CRC value to print > + * > + * This formats @crc into a string buffer which is owned by > igt_crc_to_string(). > +
[Intel-gfx] [PATCH i-g-t] lib/igt_debugfs: Update documentation and clenup
The documentation was lying. The igt_crc_to_string() is threadsafe and does not return a pointer to an internal buffer. Actually the caller is responsible for the memory that is allocated (and they are for all the current cases), so let's put that in the doc too. While I was at it I got rid of strdup() in favor of an early allocation. Cc: Martin Peres Cc: Liviu Dudau Signed-off-by: Arkadiusz Hiler --- lib/igt_debugfs.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c index 80f25c6..5e4d72c 100644 --- a/lib/igt_debugfs.c +++ b/lib/igt_debugfs.c @@ -304,21 +304,20 @@ void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) * igt_crc_to_string: * @crc: pipe CRC value to print * - * This formats @crc into a string buffer which is owned by igt_crc_to_string(). - * The next call will override the buffer again, which makes this multithreading - * unsafe. + * This formats @crc into a string. Function allocates memory - the caller is + * in charge of freeing it. * * This should only ever be used for diagnostic debug output. */ char *igt_crc_to_string(igt_crc_t *crc) { int i; - char buf[128] = { 0 }; + char *buf = calloc(128, sizeof(char)); for (i = 0; i < crc->n_words; i++) sprintf(buf + strlen(buf), "%08x ", crc->crc[i]); - return strdup(buf); + return buf; } #define MAX_CRC_ENTRIES 10 -- 2.9.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for Add support for loadable OA configs
== Series Details == Series: Add support for loadable OA configs URL : https://patchwork.freedesktop.org/series/27403/ State : warning == Summary == Series 27403v1 Add support for loadable OA configs https://patchwork.freedesktop.org/api/1.0/series/27403/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: pass -> FAIL (fi-snb-2600) fdo#17 Test gem_exec_suspend: Subgroup basic-s4-devices: dmesg-warn -> PASS (fi-kbl-7560u) k.org#196399 +1 Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: fail -> PASS (fi-snb-2600) fdo#100215 Test kms_flip: Subgroup basic-flip-vs-modeset: skip -> PASS (fi-skl-x1585l) fdo#101781 Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-a: dmesg-warn -> PASS (fi-pnv-d510) fdo#101597 Subgroup suspend-read-crc-pipe-a: pass -> DMESG-WARN (fi-skl-gvtdvm) Subgroup suspend-read-crc-pipe-b: pass -> DMESG-WARN (fi-byt-j1900) fdo#101705 fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 k.org#196399 https://bugzilla.kernel.org/show_bug.cgi?id=196399 fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781 fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597 fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:427s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:415s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:355s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:483s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:478s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:492s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:487s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:574s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:421s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:404s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:423s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:496s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:482s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:453s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:573s fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:568s fi-pnv-d510 total:279 pass:222 dwarn:2 dfail:0 fail:0 skip:55 time:567s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:440s fi-skl-6700hqtotal:279 pass:262 dwarn:0 dfail:0 fail:0 skip:17 time:575s fi-skl-6700k total:279 pass:257 dwarn:4 dfail:0 fail:0 skip:18 time:631s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:457s fi-skl-gvtdvmtotal:279 pass:265 dwarn:1 dfail:0 fail:0 skip:13 time:425s fi-skl-x1585ltotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:492s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:544s fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:407s d93246177cf41d1731dae587923544b7a28659a6 drm-tip: 2017y-07m-17d-11h-27m-05s UTC integration manifest c7a833a drm/i915: Implement I915_PERF_ADD/REMOVE_CONFIG interface 1496e79 drm/i915/perf: prune OA configs e867034 drm/i915/perf: fix flex eu registers programming == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5209/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Explicit the connector name for DP link training result
This adds the connector name when printing a debug message about the DP link training result. It is useful to figure out what connector is failing when multiple DP connectors are used. Signed-off-by: Paul Kocialkowski --- drivers/gpu/drm/i915/intel_dp_link_training.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c index b79c1c0e404c..75a411c94ce5 100644 --- a/drivers/gpu/drm/i915/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c @@ -321,13 +321,15 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) if (!intel_dp_link_training_channel_equalization(intel_dp)) goto failure_handling; - DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", - intel_dp->link_rate, intel_dp->lane_count); + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d for connector %s", + intel_dp->link_rate, intel_dp->lane_count, + intel_connector->base.name); return; failure_handling: - DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", - intel_dp->link_rate, intel_dp->lane_count); + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d for connector %s", + intel_dp->link_rate, intel_dp->lane_count, + intel_connector->base.name); if (!intel_dp_get_link_train_fallback_values(intel_dp, intel_dp->link_rate, intel_dp->lane_count)) -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 1/6] drm/i915: add config function for YCBCR420 outputs
This patch checks encoder level support for YCBCR420 outputs. The logic goes as simple as this: If the input mode is YCBCR420-only mode: prepare HDMI for YCBCR420 output, else continue with RGB output mode. It checks if the mode is YCBCR420 and source can support this output then it marks the ycbcr_420 output indicator into crtc state, for further staging in driver. V2: Split the patch into two, kept helper functions in DRM layer. V3: Changed the compute_config function based on new DRM API. V4: Rebase V5: Rebase V6: Check and handle YCBCR420-only modes, discard the property based approach (Ville) V7: Addressed review comments from Ville - add else case in 12BPC check. - extract ycbcr420 state inside hdmi_12bpc_possible function. V8: Addressed review comments from Ville - Remove extra blank lines. - Remove "HDMI" from the description of ycbcr420 state variable. - Remove local variable, use crtc_state->ycbcr420 instead. Added r-b from Ville. Cc: Ville Syrjala Cc: Daniel Vetter Cc: Ander Conselvan de Oliveira Reviewed-by: Ville Syrjala Signed-off-by: Shashank Sharma --- drivers/gpu/drm/i915/intel_display.c | 1 + drivers/gpu/drm/i915/intel_drv.h | 3 +++ drivers/gpu/drm/i915/intel_hdmi.c| 40 +--- 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2144adc..a5140e4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11945,6 +11945,7 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv, PIPE_CONF_CHECK_I(hdmi_scrambling); PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio); PIPE_CONF_CHECK_I(has_infoframe); + PIPE_CONF_CHECK_I(ycbcr420); PIPE_CONF_CHECK_I(has_audio); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index d17a324..372aebf 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -780,6 +780,9 @@ struct intel_crtc_state { /* HDMI High TMDS char rate ratio */ bool hdmi_high_tmds_clock_ratio; + + /* output format is YCBCR 4:2:0 */ + bool ycbcr420; }; struct intel_crtc { diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 2f831cf..64bb8cc 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1330,8 +1330,15 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state) if (connector_state->crtc != crtc_state->base.crtc) continue; - if ((info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36) == 0) - return false; + if (crtc_state->ycbcr420) { + const struct drm_hdmi_info *hdmi = &info->hdmi; + + if (!(hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_36)) + return false; + } else { + if (!(info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36)) + return false; + } } /* Display Wa #1139 */ @@ -1342,6 +1349,24 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state) return true; } +static bool +intel_hdmi_ycbcr420_config(struct drm_connector *connector, + struct intel_crtc_state *config, + int *clock_12bpc, int *clock_8bpc) +{ + if (!connector->ycbcr_420_allowed) { + DRM_ERROR("Platform doesn't support YCBCR420 output\n"); + return false; + } + + /* YCBCR420 TMDS rate requirement is half the pixel clock */ + config->port_clock /= 2; + *clock_12bpc /= 2; + *clock_8bpc /= 2; + config->ycbcr420 = true; + return true; +} + bool intel_hdmi_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config, struct drm_connector_state *conn_state) @@ -1349,7 +1374,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; - struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc; + struct drm_connector *connector = conn_state->connector; + struct drm_scdc *scdc = &connector->display_info.hdmi.scdc; struct intel_digital_connector_state *intel_conn_state = to_intel_digital_connector_state(conn_state); int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock; @@ -1379,6 +1405,14 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, clock_12bpc *= 2; } +
[Intel-gfx] [PATCH v4 0/6] YCBCR 4:2:0 handling in i915 layer
This patch series is a subset of series "YCBCR 4:2:0 handling in DRM layer" Published and reviewed here: https://patchwork.freedesktop.org/series/26973/ The original series had 14 patches, out of which first 8 (which were in DRM layer) are reviewed merged. These 6 patches are the I915 layer handling of YCBCR 4:2:0 output. The patch series got review comments from Ville Syrjala in previous patch set, addressed those here. Shashank Sharma (6): drm/i915: add config function for YCBCR420 outputs drm/i915: prepare scaler for YCBCR420 modeset drm/i915: prepare pipe for YCBCR420 output drm/i915: prepare csc unit for YCBCR420 output drm/i915: set colorspace for YCBCR420 outputs drm/i915/glk: set HDMI 2.0 identifier drivers/gpu/drm/i915/i915_reg.h | 3 ++ drivers/gpu/drm/i915/intel_color.c | 46 - drivers/gpu/drm/i915/intel_display.c | 67 ++-- drivers/gpu/drm/i915/intel_drv.h | 3 ++ drivers/gpu/drm/i915/intel_hdmi.c| 61 ++-- drivers/gpu/drm/i915/intel_panel.c | 3 +- 6 files changed, 175 insertions(+), 8 deletions(-) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 2/6] drm/i915: prepare scaler for YCBCR420 modeset
To get a YCBCR420 output from intel platforms, we need one scaler to scale down YCBCR444 samples to YCBCR420 samples. This patch: - Does scaler allocation for HDMI ycbcr420 outputs. - Programs PIPE_MISC register for ycbcr420 output. - Adds a new scaler user "HDMI output" to plug-into existing scaler framework. This output type is identified using bit 30 of the scaler users bitmap. V2: rebase V3: rebase V4: rebase V5: addressed review comments from Ander: - No need to check both scaler_user && hdmi_output. Check for scaler_user is enough. V6: rebase V7: Do not create a new scaler user, use existing pipe scaler user. V8: rebase V9: Addressed review comments from Ville: - Remove leftover comment for HDMI scaler user. - Remove unnecessary blank line. - Make scaler alocation failure a DEBUG log instead of ERROR. Added r-b from Ville Cc: Ville Syrjala Cc: Ander Conselvan De Oliveira Reviewed-by: Ville Syrjala Signed-off-by: Shashank Sharma --- drivers/gpu/drm/i915/intel_display.c | 3 +++ drivers/gpu/drm/i915/intel_hdmi.c| 12 drivers/gpu/drm/i915/intel_panel.c | 3 ++- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a5140e4..d78f1c2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4624,6 +4624,9 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, */ need_scaling = src_w != dst_w || src_h != dst_h; + if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX) + need_scaling = true; + /* * Scaling/fitting not supported in IF-ID mode in GEN9+ * TODO: Interlace fetch mode doesn't support YUV420 planar formats. diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 64bb8cc..6addef5 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1354,6 +1354,8 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector, struct intel_crtc_state *config, int *clock_12bpc, int *clock_8bpc) { + struct intel_crtc *intel_crtc = to_intel_crtc(config->base.crtc); + if (!connector->ycbcr_420_allowed) { DRM_ERROR("Platform doesn't support YCBCR420 output\n"); return false; @@ -1364,6 +1366,16 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector, *clock_12bpc /= 2; *clock_8bpc /= 2; config->ycbcr420 = true; + + /* YCBCR 420 output conversion needs a scaler */ + if (skl_update_scaler_crtc(config)) { + DRM_DEBUG_KMS("Scaler allocation for output failed\n"); + return false; + } + + intel_pch_panel_fitting(intel_crtc, config, + DRM_MODE_SCALE_FULLSCREEN); + return true; } diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 96c2cbd..fd2e081 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -110,7 +110,8 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc, /* Native modes don't need fitting */ if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w && - adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) + adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h && + !pipe_config->ycbcr420) goto done; switch (fitting_mode) { -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 5/6] drm/i915: set colorspace for YCBCR420 outputs
When output colorspace is YCBCR420, we have to load the corresponding colorspace in AVI infoframe. This patch fills the colorspace of AVI infoframe as per the output mode. V2: Rebase V3: Rebase V4: Rebase V5: Added r-b from Ander V6: Checking RGB/YCBCR420 output only (Ville) V7: Add colorspace info in driver(not drm layer) (Ville) V8: Rebase V9: Added r-b from Ville Cc: Ville Syrjala Cc: Ander Conselvan de Oliveira Reviewed-by: Ander Conselvan de Oliveira Reviewed-by: Ville Syrjala Signed-off-by: Shashank Sharma --- drivers/gpu/drm/i915/intel_hdmi.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 6addef5..bedc6eb 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -472,12 +472,18 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder, return; } + if (crtc_state->ycbcr420) + frame.avi.colorspace = HDMI_COLORSPACE_YUV420; + else + frame.avi.colorspace = HDMI_COLORSPACE_RGB; + drm_hdmi_avi_infoframe_quant_range(&frame.avi, adjusted_mode, crtc_state->limited_color_range ? HDMI_QUANTIZATION_RANGE_LIMITED : HDMI_QUANTIZATION_RANGE_FULL, intel_hdmi->rgb_quant_range_selectable); + /* TODO: handle pixel repetition for YCBCR420 outputs */ intel_write_infoframe(encoder, crtc_state, &frame); } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 3/6] drm/i915: prepare pipe for YCBCR420 output
To get HDMI YCBCR420 output, the PIPEMISC register should be programmed to: - Generate YCBCR output (bit 11) - In case of YCBCR420 outputs, it should be programmed in full blend mode to use the scaler in 5x3 ratio (bits 26 and 27) This patch: - Adds definition of these bits. - Programs PIPEMISC for YCBCR420 outputs. - Adds readouts to compare HW and SW states. V2: rebase V3: rebase V4: rebase V5: added r-b from Ander V6: Handle only YCBCR420 outputs (ville) V7: rebase V8: Addressed review comments from Ville - Add readouts for state->ycbcr420 and 420 pixel_clock. - Handle warning due to mismatch in clock for ycbcr420 clock. - Rename PIPEMISC macros to match the Bspec. - Add a debug print stating if YCBCR 4:2:0 output enabled. Added r-b from Ville Cc: Ville Syrjala Cc: Ander Conselvan de Oliveira Cc: Daniel Vetter Reviewed-by: Ander Conselvan de Oliveira Reviewed-by: Ville Syrjala Signed-off-by: Shashank Sharma --- drivers/gpu/drm/i915/i915_reg.h | 3 ++ drivers/gpu/drm/i915/intel_display.c | 55 ++-- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index c712d01..6dfcdd3 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5227,6 +5227,9 @@ enum { #define _PIPE_MISC_A 0x70030 #define _PIPE_MISC_B 0x71030 +#define PIPEMISC_YUV420_ENABLE (1<<27) +#define PIPEMISC_YUV420_MODE_BLEND (1<<26) +#define PIPEMISC_OUTPUT_YUV (1<<11) #define PIPEMISC_DITHER_BPC_MASK (7<<5) #define PIPEMISC_DITHER_8_BPC(0<<5) #define PIPEMISC_DITHER_10_BPC (1<<5) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d78f1c2..788502a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6216,7 +6216,6 @@ static uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config) * We only use IF-ID interlacing. If we ever use * PF-ID we'll need to adjust the pixel_rate here. */ - if (pipe_config->pch_pfit.enabled) { uint64_t pipe_w, pipe_h, pfit_w, pfit_h; uint32_t pfit_size = pipe_config->pch_pfit.size; @@ -8076,6 +8075,7 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(crtc->dev); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_crtc_state *config = intel_crtc->config; if (IS_BROADWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 9) { u32 val = 0; @@ -8101,6 +8101,12 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc) if (intel_crtc->config->dither) val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP; + if (config->ycbcr420) { + val |= PIPEMISC_OUTPUT_YUV | + PIPEMISC_YUV420_ENABLE | + PIPEMISC_YUV420_MODE_BLEND; + } + I915_WRITE(PIPEMISC(intel_crtc->pipe), val); } } @@ -9170,6 +9176,10 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX); } + if (IS_GEMINILAKE(dev_priv)) + pipe_config->ycbcr420 = I915_READ(PIPEMISC(crtc->pipe)) & + PIPEMISC_YUV420_ENABLE; + power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe); if (intel_display_power_get_if_enabled(dev_priv, power_domain)) { power_domain_mask |= BIT_ULL(power_domain); @@ -11377,6 +11387,9 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc, pipe_config->fdi_lanes, &pipe_config->fdi_m_n); + if (pipe_config->ycbcr420) + DRM_DEBUG_KMS("YCbCr 4:2:0 output enabled\n"); + if (intel_crtc_has_dp_encoder(pipe_config)) { intel_dump_m_n_config(pipe_config, "dp m_n", pipe_config->lane_count, &pipe_config->dp_m_n); @@ -11704,6 +11717,22 @@ intel_modeset_update_crtc_state(struct drm_atomic_state *state) } } +static bool intel_420_clock_check(int clock1, int clock2) +{ + int diff; + + if (clock1 == clock2 * 2) + return true; + + clock2 *= 2; + diff = abs(clock1 - clock2); + + if (diff + clock1 + clock2) * 100)) / (clock1 + clock2)) < 105) + return true; + + return false; +} + static bool intel_fuzzy_clock_check(int clock1, int clock2) { int diff; @@ -11832,6 +11861,18 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv, ret = false; \ } +#define PIPE_CONF_CHECK_CLOCK_420(name) \
[Intel-gfx] [PATCH v4 4/6] drm/i915: prepare csc unit for YCBCR420 output
To support ycbcr output, we need a pipe CSC block to do RGB->YCBCR conversion. Current Intel platforms have only one pipe CSC unit, so we can either do color correction using it, or we can perform RGB->YCBCR conversion. This function adds a csc handler, which uses recommended bspec values to perform RGB->YCBCR conversion (target color space BT709) V2: Rebase V3: Rebase V4: Rebase V5: Addressed review comments from Ander - Remove extra line added in the patch - Add the spec details in the commit message - Combine two if(cond) while calling intel_crtc_compute_config V6: Handle YCBCR420 outputs only (Ville) V7: Addressed review comments from Ville: - Add description about target colorspace - Remove the comments from CSC function - DRM_DEBUG->DEBUG_KMS for atomic failure due to CSC unit busy - Remove unnecessary debug message about YCBCR420 possibe V8: Addressed review comments from Ville: - Remove extra comment, not required. - Do not add extra variable for CTM, reuse pipe_config Added r-b from Ville Cc: Ville Syrjala Cc: Daniel Vetter Cc: Ander Conselvan de Oliveira Reviewed-by: Ville Syrjala Signed-off-by: Shashank Sharma --- drivers/gpu/drm/i915/intel_color.c | 46 +++- drivers/gpu/drm/i915/intel_display.c | 11 + 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index f85d575..8698691 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -41,6 +41,22 @@ #define LEGACY_LUT_LENGTH (sizeof(struct drm_color_lut) * 256) +/* Post offset values for RGB->YCBCR conversion */ +#define POSTOFF_RGB_TO_YUV_HI 0x800 +#define POSTOFF_RGB_TO_YUV_ME 0x100 +#define POSTOFF_RGB_TO_YUV_LO 0x800 + +/* + * These values are direct register values specified in the Bspec, + * for RGB->YUV conversion matrix (colorspace BT709) + */ +#define CSC_RGB_TO_YUV_RU_GU 0x2ba809d8 +#define CSC_RGB_TO_YUV_BU 0x37e8 +#define CSC_RGB_TO_YUV_RY_GY 0x1e089cc0 +#define CSC_RGB_TO_YUV_BY 0xb528 +#define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 +#define CSC_RGB_TO_YUV_BV 0x1e08 + /* * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point * format). This macro takes the coefficient we want transformed and the @@ -91,6 +107,31 @@ static void ctm_mult_by_limited(uint64_t *result, int64_t *input) } } +void i9xx_load_ycbcr_conversion_matrix(struct intel_crtc *intel_crtc) +{ + int pipe = intel_crtc->pipe; + struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); + + + I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0); + I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0); + I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0); + + I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), CSC_RGB_TO_YUV_RU_GU); + I915_WRITE(PIPE_CSC_COEFF_BU(pipe), CSC_RGB_TO_YUV_BU); + + I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), CSC_RGB_TO_YUV_RY_GY); + I915_WRITE(PIPE_CSC_COEFF_BY(pipe), CSC_RGB_TO_YUV_BY); + + I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), CSC_RGB_TO_YUV_RV_GV); + I915_WRITE(PIPE_CSC_COEFF_BV(pipe), CSC_RGB_TO_YUV_BV); + + I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), POSTOFF_RGB_TO_YUV_HI); + I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), POSTOFF_RGB_TO_YUV_ME); + I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), POSTOFF_RGB_TO_YUV_LO); + I915_WRITE(PIPE_CSC_MODE(pipe), 0); +} + /* Set up the pipe CSC unit. */ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state) { @@ -101,7 +142,10 @@ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state) uint16_t coeffs[9] = { 0, }; struct intel_crtc_state *intel_crtc_state = to_intel_crtc_state(crtc_state); - if (crtc_state->ctm) { + if (intel_crtc_state->ycbcr420) { + i9xx_load_ycbcr_conversion_matrix(intel_crtc); + return; + } else if (crtc_state->ctm) { struct drm_color_ctm *ctm = (struct drm_color_ctm *)crtc_state->ctm->data; uint64_t input[9] = { 0, }; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 788502a..fb60811 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6282,6 +6282,17 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, return -EINVAL; } + if (pipe_config->ycbcr420 && pipe_config->base.ctm) { + + /* +* There is only one pipe CSC unit per pipe, and we need that +* for output conversion from RGB->YCBCR. So if CTM is already +* applied we can't support YCBCR420 output. +*/ + DRM_DEBUG_KMS("YCBCR420 and CTM together are not possible\n"); + return -EINVAL; + } + /* * Pipe horizontal size must be even in:
[Intel-gfx] [PATCH v4 6/6] drm/i915/glk: set HDMI 2.0 identifier
This patch sets the is_hdmi2_src identifier in drm connector for GLK platform. GLK contains a native HDMI 2.0 controller. This identifier will help the EDID handling functions to save lot of work which is specific to HDMI 2.0 sources. V3: Added this patch V4: Rebase V4: Rebase V5: Added r-b from Ander V6: Rebase V7: Rebase V8: Rebase V9: Added r-b from Ville Cc: Ville Syrjala Cc: Ander Conselvan de Oliveira Reviewed-by: Ander Conselvan de Oliveira Reviewed-by: Ville Syrjala Signed-off-by: Shashank Sharma --- drivers/gpu/drm/i915/intel_hdmi.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index bedc6eb..7e103b7 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1914,6 +1914,9 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, connector->doublescan_allowed = 0; connector->stereo_allowed = 1; + if (IS_GEMINILAKE(dev_priv)) + connector->ycbcr_420_allowed = true; + intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port); switch (port) { -- 2.7.4 ___ 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: Explicit the connector name for DP link training result
== Series Details == Series: drm/i915: Explicit the connector name for DP link training result URL : https://patchwork.freedesktop.org/series/27410/ State : success == Summary == Series 27410v1 drm/i915: Explicit the connector name for DP link training result https://patchwork.freedesktop.org/api/1.0/series/27410/revisions/1/mbox/ Test kms_flip: Subgroup basic-flip-vs-modeset: skip -> PASS (fi-skl-x1585l) fdo#101781 fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:445s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:428s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:353s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:534s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:506s fi-byt-j1900 total:279 pass:255 dwarn:0 dfail:0 fail:0 skip:24 time:482s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:487s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:596s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:436s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:414s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:425s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:495s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:468s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:463s fi-kbl-7560u total:279 pass:268 dwarn:1 dfail:0 fail:0 skip:10 time:575s fi-kbl-r total:279 pass:260 dwarn:1 dfail:0 fail:0 skip:18 time:584s fi-pnv-d510 total:279 pass:221 dwarn:3 dfail:0 fail:0 skip:55 time:567s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:461s fi-skl-6700hqtotal:279 pass:262 dwarn:0 dfail:0 fail:0 skip:17 time:592s fi-skl-6700k total:279 pass:257 dwarn:4 dfail:0 fail:0 skip:18 time:472s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:484s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:438s fi-skl-x1585ltotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:487s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:540s fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:404s d93246177cf41d1731dae587923544b7a28659a6 drm-tip: 2017y-07m-17d-11h-27m-05s UTC integration manifest cfc64cd drm/i915: Explicit the connector name for DP link training result == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5210/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] lib/igt_debugfs: Update documentation and clenup
On Mon, Jul 17, 2017 at 04:59:48PM +0300, Arkadiusz Hiler wrote: > The documentation was lying. The igt_crc_to_string() is threadsafe and > does not return a pointer to an internal buffer. > > Actually the caller is responsible for the memory that is allocated (and > they are for all the current cases), so let's put that in the doc too. > > While I was at it I got rid of strdup() in favor of an early allocation. > > Cc: Martin Peres > Cc: Liviu Dudau Reviewed-by: Liviu Dudau > Signed-off-by: Arkadiusz Hiler > --- > lib/igt_debugfs.c | 9 - > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c > index 80f25c6..5e4d72c 100644 > --- a/lib/igt_debugfs.c > +++ b/lib/igt_debugfs.c > @@ -304,21 +304,20 @@ void igt_assert_crc_equal(const igt_crc_t *a, const > igt_crc_t *b) > * igt_crc_to_string: > * @crc: pipe CRC value to print > * > - * This formats @crc into a string buffer which is owned by > igt_crc_to_string(). > - * The next call will override the buffer again, which makes this > multithreading > - * unsafe. > + * This formats @crc into a string. Function allocates memory - the caller is > + * in charge of freeing it. > * > * This should only ever be used for diagnostic debug output. > */ > char *igt_crc_to_string(igt_crc_t *crc) > { > int i; > - char buf[128] = { 0 }; > + char *buf = calloc(128, sizeof(char)); > > for (i = 0; i < crc->n_words; i++) > sprintf(buf + strlen(buf), "%08x ", crc->crc[i]); > > - return strdup(buf); > + return buf; > } > > #define MAX_CRC_ENTRIES 10 > -- > 2.9.4 > -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 1/7] igt: lib/igt_crc: Split out CRC functionality
On Mon, Jul 17, 2017 at 04:50:27PM +0300, Arkadiusz Hiler wrote: > On Thu, Jul 06, 2017 at 05:14:18PM +0100, Liviu Dudau wrote: > > From: Brian Starkey > > > > Separate out the CRC code for better compartmentalisation. Should ease > > the addition of more/different CRC sources in the future. > > > > Signed-off-by: Brian Starkey > > Signed-off-by: Liviu Dudau > > > > --- > > lib/Makefile.sources | 2 + > > lib/igt_chamelium.h | 1 + > > lib/igt_crc.c | 563 > > ++ > > lib/igt_crc.h | 125 + > > lib/igt_debugfs.c | 547 > > > > lib/igt_debugfs.h | 81 -- > > tests/chamelium.c | 1 + > > tests/kms_atomic_transition.c | 1 + > > tests/kms_ccs.c | 1 + > > tests/kms_chv_cursor_fail.c | 1 + > > tests/kms_crtc_background_color.c | 1 + > > tests/kms_cursor_crc.c| 1 + > > tests/kms_cursor_legacy.c | 1 + > > tests/kms_draw_crc.c | 1 + > > tests/kms_fbc_crc.c | 1 + > > tests/kms_flip_tiling.c | 1 + > > tests/kms_frontbuffer_tracking.c | 1 + > > tests/kms_mmap_write_crc.c| 1 + > > tests/kms_mmio_vs_cs_flip.c | 1 + > > tests/kms_pipe_color.c| 1 + > > tests/kms_pipe_crc_basic.c| 1 + > > tests/kms_plane.c | 1 + > > tests/kms_plane_lowres.c | 1 + > > tests/kms_plane_multiple.c| 1 + > > tests/kms_plane_scaling.c | 1 + > > tests/kms_pwrite_crc.c| 1 + > > tests/kms_rotation_crc.c | 1 + > > tests/kms_universal_plane.c | 1 + > > tools/intel_display_crc.c | 1 + > > 29 files changed, 714 insertions(+), 628 deletions(-) > > create mode 100644 lib/igt_crc.c > > create mode 100644 lib/igt_crc.h > > > > diff --git a/lib/Makefile.sources b/lib/Makefile.sources > > index 53fdb54c..cfba15c9 100644 > > --- a/lib/Makefile.sources > > +++ b/lib/Makefile.sources > > @@ -11,6 +11,8 @@ lib_source_list = \ > > igt_debugfs.h \ > > igt_aux.c \ > > igt_aux.h \ > > + igt_crc.c \ > > + igt_crc.h \ > > igt_edid_template.h \ > > igt_gt.c\ > > igt_gt.h\ > > diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h > > index 81322ad2..ea5abc2e 100644 > > --- a/lib/igt_chamelium.h > > +++ b/lib/igt_chamelium.h > > @@ -31,6 +31,7 @@ > > #endif > > > > #include "igt.h" > > +#include "igt_crc.h" > > #include > > > > struct chamelium; > > diff --git a/lib/igt_crc.c b/lib/igt_crc.c > > new file mode 100644 > > index ..91a0b5a8 > > --- /dev/null > > +++ b/lib/igt_crc.c > > @@ -0,0 +1,563 @@ > > +/* > > + * Copyright © 2013 Intel Corporation > > + * > > + * Permission is hereby granted, free of charge, to any person obtaining a > > + * copy of this software and associated documentation files (the > > "Software"), > > + * to deal in the Software without restriction, including without > > limitation > > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > > + * and/or sell copies of the Software, and to permit persons to whom the > > + * Software is furnished to do so, subject to the following conditions: > > + * > > + * The above copyright notice and this permission notice (including the > > next > > + * paragraph) shall be included in all copies or substantial portions of > > the > > + * Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS > > OR > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR > > OTHER > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > > DEALINGS > > + * IN THE SOFTWARE. > > + * > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include "igt_aux.h" > > +#include "igt_crc.h" > > +#include "igt_core.h" > > +#include "igt_debugfs.h" > > +#include "igt_kms.h" > > + > > +/** > > + * igt_assert_crc_equal: > > + * @a: first pipe CRC value > > + * @b: second pipe CRC value > > + * > > + * Compares two CRC values and fails the testcase if they don't match with > > + * igt_fail(). Note that due to CRC collisions CRC based testcase can only > > + * assert that CRCs match, never that they are different. Otherwise there > > might > > + * be random testcase failures when different screen contents end up with > > the > > + * same CRC by chance. > > + */ > > +void igt_assert_crc_
Re: [Intel-gfx] [PATCH] drm: Don't complain too much about struct_mutex.
On Mon, Jul 17, 2017 at 11:35 AM, Peter Zijlstra wrote: > On Sat, Jul 15, 2017 at 11:53:28AM +0200, Daniel Vetter wrote: >> A more complete solution would be to do the mutex_init in the drm core >> only for legacy drivers, plus add it to each modern driver that still >> needs it, which would also give each its own lockdep key. Trying to do >> that dynamically doesn't work, because lockdep requires it's keys to >> be statically allocated. > > Would something like the below work? Its not pretty, but would give each > user of drm_dev_init() its own key. We're very close to getting rid of struct_mutex, neither nouveau.ko nor i915.ko use it, and only about 4 drivers really still need it (out of about 30+). I think the minimal patch is enough, and then not initializing the mutex to prevent more users croping up again in the future. -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] ✓ Fi.CI.BAT: success for YCBCR 4:2:0 handling in i915 layer
== Series Details == Series: YCBCR 4:2:0 handling in i915 layer URL : https://patchwork.freedesktop.org/series/27412/ State : success == Summary == Series 27412v1 YCBCR 4:2:0 handling in i915 layer https://patchwork.freedesktop.org/api/1.0/series/27412/revisions/1/mbox/ Test gem_exec_suspend: Subgroup basic-s4-devices: dmesg-warn -> PASS (fi-kbl-7560u) k.org#196399 Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: fail -> PASS (fi-snb-2600) fdo#100215 Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-b: dmesg-warn -> PASS (fi-pnv-d510) fdo#101597 Subgroup suspend-read-crc-pipe-b: pass -> DMESG-WARN (fi-byt-j1900) fdo#101705 k.org#196399 https://bugzilla.kernel.org/show_bug.cgi?id=196399 fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597 fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:443s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:431s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:351s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:521s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:507s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:489s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:482s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:593s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:434s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:417s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:426s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:500s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:472s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:462s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:574s fi-kbl-r total:279 pass:260 dwarn:1 dfail:0 fail:0 skip:18 time:584s fi-pnv-d510 total:279 pass:222 dwarn:2 dfail:0 fail:0 skip:55 time:559s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:457s fi-skl-6700hqtotal:279 pass:262 dwarn:0 dfail:0 fail:0 skip:17 time:583s fi-skl-6700k total:279 pass:257 dwarn:4 dfail:0 fail:0 skip:18 time:468s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:484s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:437s fi-skl-x1585ltotal:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:479s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:541s fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:407s d93246177cf41d1731dae587923544b7a28659a6 drm-tip: 2017y-07m-17d-11h-27m-05s UTC integration manifest 12c2f27 drm/i915/glk: set HDMI 2.0 identifier f75c4bf drm/i915: set colorspace for YCBCR420 outputs 5306c52 drm/i915: prepare csc unit for YCBCR420 output a0f0ce3 drm/i915: prepare pipe for YCBCR420 output d00f31e drm/i915: prepare scaler for YCBCR420 modeset 2df8924 drm/i915: add config function for YCBCR420 outputs == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5211/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] lib/igt_debugfs: Update documentation and clenup
clenup -> cleanup On 17/07/17 18:05, Liviu Dudau wrote: On Mon, Jul 17, 2017 at 04:59:48PM +0300, Arkadiusz Hiler wrote: The documentation was lying. The igt_crc_to_string() is threadsafe and does not return a pointer to an internal buffer. Actually the caller is responsible for the memory that is allocated (and they are for all the current cases), so let's put that in the doc too. While I was at it I got rid of strdup() in favor of an early allocation. Cc: Martin Peres Cc: Liviu Dudau Reviewed-by: Liviu Dudau Signed-off-by: Arkadiusz Hiler --- lib/igt_debugfs.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c index 80f25c6..5e4d72c 100644 --- a/lib/igt_debugfs.c +++ b/lib/igt_debugfs.c @@ -304,21 +304,20 @@ void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) * igt_crc_to_string: * @crc: pipe CRC value to print * - * This formats @crc into a string buffer which is owned by igt_crc_to_string(). - * The next call will override the buffer again, which makes this multithreading - * unsafe. + * This formats @crc into a string. Function allocates memory - the caller is + * in charge of freeing it. These sentences are a little strange from a grammatical point of view. How about: This function allocates a string and formats @crc into it. The caller is responsible for freeing the string. * * This should only ever be used for diagnostic debug output. */ char *igt_crc_to_string(igt_crc_t *crc) { int i; - char buf[128] = { 0 }; + char *buf = calloc(128, sizeof(char)); To mimic the previous code's behaviour, please add the following: if (!buf) return NULL; for (i = 0; i < crc->n_words; i++) sprintf(buf + strlen(buf), "%08x ", crc->crc[i]); - return strdup(buf); + return buf; } #define MAX_CRC_ENTRIES 10 -- 2.9.4 With at least the title and code fixed, this is: Reviewed-by: Martin Peres Thanks for doing this :) Martin - Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/atomic-helper: Fix leak in disable_all
On Mon, Jul 17, 2017 at 11:39:56AM +0200, Maarten Lankhorst wrote: > Op 15-07-17 om 11:31 schreef Daniel Vetter: > > The legacy plane->fb pointer is refcounted by calling > > drm_atomic_clean_old_fb(). > > > > In practice this isn't a real problem because: > > - The caller in the i915 gpu reset code restores the original state > > again, which means the plane->fb pointer won't change, hence can't > > leak. > > - Drivers using drm_atomic_helper_shutdown call the fbdev cleanup > > first, and that usually cleans up the fb through > > drm_remove_framebuffer, which does this correctly. > > - Without fbdev the only framebuffers are from userspace, and those > > get cleaned up (again using drm_remove_framebuffer) befor the driver > > can even be unloaded. > > > > But in i915 I've switched the cleanup sequence around so that the > > _shutdown() calls happens after the drm_remove_framebuffer(), which is > > how I discovered this issue. > > > > v2: My analysis why the current code was ok for gpu reset and > > suspend/resume was correct, but then I totally failed to realize that > > we better keep this symmetric. Thanksfully CI noticed that for > > balance, a refcounting bug must exist at 2 places if previously there > > was no issue ... > > > > v3: Don't be lazy and compute the plane_mask in > > commit_duplicated_state properly too, instead of just using ~0U. > > > > Cc: martin.pe...@free.fr > > Cc: ch...@chris-wilson.co.uk > > Acked-by: Dave Airlie (v1) > > Signed-off-by: Daniel Vetter > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 18 -- > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c > > index b07fc30372d3..545328a9a769 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -2726,6 +2726,7 @@ int drm_atomic_helper_disable_all(struct drm_device > > *dev, > > struct drm_plane *plane; > > struct drm_crtc_state *crtc_state; > > struct drm_crtc *crtc; > > + unsigned plane_mask = 0; > > int ret, i; > > > > state = drm_atomic_state_alloc(dev); > > @@ -2768,10 +2769,14 @@ int drm_atomic_helper_disable_all(struct drm_device > > *dev, > > goto free; > > > > drm_atomic_set_fb_for_plane(plane_state, NULL); > > + plane_mask |= BIT(drm_plane_index(plane)); > > + plane->old_fb = plane->fb; > > } > > > > ret = drm_atomic_commit(state); > > free: > > + if (plane_mask) > > + drm_atomic_clean_old_fb(dev, plane_mask, ret); > > drm_atomic_state_put(state); > > return ret; > > } > > @@ -2902,11 +2907,16 @@ int > > drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state, > > struct drm_connector_state *new_conn_state; > > struct drm_crtc *crtc; > > struct drm_crtc_state *new_crtc_state; > > + unsigned plane_mask = 0; > > + struct drm_device *dev = state->dev; > > + int ret; > > > > state->acquire_ctx = ctx; > > > > - for_each_new_plane_in_state(state, plane, new_plane_state, i) > > + for_each_new_plane_in_state(state, plane, new_plane_state, i) { > > + plane_mask |= BIT(drm_plane_index(plane)); > We should really add a drm_plane_mask/drm_connector_mask at some point, to > complement drm_crtc_mask. > > state->planes[i].old_state = plane->state; > > + } > > > > for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) > > state->crtcs[i].old_state = crtc->state; > > @@ -2914,7 +2924,11 @@ int drm_atomic_helper_commit_duplicated_state(struct > > drm_atomic_state *state, > > for_each_new_connector_in_state(state, connector, new_conn_state, i) > > state->connectors[i].old_state = connector->state; > > > > - return drm_atomic_commit(state); > > + ret = drm_atomic_commit(state); > > + if (plane_mask) > > + drm_atomic_clean_old_fb(dev, plane_mask, ret); > Kill the if () part, and make it unconditional? On second thought, I should > have done the same in drm_framebuffer.c I'd prefer to not bikeshed this stuff too much and just go with what we do everywhere else (i.e. rmfb code and atomic commit) for consistency. clean_old_fb is definitely a horrible function with misleading kerneldoc on top, and I think the best way to clean that up would be to: - Move the plane_mask computation in drm_atmic_commit and also put the clean_old_fb call in there. Maybe give it a more descriptive name like update_legacy_fb or whatever. Unexport them. - This would break the compat helpers, where drm_atomic_commit must not update the legacy refcounts, because for set_config, page_flip and the plane hooks the core does that already. Create a drm_atomic_commit_legacy for these. But since one of my patches caused an existing issue to pop up as a regression, and this series fixes it, I'd like to first get the minimal fix in through drm-
[Intel-gfx] [Bug Report] Weekly progress report ww28
Highlights: - Bugs open during the week: 17 - Bugs closed during the week: 8 - Bugs labeled ReadyForDev during the week: 9 - Total bugs remain open: 327 - Total bugs labeled ReadyForDev: 130 Details: - Bugs open during the week(priority): 17 +--+---+ | Priority | Total | +--+---+ | Highest | 2 | | High | 2 | | Medium | 13| | Low | 0 | | Lowest | 0 | +--+---+ https://bugs.freedesktop.org/buglist.cgi?product=DRI&component=DRM%2FIntel&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&bug_status=RESOLVED&bug_status=CLOSED&bug_status=NEEDINFO&bug_status=VERIFIED&f1=creation_ts&o1=greaterthaneq&v1=2017-07-08&f2=creation_ts&o2=lessthaneq&v2=2017-07-15 - Bugs closed during the week(by priority): 8 +--+---+ | Priority | Total | +--+---+ | Highest | 1 | | High | 1 | | Medium | 6 | | Low | 0 | | Lowest | 0 | +--+---+ https://bugs.freedesktop.org/buglist.cgi?product=DRI&component=DRM%2FIntel&bug_status=CLOSED&bug_status=RESOLVED&bug_status=VERIFIED&&chfield=bug_status&chfieldvalue=RESOLVED&chfieldfrom=2017-07-08&chfieldto=2017-07-15 - ReadyForDev during the week(by priority): 9 +--+---+ | Priority | Total | +--+---+ | Highest | 2 | | High | 1 | | Medium | 6 | | Low | 0 | | Lowest | 0 | +--+---+ https://bugs.freedesktop.org/buglist.cgi?product=DRI&component=DRM%2FIntel&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&bug_status=NEEDINFO&status_whiteboard=ReadyForDev&status_whiteboard_type=allwordssubstr&f1=creation_ts&o1=greaterthaneq&v1=2017-07-08&f2=creation_ts&o2=lessthaneq&v2=2017-07-15 - Total Bugs(by priority): 327 +--+---+ | Priority | Total | +--+---+ | Highest | 19| | High | 45| | Medium | 251 | | Low | 10| | Lowest | 2 | +--+---+ https://bugs.freedesktop.org/buglist.cgi?product=DRI&component=DRM%2FIntel&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&bug_status=NEEDINFO&f1=creation_ts&o1=lessthan&v1=2017-07-15 - Total ReadyForDev(by priority): 130 +--+---+ | Priority | Total | +--+---+ | Highest | 13| | High | 35| | Medium | 81| | Low | 1 | | Lowest | 0 | +--+---+ https://bugs.freedesktop.org/buglist.cgi?product=DRI&component=DRM%2FIntel&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&bug_status=NEEDINFO&status_whiteboard=ReadyForDev&status_whiteboard_type=allwordssubstr&f1=creation_ts&o1=lessthan&v1=2017-07-15 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [Regression report] Weekly regression report WW27
Link to FDO regression list: https://bugs.freedesktop.org/buglist.cgi?bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&bug_status=NEEDINFO&component=DRM%2FIntel&f0=OP&f1=OP&f2=short_desc&f3=keywords&f4=CP&f5=CP&j1=OR&known_name=i915%20regressions&list_id=600614&o2=anywordssubstr&o3=anywordssubstr&op_sys=All&op_sys=Linux%20%28All%29&op_sys=other&order=bug_id%20DESC&product=DRI&query_based_on=i915%20regressions&query_format=advanced&rep_platform=All&rep_platform=x86%20%28IA32%29&rep_platform=x86-64%20%28AMD64%29&rep_platform=IA64%20%28Itanium%29&rep_platform=Other&v2=regression%20bisect&v3=regression%20bisected%20pending_bisect Total regressions: 23 This week regressions:1 ++---+++ | BugId | Summary | Created on | Bisect | ++---+++ | 101790 | [Regression] Cursor does not move | 2017-07-14 | No | ++---+++ Previous regressions:22 ++--+++ | BugId | Summary | Created on | Bisect | ++--+++ | 99448 | [SNB] HDMI Output Fault - Unstable Display (drm_dp_dpcd_access Error in dmesg). | 2017-01-18 | No | | 99322 | [SKL] Flashing black screen with dual monitors | 2017-01-08 | No | | 99093 | [PNV][BLK][ELK][BXT/SKL/KBL/BDW/BSW/GLK][regression] N450 and D510 machines get | 2016-12-15 | Yes| | 98517 | Skylake gen6 suspend/resume video regression 4.9 | 2016-10-31 | No | | 97918 | [bdw regression 4.8] Severe graphics regression, rainbow glitching and flickerin | 2016-09-25 | No | | 95736 | [SNB bisected] *ERROR* uncleared fifo underrun on pipe A | 2016-05-24 | Yes| | 93782 | [i9xx TV][BISECT] vblank wait timeout on crtc | 2016-01-19 | Yes| | 91974 | [bisected] unrecoverable black screen after killing X | 2015-09-11 | Yes| | 88124 | i915: regression: after DP connected (via docking station) monitor is turned off | 2015-01-06 | Yes| | 101679 | [regression] display corruption since bb0f4aab0e7677e9 | 2017-07-03 | Yes| | 101639 | [KBL] External display pixelated after upgrade to kernel >= 4.11.0-rc1 | 2017-06-29 | No | | 101635 | [IGT] [HSW/GLK/BSW] [regresion] gem_exec_reloc some subtest causes assertion fai | 2017-06-28 | No | | 101577 | [HSW] System freezes when fbc is enabled and deep package states are allowed | 2017-06-24 | Yes| | 101555 | [IGT] [SKL/BDW/IVB/SND] Regression igt@kms_pwrite_crc fail extended list | 2017-06-22 | No | | 101373 | [KBL] modeset to a different refresh rate periodically results in a black screen | 2017-06-10 | No | | 10 | [Regression BDW] backlight flickering/display artifacting on Broadwell integrate | 2017-05-20 | No | | 101049 | [IVB] [bisected] Freeze when booting with kernel 4.9 and later | 2017-05-15 | Yes| | 100907 | [i965 bisected] boot hang on Dell D830 with kernel > 4.7 | 2017-05-02 | Yes| | 100725 | [i915] oops in intel_update_cursor_plane(): "BUG: unable to handle kernel NULL p | 2017-04-19 | No | | 100617 | [ILK][Regression bisected] Gnome Shell slow animations | 2017-04-08 | Yes| | 100610 | WARNING: CPU: 1 PID: 16615 at drivers/gpu/drm/drm_debugfs_crc.c:185 crtc_crc_ope | 2017-04-07 | Yes| | 100414 | [IVB] Refresh rate changes constantly with ONKYO AVR | 2017-03-27 | No | ++--+++___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Don't complain too much about struct_mutex.
On Mon, Jul 17, 2017 at 05:06:42PM +0200, Daniel Vetter wrote: > On Mon, Jul 17, 2017 at 11:35 AM, Peter Zijlstra wrote: > > On Sat, Jul 15, 2017 at 11:53:28AM +0200, Daniel Vetter wrote: > >> A more complete solution would be to do the mutex_init in the drm core > >> only for legacy drivers, plus add it to each modern driver that still > >> needs it, which would also give each its own lockdep key. Trying to do > >> that dynamically doesn't work, because lockdep requires it's keys to > >> be statically allocated. > > > > Would something like the below work? Its not pretty, but would give each > > user of drm_dev_init() its own key. > > We're very close to getting rid of struct_mutex, neither nouveau.ko > nor i915.ko use it, and only about 4 drivers really still need it (out > of about 30+). I think the minimal patch is enough, and then not > initializing the mutex to prevent more users croping up again in the > future. Whatever you want of course. I was just going by that one paragraphy I quoted :-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v4 2/7] chamelium: Calculate CRC from framebuffer instead of hardcoding it
On Wed, 2017-07-12 at 17:50 +0300, Paul Kocialkowski wrote: > This introduces CRC calculation for reference frames, instead of > using > hardcoded values for them. The rendering of reference frames may > differ > from machine to machine, especially due to font rendering, and the > frame itself may change with subsequent IGT changes. > > These differences would cause the CRC checks to fail on different > setups. This allows them to pass regardless of the setup. Just one question before I push this since I didn't think of testing this previously and don't have access to my chamelium at the moment. Have you made sure that this doesn't break things with igt if a test gets interrupted due to failure in the middle of an asynchronous CRC calculation? Other then that, everything here looks good. > > Signed-off-by: Paul Kocialkowski > --- > lib/igt_chamelium.c | 143 > > lib/igt_chamelium.h | 5 ++ > tests/chamelium.c | 77 +++- > 3 files changed, 167 insertions(+), 58 deletions(-) > > diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c > index 93392af7..baa6399c 100644 > --- a/lib/igt_chamelium.c > +++ b/lib/igt_chamelium.c > @@ -94,6 +94,14 @@ struct chamelium_frame_dump { > struct chamelium_port *port; > }; > > +struct chamelium_fb_crc_async_data { > + int fd; > + struct igt_fb *fb; > + > + pthread_t thread_id; > + igt_crc_t *ret; > +}; > + > struct chamelium { > xmlrpc_env env; > xmlrpc_client *client; > @@ -998,6 +1006,141 @@ int chamelium_get_frame_limit(struct chamelium > *chamelium, > return ret; > } > > +static uint32_t chamelium_xrgb_hash16(const unsigned char *buffer, > int width, > + int height, int k, int m) > +{ > + unsigned char r, g, b; > + uint64_t sum = 0; > + uint64_t count = 0; > + uint64_t value; > + uint32_t hash; > + int index; > + int i; > + > + for (i=0; i < width * height; i++) { > + if ((i % m) != k) > + continue; > + > + index = i * 4; > + > + r = buffer[index + 2]; > + g = buffer[index + 1]; > + b = buffer[index + 0]; > + > + value = r | (g << 8) | (b << 16); > + sum += ++count * value; > + } > + > + hash = ((sum >> 0) ^ (sum >> 16) ^ (sum >> 32) ^ (sum >> > 48)) & 0x; > + > + return hash; > +} > + > +static void chamelium_do_calculate_fb_crc(int fd, struct igt_fb *fb, > + igt_crc_t *out) > +{ > + unsigned char *buffer; > + cairo_surface_t *fb_surface; > + int n = 4; > + int w, h; > + int i, j; > + > + /* Get the cairo surface for the framebuffer */ > + fb_surface = igt_get_cairo_surface(fd, fb); > + > + buffer = cairo_image_surface_get_data(fb_surface); > + w = fb->width; > + h = fb->height; > + > + for (i = 0; i < n; i++) { > + j = n - i - 1; > + out->crc[i] = chamelium_xrgb_hash16(buffer, w, h, j, > n); > + } > + > + out->n_words = n; > +} > + > +/** > + * chamelium_calculate_fb_crc: > + * @fd: The drm file descriptor > + * @fb: The framebuffer to calculate the CRC for > + * > + * Calculates the CRC for the provided framebuffer, using the > Chamelium's CRC > + * algorithm. This calculates the CRC in a synchronous fashion. > + * > + * Returns: The calculated CRC > + */ > +igt_crc_t *chamelium_calculate_fb_crc(int fd, struct igt_fb *fb) > +{ > + igt_crc_t *ret = calloc(1, sizeof(igt_crc_t)); > + > + chamelium_do_calculate_fb_crc(fd, fb, ret); > + > + return ret; > +} > + > +static void *chamelium_calculate_fb_crc_async_work(void *data) > +{ > + struct chamelium_fb_crc_async_data *fb_crc; > + > + fb_crc = (struct chamelium_fb_crc_async_data *) data; > + > + chamelium_do_calculate_fb_crc(fb_crc->fd, fb_crc->fb, > fb_crc->ret); > + > + return NULL; > +} > + > +/** > + * chamelium_calculate_fb_crc_launch: > + * @fd: The drm file descriptor > + * @fb: The framebuffer to calculate the CRC for > + * > + * Launches the CRC calculation for the provided framebuffer, using > the > + * Chamelium's CRC algorithm. This calculates the CRC in an > asynchronous > + * fashion. > + * > + * The returned structure should be passed to a subsequent call to > + * chamelium_calculate_fb_crc_result. It should not be freed. > + * > + * Returns: An intermediate structure for the CRC calculation work. > + */ > +struct chamelium_fb_crc_async_data > *chamelium_calculate_fb_crc_async_start(int fd, > + >struct igt_fb *fb) > +{ > + struct chamelium_fb_crc_async_data *fb_crc; > + > + fb_crc = calloc(1, sizeof(struct > chamelium_fb_crc_async_data)); > + fb_crc->ret = calloc(1, sizeof(igt_crc_t)); > + fb_crc->fd = fd; > + fb_crc->fb = fb; > + > + pthread_create(&fb_crc
Re: [Intel-gfx] [PATCH i-g-t v4 0/7] CRC testing with Chamelium improvements
The only thing that needs changing is the thing I mentioned on patch #2. After you make sure exiting prematurely doesn't cause the rest of IGT to segfault or something like that, that should hopefully be the last change that will need to be done. Afterwards I will push the whole series at once. Thanks for all the great work! And the changelist ;) On Wed, 2017-07-12 at 17:50 +0300, Paul Kocialkowski wrote: > Changes since v3: > * Renamed structure used by async crc calculation for more clarity > * Used const qualifier for untouched buffer when hashing > * Split actual calculation to a dedicated function > * Reworked async functions names for more clarity > * Reworked descriptions for better accuracy > * Exported framebuffer cairo surface and use it directly instead of > (unused) png dumping > * Fix how the framebuffer cairo surface is obtained to avoid > destroying > it too early > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Unbreak gpu reset vs. modeset locking
Op 15-07-17 om 13:40 schreef Daniel Vetter: > Taking the modeset locks unconditionally isn't the greatest idea, > because atm that part is still broken and times out (and then atomic > keels over). And there's really no reason to do so, the old code > didn't do that either. > > To make the patch a bit simpler let's also nuke 2 cases that are only > around for the old mmioflip paths. Atomic nonblocking workers will not > die (minus bugs) when a gpu reset happens. > > And of course this doesn't fix any of the gpu reset vs. modeset > deadlock fun, but it at least stop modern CI machines from keeling > over all over the place for no reason at all. > > And we still have the explicit testcases to run the fake gpu reset, so > coverage isn't that much worse. > > v2: Split out additional changes on top, restrict this to purely reducing > the critical section of modeset locks. > > Fixes: 739748939974 ("drm/i915: Fix modeset handling during gpu reset, v5.") > Cc: Maarten Lankhorst > Cc: Ville Syrjälä > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_display.c | 53 > +--- > 1 file changed, 13 insertions(+), 40 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index f69333b8995c..e3c55a996f6b 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3413,26 +3413,6 @@ static void intel_complete_page_flips(struct > drm_i915_private *dev_priv) > intel_finish_page_flip_cs(dev_priv, crtc->pipe); > } > > -static void intel_update_primary_planes(struct drm_device *dev) > -{ > - struct drm_crtc *crtc; > - > - for_each_crtc(dev, crtc) { > - struct intel_plane *plane = to_intel_plane(crtc->primary); > - struct intel_plane_state *plane_state = > - to_intel_plane_state(plane->base.state); > - > - if (plane_state->base.visible) { > - trace_intel_update_plane(&plane->base, > - to_intel_crtc(crtc)); > - > - plane->update_plane(plane, > - to_intel_crtc_state(crtc->state), > - plane_state); > - } > - } > -} > - > static int > __intel_display_resume(struct drm_device *dev, > struct drm_atomic_state *state, > @@ -3485,6 +3465,12 @@ void intel_prepare_reset(struct drm_i915_private > *dev_priv) > struct drm_atomic_state *state; > int ret; > > + > + /* reset doesn't touch the display, but flips might get nuked anyway, */ I think this comment was meant to address the reasoning for taking all locks, so the part about flips being nuked no longer applies with mmio flips gone. > + if (!i915.force_reset_modeset_test && > + !gpu_reset_clobbers_display(dev_priv)) > + return; > + > /* >* Need mode_config.mutex so that we don't >* trample ongoing ->detect() and whatnot. > @@ -3498,12 +3484,6 @@ void intel_prepare_reset(struct drm_i915_private > *dev_priv) > > drm_modeset_backoff(ctx); > } > - > - /* reset doesn't touch the display, but flips might get nuked anyway, */ > - if (!i915.force_reset_modeset_test && > - !gpu_reset_clobbers_display(dev_priv)) > - return; > - > /* >* Disabling the crtcs gracefully seems nicer. Also the >* g33 docs say we should at least disable all the planes. > @@ -3533,6 +3513,11 @@ void intel_finish_reset(struct drm_i915_private > *dev_priv) > struct drm_atomic_state *state = dev_priv->modeset_restore_state; > int ret; > > + /* reset doesn't touch the display, but flips might get nuked anyway, */ > + if (!i915.force_reset_modeset_test && > + !gpu_reset_clobbers_display(dev_priv)) > + return; Same thing about comment here. Perhaps change the if (!i915.force_reset_modeset_test && !gpu_reset_clobbers_display()) to if (!state && !gpu_reset_clobbers_display()) so we don't run into a kernel panic if we change the parameter during reset? Hm perhaps also either add if (state) to the __intel_display_resume call for ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Consistently use enum pipe for PCH transcoders
El Mon, Jul 17, 2017 at 11:07:01AM +0200 Daniel Vetter ha dit: > On Fri, Jul 14, 2017 at 06:04:03PM -0700, Matthias Kaehlcke wrote: > > The current code uses two different enum types for PCH transcoders and > > performs implicit conversions between the two types. This is error prone > > and causes clang to raise warnings like this: > > > > drivers/gpu/drm/i915/intel_dp.c:3546:51: warning: implicit conversion > > from enumeration type 'enum pipe' to different enumeration type > > 'enum transcoder' [-Wenum-conversion] > > intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false); > > > > Consistently use the type enum pipe for PCH transcoders. > > > > Signed-off-by: Matthias Kaehlcke > > Thanks for respinning. Unfortunately it doesn't apply cleanly to > drm-intel-next-queued, and I don't have a clang setup ready to confirm I > didn't screw up anything. Can you pls rebase? Thanks for having a look. The patch was against v4.12, I will rebase it on drm-intel-next-queued. > > --- > > drivers/gpu/drm/i915/i915_irq.c| 10 +- > > drivers/gpu/drm/i915/intel_display.c | 24 ++-- > > drivers/gpu/drm/i915/intel_drv.h | 6 +++--- > > drivers/gpu/drm/i915/intel_fifo_underrun.c | 6 +++--- > > 4 files changed, 21 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > b/drivers/gpu/drm/i915/i915_irq.c > > index 190f6aa5d15e..7960d2170750 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -2132,10 +2132,10 @@ static void ibx_irq_handler(struct drm_i915_private > > *dev_priv, u32 pch_iir) > > DRM_DEBUG_DRIVER("PCH transcoder CRC error interrupt\n"); > > > > if (pch_iir & SDE_TRANSA_FIFO_UNDER) > > - intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A); > > + intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A); > > > > if (pch_iir & SDE_TRANSB_FIFO_UNDER) > > - intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B); > > + intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B); > > } > > > > static void ivb_err_int_handler(struct drm_i915_private *dev_priv) > > @@ -2169,13 +2169,13 @@ static void cpt_serr_int_handler(struct > > drm_i915_private *dev_priv) > > DRM_ERROR("PCH poison interrupt\n"); > > > > if (serr_int & SERR_INT_TRANS_A_FIFO_UNDERRUN) > > - intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A); > > + intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A); > > > > if (serr_int & SERR_INT_TRANS_B_FIFO_UNDERRUN) > > - intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B); > > + intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B); > > > > if (serr_int & SERR_INT_TRANS_C_FIFO_UNDERRUN) > > - intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_C); > > + intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_C); > > > > I915_WRITE(SERR_INT, serr_int); > > } > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 9106ea32b048..21a8fea46ad9 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -1782,7 +1782,7 @@ static void lpt_enable_pch_transcoder(struct > > drm_i915_private *dev_priv, > > > > /* FDI must be feeding us bits for PCH ports */ > > assert_fdi_tx_enabled(dev_priv, (enum pipe) cpu_transcoder); > > - assert_fdi_rx_enabled(dev_priv, TRANSCODER_A); > > + assert_fdi_rx_enabled(dev_priv, PIPE_A); > > > > /* Workaround: set timing override bit. */ > > val = I915_READ(TRANS_CHICKEN2(PIPE_A)); > > @@ -1858,16 +1858,16 @@ void lpt_disable_pch_transcoder(struct > > drm_i915_private *dev_priv) > > I915_WRITE(TRANS_CHICKEN2(PIPE_A), val); > > } > > > > -enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc) > > +enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc) > > { > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > > WARN_ON(!crtc->config->has_pch_encoder); > > > > if (HAS_PCH_LPT(dev_priv)) > > - return TRANSCODER_A; > > + return PIPE_A; > > else > > - return (enum transcoder) crtc->pipe; > > + return crtc->pipe; > > } > > > > /** > > @@ -1906,7 +1906,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc) > > if (crtc->config->has_pch_encoder) { > > /* if driving the PCH, we need FDI enabled */ > > assert_fdi_rx_pll_enabled(dev_priv, > > - (enum pipe) > > intel_crtc_pch_transcoder(crtc)); > > + > > intel_crtc_pch_transcoder(crtc)); > > assert_fdi_tx_pll_enabled(dev_priv, > > (enum pipe) cpu_transcoder); > >
Re: [Intel-gfx] [PATCH i-g-t v2 2/2] chamelium: Add support for VGA frame comparison testing
Just one change for this patch below On Wed, 2017-07-12 at 17:57 +0300, Paul Kocialkowski wrote: > This adds support for VGA frame comparison testing with the reference > generated from cairo. The retrieved frame from the chamelium is first > cropped, as it contains the blanking intervals, through a dedicated > helper. Another helper function asserts that the analogue frame > matches or dump it to png if not. > > Signed-off-by: Paul Kocialkowski > --- > lib/igt_chamelium.c | 164 > ++-- > lib/igt_chamelium.h | 7 ++- > lib/igt_frame.c | 6 +- > tests/chamelium.c | 57 ++ > 4 files changed, 225 insertions(+), 9 deletions(-) > > diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c > index df49936b..8701192e 100644 > --- a/lib/igt_chamelium.c > +++ b/lib/igt_chamelium.c > @@ -938,6 +938,8 @@ static cairo_surface_t > *convert_frame_dump_argb32(const struct chamelium_frame_d > int w = dump->width, h = dump->height; > uint32_t *bits_bgr = (uint32_t *) dump->bgr; > unsigned char *bits_argb; > + unsigned char *bits_target; > + int size; > > image_bgr = pixman_image_create_bits( > PIXMAN_b8g8r8, w, h, bits_bgr, > @@ -947,9 +949,13 @@ static cairo_surface_t > *convert_frame_dump_argb32(const struct chamelium_frame_d > > bits_argb = (unsigned char *) > pixman_image_get_data(image_argb); > > - dump_surface = cairo_image_surface_create_for_data( > - bits_argb, CAIRO_FORMAT_ARGB32, w, h, > - PIXMAN_FORMAT_BPP(PIXMAN_x8r8g8b8) / 8 * w); > + dump_surface = cairo_image_surface_create( > + CAIRO_FORMAT_ARGB32, w, h); > + > + bits_target = cairo_image_surface_get_data(dump_surface); > + size = cairo_image_surface_get_stride(dump_surface) * h; > + memcpy(bits_target, bits_argb, size); > + cairo_surface_mark_dirty(dump_surface); > > pixman_image_unref(image_argb); > > @@ -1055,6 +1061,154 @@ void chamelium_assert_crc_eq_or_dump(struct > chamelium *chamelium, > } > > /** > + * chamelium_assert_analogue_frame_match_or_dump: > + * @chamelium: The chamelium instance the frame dump belongs to > + * @frame: The chamelium frame dump to match > + * @fb: pointer to an #igt_fb structure > + * > + * Asserts that the provided captured frame matches the reference > frame from > + * the framebuffer. If they do not, this saves the reference and > captured frames > + * to a png file. > + */ > +void chamelium_assert_analogue_frame_match_or_dump(struct chamelium > *chamelium, > + struct > chamelium_port *port, > + const struct > chamelium_frame_dump *frame, > + struct igt_fb *fb) > +{ > + cairo_surface_t *reference; > + cairo_surface_t *capture; > + igt_crc_t *reference_crc; > + igt_crc_t *capture_crc; > + char *reference_suffix; > + char *capture_suffix; > + bool match; > + > + /* Grab the reference frame from framebuffer */ > + reference = igt_get_cairo_surface(chamelium->drm_fd, fb); > + > + /* Grab the captured frame from chamelium */ > + capture = convert_frame_dump_argb32(frame); > + > + match = igt_check_analogue_frame_match(reference, capture); > + if (!match && igt_frame_dump_is_enabled()) { > + reference_crc = chamelium_calculate_fb_crc(chamelium- > >drm_fd, > + fb); > + capture_crc = chamelium_get_crc_for_area(chamelium, > port, 0, 0, > +0, 0); > + > + reference_suffix = > igt_crc_to_string_extended(reference_crc, > + '-', > 2); > + capture_suffix = > igt_crc_to_string_extended(capture_crc, '-', > + 2); > + > + /* Write reference and capture frames to png */ > + igt_write_compared_frames_to_png(reference, capture, > +reference_suffix, > +capture_suffix); > + > + free(reference_suffix); > + free(capture_suffix); > + } > + > + cairo_surface_destroy(capture); > + > + igt_assert(match); > +} > + > + > +/** > + * chamelium_analogue_frame_crop: > + * @chamelium: The Chamelium instance to use > + * @dump: The chamelium frame dump to crop > + * @width: The cropped frame width > + * @height: The cropped frame height > + * > + * Detects the corners of a chamelium frame and crops it to the > requested > + * width/height. This is useful for VGA frame dumps that also > contain the > + * pixels dumped during the blanking intervals. > + * > + * The d
Re: [Intel-gfx] [PATCH i-g-t 0/1] chamelium: Add support for VGA frame comparison testing
Hey! The only changes I need on this is the small fix I mentioned for patch #2, and a nitpick regarding the spellings being used for the functions. I wasn't sure whether or not we should be using analog or analogue (us spelling vs. everyone else) but apparently the answer is to use the US spellings: 13:15 hm, weird question: what nationality's english spellings do we actually prefer for igt functions? like, would it be igt_check_analogue_frame_match() or igt_check_analog_frame_match(), or does no one really care? 13:16 → DottorLeo (~leona...@net-93-151-90-55.cust.dsl.teletu.it) has joined #intel-gfx 13:18 mupuf: ^ ? 13:19 oh dear 13:19 danvet: ^ 13:20 I think personally I do us spelling in functions, and something more british in comments 13:20 "nondigital" 13:20 hehe 13:20 alright gotcha, thanks! On Thu, 2017-07-06 at 16:26 +0300, Paul Kocialkowski wrote: > I am only sending this for comments at this point, since this will > need > to be reworked according to the changes made on previous series > still > under review. Especially, it will benefit from making the frame save > mechanism common. > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Consistently use enum pipe for PCH transcoders
The current code uses in some instances enum transcoder for PCH transcoders and enum pipe in others. This is error prone and clang raises warnings like this: drivers/gpu/drm/i915/intel_dp.c:3546:51: warning: implicit conversion from enumeration type 'enum pipe' to different enumeration type 'enum transcoder' [-Wenum-conversion] intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false); Consistently use the type enum pipe for PCH transcoders. Signed-off-by: Matthias Kaehlcke --- Changes in v2: - rebased on drm-intel/drm-intel-next-queued drivers/gpu/drm/i915/i915_irq.c| 10 +- drivers/gpu/drm/i915/intel_display.c | 24 ++-- drivers/gpu/drm/i915/intel_drv.h | 6 +++--- drivers/gpu/drm/i915/intel_fifo_underrun.c | 6 +++--- 4 files changed, 21 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 1d33cea01a1b..0b6f310101ee 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2086,10 +2086,10 @@ static void ibx_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir) DRM_DEBUG_DRIVER("PCH transcoder CRC error interrupt\n"); if (pch_iir & SDE_TRANSA_FIFO_UNDER) - intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A); + intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A); if (pch_iir & SDE_TRANSB_FIFO_UNDER) - intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B); + intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B); } static void ivb_err_int_handler(struct drm_i915_private *dev_priv) @@ -2123,13 +2123,13 @@ static void cpt_serr_int_handler(struct drm_i915_private *dev_priv) DRM_ERROR("PCH poison interrupt\n"); if (serr_int & SERR_INT_TRANS_A_FIFO_UNDERRUN) - intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_A); + intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_A); if (serr_int & SERR_INT_TRANS_B_FIFO_UNDERRUN) - intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_B); + intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_B); if (serr_int & SERR_INT_TRANS_C_FIFO_UNDERRUN) - intel_pch_fifo_underrun_irq_handler(dev_priv, TRANSCODER_C); + intel_pch_fifo_underrun_irq_handler(dev_priv, PIPE_C); I915_WRITE(SERR_INT, serr_int); } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index bb9c9c3c391f..5c7054c3be0e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1777,7 +1777,7 @@ static void lpt_enable_pch_transcoder(struct drm_i915_private *dev_priv, /* FDI must be feeding us bits for PCH ports */ assert_fdi_tx_enabled(dev_priv, (enum pipe) cpu_transcoder); - assert_fdi_rx_enabled(dev_priv, TRANSCODER_A); + assert_fdi_rx_enabled(dev_priv, PIPE_A); /* Workaround: set timing override bit. */ val = I915_READ(TRANS_CHICKEN2(PIPE_A)); @@ -1853,16 +1853,16 @@ void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv) I915_WRITE(TRANS_CHICKEN2(PIPE_A), val); } -enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc) +enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); WARN_ON(!crtc->config->has_pch_encoder); if (HAS_PCH_LPT(dev_priv)) - return TRANSCODER_A; + return PIPE_A; else - return (enum transcoder) crtc->pipe; + return crtc->pipe; } /** @@ -1901,7 +1901,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc) if (crtc->config->has_pch_encoder) { /* if driving the PCH, we need FDI enabled */ assert_fdi_rx_pll_enabled(dev_priv, - (enum pipe) intel_crtc_pch_transcoder(crtc)); + intel_crtc_pch_transcoder(crtc)); assert_fdi_tx_pll_enabled(dev_priv, (enum pipe) cpu_transcoder); } @@ -4579,7 +4579,7 @@ static void lpt_pch_enable(const struct intel_crtc_state *crtc_state) struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; - assert_pch_transcoder_disabled(dev_priv, TRANSCODER_A); + assert_pch_transcoder_disabled(dev_priv, PIPE_A); lpt_program_iclkip(crtc); @ -5347,8 +5347,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, return; if (intel_crtc->config->has_pch_encoder) - intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A, -
[Intel-gfx] [PATCH] drm/i915: Return correct EDP voltage swing table for 0.85V
For 0.85V cnl_get_buf_trans_edp() returns the DP table, instead of EDP. Use the correct table. The error was pointed out by this clang warning: drivers/gpu/drm/i915/intel_ddi.c:392:39: warning: variable 'cnl_ddi_translations_edp_0_85V' is not needed and will not be emitted [-Wunneeded-internal-declaration] static const struct cnl_ddi_buf_trans cnl_ddi_translations_edp_0_85V[] = { Signed-off-by: Matthias Kaehlcke --- drivers/gpu/drm/i915/intel_ddi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index efb13582dc73..6fa02e6a7a9b 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1873,7 +1873,7 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, if (dev_priv->vbt.edp.low_vswing) { if (voltage == VOLTAGE_INFO_0_85V) { *n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_85V); - return cnl_ddi_translations_dp_0_85V; + return cnl_ddi_translations_edp_0_85V; } else if (voltage == VOLTAGE_INFO_0_95V) { *n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_95V); return cnl_ddi_translations_edp_0_95V; -- 2.13.2.932.g7449e964c-goog ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Return correct EDP voltage swing table for 0.85V
Looks like a typo in cf54ca8 ("drm/i915/cnl: Implement voltage swing sequence.") but Cc'ing Rodrigo, Clint to make sure this wasn't a workaround. -DK On Mon, 2017-07-17 at 11:21 -0700, Matthias Kaehlcke wrote: > For 0.85V cnl_get_buf_trans_edp() returns the DP table, instead of EDP. > Use the correct table. > > The error was pointed out by this clang warning: > > drivers/gpu/drm/i915/intel_ddi.c:392:39: warning: variable > 'cnl_ddi_translations_edp_0_85V' is not needed and will not be emitted > [-Wunneeded-internal-declaration] > static const struct cnl_ddi_buf_trans cnl_ddi_translations_edp_0_85V[] = { > > Signed-off-by: Matthias Kaehlcke > --- > drivers/gpu/drm/i915/intel_ddi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index efb13582dc73..6fa02e6a7a9b 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1873,7 +1873,7 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, > if (dev_priv->vbt.edp.low_vswing) { > if (voltage == VOLTAGE_INFO_0_85V) { > *n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_85V); > - return cnl_ddi_translations_dp_0_85V; > + return cnl_ddi_translations_edp_0_85V; > } else if (voltage == VOLTAGE_INFO_0_95V) { > *n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_95V); > return cnl_ddi_translations_edp_0_95V; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Return correct EDP voltage swing table for 0.85V
This makes sense, it should have returned the edp ddi buf translation table as per the Bspec. Please add this to the commit message: Fixes: cf54ca8bc567 ("drm/i915/cnl: Implement voltage swing sequence.") After that, Reviewed-by: Manasi Navare Manasi On Mon, Jul 17, 2017 at 11:21:27AM -0700, Matthias Kaehlcke wrote: > For 0.85V cnl_get_buf_trans_edp() returns the DP table, instead of EDP. > Use the correct table. > > The error was pointed out by this clang warning: > > drivers/gpu/drm/i915/intel_ddi.c:392:39: warning: variable > 'cnl_ddi_translations_edp_0_85V' is not needed and will not be emitted > [-Wunneeded-internal-declaration] > static const struct cnl_ddi_buf_trans cnl_ddi_translations_edp_0_85V[] = { > > Signed-off-by: Matthias Kaehlcke > --- > drivers/gpu/drm/i915/intel_ddi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index efb13582dc73..6fa02e6a7a9b 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1873,7 +1873,7 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, > if (dev_priv->vbt.edp.low_vswing) { > if (voltage == VOLTAGE_INFO_0_85V) { > *n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_85V); > - return cnl_ddi_translations_dp_0_85V; > + return cnl_ddi_translations_edp_0_85V; > } else if (voltage == VOLTAGE_INFO_0_95V) { > *n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_95V); > return cnl_ddi_translations_edp_0_95V; > -- > 2.13.2.932.g7449e964c-goog > > ___ > 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
[Intel-gfx] Skylake / (EE) modeset(0): present flip failed loop
Ok, there must be a problem, sent 5 messages to the list with clear details on how the intel driver is failing for me, got one answer from Chris which sadly was a bit too short to give me a good hint of what I should try next or what further debugging I should give to help fix/improve the driver, and then nothing. I'm not sure if the messages are being dropped or what's going on, so I've added a bunch of intel folks who have posted to the list. I realize it's a bit rude, and I apologize for this, but since I've gotten no acknowledgement that my multiple messages are even being seen, that's my only recourse left. Again, I was sent to this list to report issues with the xorg intel driver. If this is not the correct place to report issues, please tell me where I should do so. As for the issue, from Chris' answer I think he said I shouldn't be using the KMS intel driver, except it seems to be the only option in debian. I'm not super sure what he meant or what I'm supposed to do next. Details below: Thanks for your help, the current driver setup is working badly and is filling my disk with log spam which gets reset when it finally crashes every few days: saruman:/tmp$ du -sh /var/log/Xorg.0.log 1.1G/var/log/Xorg.0.log On Fri, Jul 14, 2017 at 01:50:55PM -0700, Marc MERLIN wrote: > I'll try a more basic question: am I supposed not to use the modesetting > driver if I want a working setup? > While debian seems to default to or kind of force the use of the modesetting > driver, if you confirm I shouldn't be using it, I can look at how to switch > away from it. > > > > > On Wed, Jul 05, 2017 at 11:33:01PM -0700, Marc MERLIN wrote: > > > > > Howdy, > > > > > > > > > > I have a thinkpad P70 with debian testing and 4.11.6 kernel. > > > > > A recent-ish upgrade broke something and now I'm getting loads of spam > > > > > in my Xorg.log > > > > > > > > > > [ 5031.435] (WW) modeset(0): flip queue failed: Invalid argument > > > > > [ 5031.435] (WW) modeset(0): Page flip failed: Invalid argument > > > > > [ 5031.435] (EE) modeset(0): present flip failed > > > > > [ 5031.519] (WW) modeset(0): flip queue failed: Invalid argument > > > > > [ 5031.519] (WW) modeset(0): Page flip failed: Invalid argument > > > > > [ 5031.519] (EE) modeset(0): present flip failed > > > > > (...) > > > > > > > > > > system info: > > > > > ii libdrm-intel1:amd642.4.74-1 > > > > > ii xserver-xorg-core 2:1.19.2-1 > > > > > ii xserver-xorg-video-intel 2:2.99.917+git20161206-1 > > > > > > If you were indeed using -intel then I would be more concerned. > > > > Thanks for the reply. > > Sorry, I'm not quite parsing what you wrote here. Are you saying that I > > should be disable the modesetting driver? > > To be honest, I didn't actually choose it, it seems that Debian forced > > the switch to it. > > > > xserver-xorg-video-intel (2:2.13.0-2) unstable; urgency=low > > > > * Starting from 2.10, the Intel X driver depends on a kernel driver for > > mode setting (that's called KMS). The corresponding kernel option is > > CONFIG_DRM_I915, and is enabled in Debian kernels. > > * To enable KMS, either of those should be sufficient: > > + /etc/modprobe.d/i915-kms.conf should contain: > > options i915 modeset=1 > > > > If so, how do you recommend I switch back if that's what you meant I should > > do? > > > > > But at the very least you need to dig into dmesg (with drm.debug=fe) to > > > find out why it failed. (One way is to run -intel with debugging enabled > > > so that it includes the kernel error messages along with the failure > > > message.) > > > > Sounds like I need to switch drivers? > > Right now I have no xorg.conf and it just autodetects/sets the KMS driver. > > Sorry if I'm kind of a NOOB here, but if you give me a short pointer to > > how you'd like me to switch, I'll happily do so. Original message: > Howdy, > > I have a thinkpad P70 with debian testing and 4.11.6 kernel. > A recent-ish upgrade broke something and now I'm getting loads of spam > in my Xorg.log > > [ 5031.435] (WW) modeset(0): flip queue failed: Invalid argument > [ 5031.435] (WW) modeset(0): Page flip failed: Invalid argument > [ 5031.435] (EE) modeset(0): present flip failed > [ 5031.519] (WW) modeset(0): flip queue failed: Invalid argument > [ 5031.519] (WW) modeset(0): Page flip failed: Invalid argument > [ 5031.519] (EE) modeset(0): present flip failed > (...) > > system info: > ii libdrm-intel1:amd642.4.74-1 > ii xserver-xorg-core 2:1.19.2-1 > ii xserver-xorg-video-intel 2:2.99.917+git20161206-1 > > 4.11.6-amd64-preempt > > saruman:~$ xrandr --listproviders > Providers: number : 2 > Provider 0: id: 0x7a cap: 0xf, Source Output, Sink Output, Source Offload, > Sink Offload crtcs: 3 outputs: 1 associated providers: 0 name:modesetting > Provider 1: id: 0x46 cap: 0xf, Source Output, Sink Output, Source Offload, >
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Return correct EDP voltage swing table for 0.85V
== Series Details == Series: drm/i915: Return correct EDP voltage swing table for 0.85V URL : https://patchwork.freedesktop.org/series/27426/ State : success == Summary == Series 27426v1 drm/i915: Return correct EDP voltage swing table for 0.85V https://patchwork.freedesktop.org/api/1.0/series/27426/revisions/1/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-devices: pass -> DMESG-WARN (fi-kbl-7560u) k.org#196399 Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: fail -> PASS (fi-snb-2600) fdo#100215 Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-a: pass -> DMESG-WARN (fi-pnv-d510) fdo#101597 +1 Subgroup suspend-read-crc-pipe-b: pass -> DMESG-WARN (fi-byt-j1900) fdo#101705 +1 fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 k.org#196399 https://bugzilla.kernel.org/show_bug.cgi?id=196399 fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597 fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:440s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:434s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:358s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:533s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:507s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:495s fi-byt-n2820 total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:479s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:593s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:434s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:418s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:419s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:496s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:472s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:462s fi-kbl-7560u total:279 pass:268 dwarn:1 dfail:0 fail:0 skip:10 time:569s fi-kbl-r total:279 pass:260 dwarn:1 dfail:0 fail:0 skip:18 time:581s fi-pnv-d510 total:279 pass:221 dwarn:3 dfail:0 fail:0 skip:55 time:563s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:458s fi-skl-6700hqtotal:279 pass:262 dwarn:0 dfail:0 fail:0 skip:17 time:590s fi-skl-6700k total:279 pass:257 dwarn:4 dfail:0 fail:0 skip:18 time:464s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:479s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:436s fi-skl-x1585ltotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:487s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:549s fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:409s 3010bfcdfd837d2f7708fc37f1b8de79e8e9362d drm-tip: 2017y-07m-17d-18h-46m-40s UTC integration manifest 142b2b9 drm/i915: Return correct EDP voltage swing table for 0.85V == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5213/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Return correct EDP voltage swing table for 0.85V
For 0.85V cnl_get_buf_trans_edp() returns the DP table, instead of EDP. Use the correct table. The error was pointed out by this clang warning: drivers/gpu/drm/i915/intel_ddi.c:392:39: warning: variable 'cnl_ddi_translations_edp_0_85V' is not needed and will not be emitted [-Wunneeded-internal-declaration] static const struct cnl_ddi_buf_trans cnl_ddi_translations_edp_0_85V[] = { Fixes: cf54ca8bc567 ("drm/i915/cnl: Implement voltage swing sequence.") Signed-off-by: Matthias Kaehlcke Reviewed-by: Manasi Navare --- Changes in v2: - Added 'Fixes' tag - Added Reviewed-by: Manasi Navare drivers/gpu/drm/i915/intel_ddi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index efb13582dc73..6fa02e6a7a9b 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1873,7 +1873,7 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, if (dev_priv->vbt.edp.low_vswing) { if (voltage == VOLTAGE_INFO_0_85V) { *n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_85V); - return cnl_ddi_translations_dp_0_85V; + return cnl_ddi_translations_edp_0_85V; } else if (voltage == VOLTAGE_INFO_0_95V) { *n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_95V); return cnl_ddi_translations_edp_0_95V; -- 2.13.2.932.g7449e964c-goog ___ 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: Return correct EDP voltage swing table for 0.85V (rev2)
== Series Details == Series: drm/i915: Return correct EDP voltage swing table for 0.85V (rev2) URL : https://patchwork.freedesktop.org/series/27426/ State : success == Summary == Series 27426v2 drm/i915: Return correct EDP voltage swing table for 0.85V https://patchwork.freedesktop.org/api/1.0/series/27426/revisions/2/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-devices: pass -> DMESG-WARN (fi-kbl-7560u) k.org#196399 +1 Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: fail -> PASS (fi-snb-2600) fdo#100215 Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-a: pass -> DMESG-WARN (fi-pnv-d510) fdo#101597 Subgroup suspend-read-crc-pipe-b: pass -> DMESG-WARN (fi-byt-j1900) fdo#101705 fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 k.org#196399 https://bugzilla.kernel.org/show_bug.cgi?id=196399 fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597 fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:446s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:429s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:358s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:531s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:509s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:491s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:481s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:591s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:435s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:419s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:424s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:498s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:478s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:461s fi-kbl-7560u total:279 pass:268 dwarn:1 dfail:0 fail:0 skip:10 time:577s fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:580s fi-pnv-d510 total:279 pass:222 dwarn:2 dfail:0 fail:0 skip:55 time:557s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:451s fi-skl-6700hqtotal:279 pass:262 dwarn:0 dfail:0 fail:0 skip:17 time:587s fi-skl-6700k total:279 pass:257 dwarn:4 dfail:0 fail:0 skip:18 time:467s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:478s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:434s fi-skl-x1585ltotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:488s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:542s fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:409s 3010bfcdfd837d2f7708fc37f1b8de79e8e9362d drm-tip: 2017y-07m-17d-18h-46m-40s UTC integration manifest 2301361 drm/i915: Return correct EDP voltage swing table for 0.85V == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5214/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Skylake / (EE) modeset(0): present flip failed loop
Marc, please file a bug on freedesktop.org. We expect the modesetting driver to work well and if it's not, it should have a bug associated with it. Sorry for your frustration. On 17-07-17 12:22:00, Marc MERLIN wrote: Ok, there must be a problem, sent 5 messages to the list with clear details on how the intel driver is failing for me, got one answer from Chris which sadly was a bit too short to give me a good hint of what I should try next or what further debugging I should give to help fix/improve the driver, and then nothing. I'm not sure if the messages are being dropped or what's going on, so I've added a bunch of intel folks who have posted to the list. I realize it's a bit rude, and I apologize for this, but since I've gotten no acknowledgement that my multiple messages are even being seen, that's my only recourse left. Again, I was sent to this list to report issues with the xorg intel driver. If this is not the correct place to report issues, please tell me where I should do so. As for the issue, from Chris' answer I think he said I shouldn't be using the KMS intel driver, except it seems to be the only option in debian. I'm not super sure what he meant or what I'm supposed to do next. Details below: Thanks for your help, the current driver setup is working badly and is filling my disk with log spam which gets reset when it finally crashes every few days: saruman:/tmp$ du -sh /var/log/Xorg.0.log 1.1G/var/log/Xorg.0.log On Fri, Jul 14, 2017 at 01:50:55PM -0700, Marc MERLIN wrote: I'll try a more basic question: am I supposed not to use the modesetting driver if I want a working setup? While debian seems to default to or kind of force the use of the modesetting driver, if you confirm I shouldn't be using it, I can look at how to switch away from it. > > > On Wed, Jul 05, 2017 at 11:33:01PM -0700, Marc MERLIN wrote: > > > > Howdy, > > > > > > > > I have a thinkpad P70 with debian testing and 4.11.6 kernel. > > > > A recent-ish upgrade broke something and now I'm getting loads of spam > > > > in my Xorg.log > > > > > > > > [ 5031.435] (WW) modeset(0): flip queue failed: Invalid argument > > > > [ 5031.435] (WW) modeset(0): Page flip failed: Invalid argument > > > > [ 5031.435] (EE) modeset(0): present flip failed > > > > [ 5031.519] (WW) modeset(0): flip queue failed: Invalid argument > > > > [ 5031.519] (WW) modeset(0): Page flip failed: Invalid argument > > > > [ 5031.519] (EE) modeset(0): present flip failed > > > > (...) > > > > > > > > system info: > > > > ii libdrm-intel1:amd642.4.74-1 > > > > ii xserver-xorg-core 2:1.19.2-1 > > > > ii xserver-xorg-video-intel 2:2.99.917+git20161206-1 > > > > If you were indeed using -intel then I would be more concerned. > > Thanks for the reply. > Sorry, I'm not quite parsing what you wrote here. Are you saying that I > should be disable the modesetting driver? > To be honest, I didn't actually choose it, it seems that Debian forced > the switch to it. > > xserver-xorg-video-intel (2:2.13.0-2) unstable; urgency=low > > * Starting from 2.10, the Intel X driver depends on a kernel driver for > mode setting (that's called KMS). The corresponding kernel option is > CONFIG_DRM_I915, and is enabled in Debian kernels. > * To enable KMS, either of those should be sufficient: > + /etc/modprobe.d/i915-kms.conf should contain: > options i915 modeset=1 > > If so, how do you recommend I switch back if that's what you meant I should do? > > > But at the very least you need to dig into dmesg (with drm.debug=fe) to > > find out why it failed. (One way is to run -intel with debugging enabled > > so that it includes the kernel error messages along with the failure > > message.) > > Sounds like I need to switch drivers? > Right now I have no xorg.conf and it just autodetects/sets the KMS driver. > Sorry if I'm kind of a NOOB here, but if you give me a short pointer to > how you'd like me to switch, I'll happily do so. Original message: Howdy, I have a thinkpad P70 with debian testing and 4.11.6 kernel. A recent-ish upgrade broke something and now I'm getting loads of spam in my Xorg.log [ 5031.435] (WW) modeset(0): flip queue failed: Invalid argument [ 5031.435] (WW) modeset(0): Page flip failed: Invalid argument [ 5031.435] (EE) modeset(0): present flip failed [ 5031.519] (WW) modeset(0): flip queue failed: Invalid argument [ 5031.519] (WW) modeset(0): Page flip failed: Invalid argument [ 5031.519] (EE) modeset(0): present flip failed (...) system info: ii libdrm-intel1:amd642.4.74-1 ii xserver-xorg-core 2:1.19.2-1 ii xserver-xorg-video-intel 2:2.99.917+git20161206-1 4.11.6-amd64-preempt saruman:~$ xrandr --listproviders Providers: number : 2 Provider 0: id: 0x7a cap: 0xf, Source Output, Sink Output, Source Offload, Sink Offload crtcs: 3 outputs: 1 associated providers: 0 name:modesetting Provider 1: id: 0x46 cap: 0xf, Source Output, Sink Output, Source Offload, Sink Offlo
Re: [Intel-gfx] [PATCH] drm/i915: Return correct EDP voltage swing table for 0.85V
On Mon, 2017-07-17 at 18:55 +, Pandiyan, Dhinakaran wrote: > Looks like a typo in > > cf54ca8 ("drm/i915/cnl: Implement voltage swing sequence.") > > but Cc'ing Rodrigo, Clint to make sure this wasn't a workaround. > > -DK Checked with Clint, this wasn't a workaround, a typo indeed. > On Mon, 2017-07-17 at 11:21 -0700, Matthias Kaehlcke wrote: > > For 0.85V cnl_get_buf_trans_edp() returns the DP table, instead of EDP. > > Use the correct table. > > > > The error was pointed out by this clang warning: > > > > drivers/gpu/drm/i915/intel_ddi.c:392:39: warning: variable > > 'cnl_ddi_translations_edp_0_85V' is not needed and will not be emitted > > [-Wunneeded-internal-declaration] > > static const struct cnl_ddi_buf_trans cnl_ddi_translations_edp_0_85V[] > > = { > > > > Signed-off-by: Matthias Kaehlcke > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > b/drivers/gpu/drm/i915/intel_ddi.c > > index efb13582dc73..6fa02e6a7a9b 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1873,7 +1873,7 @@ cnl_get_buf_trans_edp(struct drm_i915_private > > *dev_priv, > > if (dev_priv->vbt.edp.low_vswing) { > > if (voltage == VOLTAGE_INFO_0_85V) { > > *n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_85V); > > - return cnl_ddi_translations_dp_0_85V; > > + return cnl_ddi_translations_edp_0_85V; > > } else if (voltage == VOLTAGE_INFO_0_95V) { > > *n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_95V); > > return cnl_ddi_translations_edp_0_95V; > ___ > 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: Return correct EDP voltage swing table for 0.85V
On Mon, Jul 17, 2017 at 08:59:18PM +, Pandiyan, Dhinakaran wrote: > > > > On Mon, 2017-07-17 at 18:55 +, Pandiyan, Dhinakaran wrote: > > Looks like a typo in > > > > cf54ca8 ("drm/i915/cnl: Implement voltage swing sequence.") > > > > but Cc'ing Rodrigo, Clint to make sure this wasn't a workaround. > > > > -DK > > Checked with Clint, this wasn't a workaround, a typo indeed. > > Yup, Like I mentioned in the previous reply, I am positive that this should return cnl_ddi_translations_edp_0_85 since I had discussed this with Rodrigo while working on ddi buffer initialization. Matthias, please go ahead and add the Fixes Tag like I mentioned in my previous message. Thanks for catching this. Manasi > > > On Mon, 2017-07-17 at 11:21 -0700, Matthias Kaehlcke wrote: > > > For 0.85V cnl_get_buf_trans_edp() returns the DP table, instead of EDP. > > > Use the correct table. > > > > > > The error was pointed out by this clang warning: > > > > > > drivers/gpu/drm/i915/intel_ddi.c:392:39: warning: variable > > > 'cnl_ddi_translations_edp_0_85V' is not needed and will not be emitted > > > [-Wunneeded-internal-declaration] > > > static const struct cnl_ddi_buf_trans > > > cnl_ddi_translations_edp_0_85V[] = { > > > > > > Signed-off-by: Matthias Kaehlcke > > > --- > > > drivers/gpu/drm/i915/intel_ddi.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > > b/drivers/gpu/drm/i915/intel_ddi.c > > > index efb13582dc73..6fa02e6a7a9b 100644 > > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > > @@ -1873,7 +1873,7 @@ cnl_get_buf_trans_edp(struct drm_i915_private > > > *dev_priv, > > > if (dev_priv->vbt.edp.low_vswing) { > > > if (voltage == VOLTAGE_INFO_0_85V) { > > > *n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_85V); > > > - return cnl_ddi_translations_dp_0_85V; > > > + return cnl_ddi_translations_edp_0_85V; > > > } else if (voltage == VOLTAGE_INFO_0_95V) { > > > *n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_0_95V); > > > return cnl_ddi_translations_edp_0_95V; > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Skylake / (EE) modeset(0): present flip failed loop
On Mon, Jul 17, 2017 at 01:55:36PM -0700, Ben Widawsky wrote: > Marc, please file a bug on freedesktop.org. > > We expect the modesetting driver to work well and if it's not, it should > have a bug associated with it. Thanks, done: https://bugs.freedesktop.org/show_bug.cgi?id=101825 Hopefully I found the right queue for it, if not feel free to re-assign. Thanks, Marc -- "A mouse is a device used to point at the xterm you want to type in" - A.S.R. Microsoft is to operating systems what McDonalds is to gourmet cooking Home page: http://marc.merlins.org/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/15] drm/i915: Serialize per-engine resets against new requests
On 17/07/17 02:11, Chris Wilson wrote: We rely on disabling the execlists (by stopping the tasklet) to prevent new requests from submitting to the engine ELSP before we are ready. However, we re-enable the engine before we call init_hw which gives userspace the opportunity to subit a new request which is then overwritten by init_hw -- but not before the HW may have started executing. The subsequent out-of-order CSB is detected by our sanity checks in intel_lrc_irq_handler(). Fixes: a1ef70e14453 ("drm/i915: Add support for per engine reset recovery") Signed-off-by: Chris Wilson Cc: Michel Thierry Cc: Mika Kuoppala --- drivers/gpu/drm/i915/i915_drv.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 901c3ff61527..bc121a46ed9a 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1955,6 +1955,12 @@ int i915_reset_engine(struct intel_engine_cs *engine) } ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine)); + if (ret) { + /* If we fail here, we expect to fallback to a global reset */ + DRM_DEBUG_DRIVER("Failed to reset %s, ret=%d\n", +engine->name, ret); + goto out; + } /* * The request that caused the hang is stuck on elsp, we know the @@ -1963,15 +1969,6 @@ int i915_reset_engine(struct intel_engine_cs *engine) */ i915_gem_reset_engine(engine, active_request); - i915_gem_reset_finish_engine(engine); - - if (ret) { - /* If we fail here, we expect to fallback to a global reset */ - DRM_DEBUG_DRIVER("Failed to reset %s, ret=%d\n", -engine->name, ret); - goto out; - } - /* * The engine and its registers (and workarounds in case of render) * have been reset to their default values. Follow the init_ring @@ -1983,6 +1980,7 @@ int i915_reset_engine(struct intel_engine_cs *engine) error->reset_engine_count[engine->id]++; out: + i915_gem_reset_finish_engine(engine); return ret; } Reviewed-by: Michel Thierry ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/cnl: Fix loadgen select programming on ddi vswing sequence
The condition for setting the Loadgen Select bit of PORT_TX_DW4 register during DDI Vswing Sequence should be Bit rate <=6 GHz whereas the existing code checks only Bit Rate < 6GHz. This patch fixes this condition. While at it also remove the redundant paranthesis. Fixes: cf54ca8bc567 ("drm/i915/cnl: Implement voltage swing sequence.") Cc: Paulo Zanoni Cc: Rodrigo Vivi Signed-off-by: Manasi Navare --- drivers/gpu/drm/i915/intel_ddi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index efb1358..f4fbb39 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -2010,8 +2010,8 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level) val = I915_READ(CNL_PORT_TX_DW4_LN(port, ln)); val &= ~LOADGEN_SELECT; - if (((rate < 60) && (width == 4) && (ln >= 1)) || - ((rate < 60) && (width < 4) && ((ln == 1) || (ln == 2 { + if ((rate <= 60 && width == 4 && ln >= 1) || + (rate <= 60 && width < 4 && (ln == 1 || ln == 2))) { val |= LOADGEN_SELECT; } I915_WRITE(CNL_PORT_TX_DW4_LN(port, ln), val); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/15] drm/i915: Clear engine irq posted following a reset
On 17/07/17 02:11, Chris Wilson wrote: When the GPU is reset, we want to discard all pending notifications as either we have manually completed them, or they are no longer applicable. Make sure we do reset the engine->irq_posted prior to re-enabling the engine (e.g. the interrupt tasklets) in i915_gem_reset_finish_engine(). Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 5517373c1bea..19511020f06e 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3027,6 +3027,8 @@ static bool i915_gem_reset_request(struct drm_i915_gem_request *request) void i915_gem_reset_engine(struct intel_engine_cs *engine, struct drm_i915_gem_request *request) { + engine->irq_posted = 0; + if (request && i915_gem_reset_request(request)) { DRM_DEBUG_DRIVER("resetting %s to restart from tail of request 0x%x\n", engine->name, request->global_seqno); Reviewed-by: Michel Thierry ___ 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/cnl: Fix loadgen select programming on ddi vswing sequence
== Series Details == Series: drm/i915/cnl: Fix loadgen select programming on ddi vswing sequence URL : https://patchwork.freedesktop.org/series/27446/ State : success == Summary == Series 27446v1 drm/i915/cnl: Fix loadgen select programming on ddi vswing sequence https://patchwork.freedesktop.org/api/1.0/series/27446/revisions/1/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-devices: pass -> DMESG-WARN (fi-kbl-7560u) k.org#196399 Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: fail -> PASS (fi-snb-2600) fdo#100215 Test kms_flip: Subgroup basic-flip-vs-modeset: pass -> SKIP (fi-skl-x1585l) fdo#101781 Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-b: pass -> DMESG-WARN (fi-byt-j1900) fdo#101705 fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 k.org#196399 https://bugzilla.kernel.org/show_bug.cgi?id=196399 fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781 fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:442s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:431s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:526s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:508s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:491s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:488s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:593s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:435s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:413s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:418s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:496s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:468s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:469s fi-kbl-7560u total:279 pass:268 dwarn:1 dfail:0 fail:0 skip:10 time:571s fi-kbl-r total:279 pass:260 dwarn:1 dfail:0 fail:0 skip:18 time:584s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:562s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:459s fi-skl-6700hqtotal:279 pass:262 dwarn:0 dfail:0 fail:0 skip:17 time:583s fi-skl-6700k total:279 pass:257 dwarn:4 dfail:0 fail:0 skip:18 time:469s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:475s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:435s fi-skl-x1585ltotal:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:485s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:547s fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:407s 3010bfcdfd837d2f7708fc37f1b8de79e8e9362d drm-tip: 2017y-07m-17d-18h-46m-40s UTC integration manifest ea21419 drm/i915/cnl: Fix loadgen select programming on ddi vswing sequence == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5215/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/15] drm/i915: Check execlist/ring status during hangcheck
On 17/07/17 02:11, Chris Wilson wrote: Before we declare an engine as idle, check if there are any pending execlist context-switches and if the ring itself reports as idle. Otherwise, we may be left in a situation where we miss a crucial execlist event (or something more sinister) yet the requests complete. Since the seqno write happens, we believe the engine to be truly idle. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_hangcheck.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_hangcheck.c b/drivers/gpu/drm/i915/intel_hangcheck.c index 9b0ece427bdc..d9d87d96fb69 100644 --- a/drivers/gpu/drm/i915/intel_hangcheck.c +++ b/drivers/gpu/drm/i915/intel_hangcheck.c @@ -324,7 +324,7 @@ hangcheck_get_action(struct intel_engine_cs *engine, if (engine->hangcheck.seqno != hc->seqno) return ENGINE_ACTIVE_SEQNO; - if (i915_seqno_passed(hc->seqno, intel_engine_last_submit(engine))) + if (intel_engine_is_idle(engine)) return ENGINE_IDLE; return engine_stuck(engine, hc->acthd); Reviewed-by: Michel Thierry ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 14/15] drm/i915: Disable per-engine reset for Broxton
On 17/07/17 02:11, Chris Wilson wrote: Triggering a GPU reset for one engine affects another, notably corrupting the context status buffer (CSB) effectively losing track of inflight requests. Adding a few printks: diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index ad41836fa5e5..a969456bc0fa 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1953,6 +1953,7 @@ int i915_reset_engine(struct intel_engine_cs *engine) goto out; } + pr_err("Resetting %s\n", engine->name); ret = intel_gpu_reset(engine->i915, intel_engine_flag(engine)); if (ret) { /* If we fail here, we expect to fallback to a global reset */ diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 716e5c9ea222..a72bc35d0870 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -355,6 +355,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN); port_set(&port[n], port_pack(rq, count)); desc = execlists_update_context(rq); + pr_err("%s: in (rq=%x) ctx=%d\n", engine->name, rq->global_seqno, upper_32_bits(desc)); GEM_DEBUG_EXEC(port[n].context_id = upper_32_bits(desc)); } else { GEM_BUG_ON(!n); @@ -594,9 +595,23 @@ static void intel_lrc_irq_handler(unsigned long data) if (!(status & GEN8_CTX_STATUS_COMPLETED_MASK)) continue; + pr_err("%s: out CSB (%x head=%d, tail=%d), ctx=%d, rq=%d\n", + engine->name, + readl(csb_mmio), + head, tail, + readl(buf+2*head+1), + port->context_id); + /* Check the context/desc id for this event matches */ - GEM_DEBUG_BUG_ON(readl(buf + 2 * head + 1) != -port->context_id); + if (readl(buf + 2 * head + 1) != port->context_id) { + pr_err("%s: BUG CSB (%x head=%d, tail=%d), ctx=%d, rq=%d\n", + engine->name, + readl(csb_mmio), + head, tail, + readl(buf+2*head+1), + port->context_id); + BUG(); + } rq = port_unpack(port, &count); GEM_BUG_ON(count == 0); Results in: [ 6423.006602] Resetting rcs0 [ 6423.009080] rcs0: in (rq=fe70) ctx=1 [ 6423.009216] rcs0: in (rq=fe6f) ctx=3 [ 6423.009542] rcs0: out CSB (2 head=1, tail=2), ctx=3, rq=3 [ 6423.009619] Resetting bcs0 [ 6423.009980] rcs0: BUG CSB (0 head=1, tail=2), ctx=0, rq=3 Note that this bug may be affect all machines and not just Broxton, Broxton is just the first machine on which I have confirmed this bug. Hopefully this is just broxton being broxton... I think I already sent this, but anyway... Acked-by: Michel Thierry Fixes: 142bc7d99bcf ("drm/i915: Modify error handler for per engine hang recovery") Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Michel Thierry --- drivers/gpu/drm/i915/i915_pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index a1e6b696bcfa..09d97e0990b7 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -398,6 +398,7 @@ static const struct intel_device_info intel_broxton_info = { GEN9_LP_FEATURES, .platform = INTEL_BROXTON, .ddb_size = 512, + .has_reset_engine = false, }; static const struct intel_device_info intel_geminilake_info = { ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/15] drm/i915: Assert that machine is wedged for nop_submit_request
On 17/07/17 02:11, Chris Wilson wrote: We should only ever do nop_submit_request when the machine is wedged, so assert it is so. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ca7a56ff3904..5517373c1bea 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3089,6 +3089,7 @@ void i915_gem_reset_finish(struct drm_i915_private *dev_priv) static void nop_submit_request(struct drm_i915_gem_request *request) { + GEM_BUG_ON(!i915_terminally_wedged(&request->i915->gpu_error)); dma_fence_set_error(&request->fence, -EIO); i915_gem_request_submit(request); intel_engine_init_global_seqno(request->engine, request->global_seqno); Not sure about this, your patch before this one, ("[PATCH 09/15] drm/i915: Wake up waiters after setting the WEDGED bit"), is moving the set_bit after engine_set_wedged: @@ -3157,10 +3157,12 @@ static int __i915_gem_set_wedged_BKL(void *data) struct intel_engine_cs *engine; enum intel_engine_id id; - set_bit(I915_WEDGED, &i915->gpu_error.flags); for_each_engine(engine, i915, id) engine_set_wedged(engine); + set_bit(I915_WEDGED, &i915->gpu_error.flags); + wake_up_all(&i915->gpu_error.reset_queue); + return 0; } I don't think it won't hit the BUG_ON because i915_gem_set_wedged is already protected by stop_machine. Anyway it looks odd. -Michel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/15] drm/i915: Emit a user level message when resetting the GPU (or engine)
On 17/07/17 02:11, Chris Wilson wrote: Although a banned context will be told to -EIO off if they try to submit more requests, we have a discrepancy between whole device resets and per-engine resets where we report the GPU reset but not the engine resets. This leaves a bit of mystery as to why the context was banned, and also reduces awareness overall of when a GPU (engine) reset occurs with its possible side-effects. Signed-off-by: Chris Wilson Cc: Michel Thierry Cc: Mika Kuoppala --- drivers/gpu/drm/i915/i915_drv.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index bc121a46ed9a..4b62fd012877 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1865,9 +1865,10 @@ void i915_reset(struct drm_i915_private *dev_priv) if (!i915_gem_unset_wedged(dev_priv)) goto wakeup; + dev_notice(dev_priv->drm.dev, + "Resetting chip after gpu hang\n"); error->reset_count++; - pr_notice("drm/i915: Resetting chip after gpu hang\n"); disable_irq(dev_priv->drm.irq); ret = i915_gem_reset_prepare(dev_priv); if (ret) { @@ -1945,7 +1946,9 @@ int i915_reset_engine(struct intel_engine_cs *engine) GEM_BUG_ON(!test_bit(I915_RESET_ENGINE + engine->id, &error->flags)); - DRM_DEBUG_DRIVER("resetting %s\n", engine->name); + dev_notice(engine->i915->drm.dev, + "Resetting %s after gpu hang\n", engine->name); + error->reset_engine_count[engine->id]++; This will increment both the engine-reset-count and gpu-reset count in the unlikely case that engine-reset gets promoted to full reset. Not a problem per-se, but I wanted to point it out (plus it makes both functions symmetric). active_request = i915_gem_reset_prepare_engine(engine); if (IS_ERR(active_request)) { @@ -1978,7 +1981,6 @@ int i915_reset_engine(struct intel_engine_cs *engine) if (ret) goto out; - error->reset_engine_count[engine->id]++; out: i915_gem_reset_finish_engine(engine); return ret; Reviewed-by: Michel Thierry ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] linux-next: build failure after merge of the drm-misc tree
Hi all, After merging the drm-misc tree, today's linux-next build (x86_64 allmodconfig) failed like this: Caused by commit b6dcaaac4474 ("drm/vblank: _ioctl posfix for ioctl handler") interacting with commit d5288c88c67c ("switch compat_drm_wait_vblank() to drm_ioctl_kernel()") from Linus' tree. You should consider rebasing your branch onto v4.13-rc1 (or merging v4.13-rc1) so that these (unnecessary) conflicts don't keep happening during development and the next merge window. I have applied the following merge fix patch for today. From: Stephen Rothwell Date: Tue, 18 Jul 2017 11:35:10 +1000 Subject: [PATCH] drm/vblank: fix for "switch compat_drm_wait_vblank() to drm_ioctl_kernel()" Signed-off-by: Stephen Rothwell --- drivers/gpu/drm/drm_ioc32.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c index d1f202852028..f8e96e648acf 100644 --- a/drivers/gpu/drm/drm_ioc32.c +++ b/drivers/gpu/drm/drm_ioc32.c @@ -842,7 +842,7 @@ static int compat_drm_wait_vblank(struct file *file, unsigned int cmd, req.request.type = req32.request.type; req.request.sequence = req32.request.sequence; req.request.signal = req32.request.signal; - err = drm_ioctl_kernel(file, drm_wait_vblank, &req, DRM_UNLOCKED); + err = drm_ioctl_kernel(file, drm_wait_vblank_ioctl, &req, DRM_UNLOCKED); if (err) return err; -- 2.13.2 -- Cheers, Stephen Rothwell ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx