All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: "Daniel, Thomas" <thomas.daniel@intel.com>
Cc: "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i915/bdw: Clean up execlist queue items in retire_work
Date: Fri, 24 Oct 2014 10:18:08 +0200	[thread overview]
Message-ID: <20141024081808.GU26941@phenom.ffwll.local> (raw)
In-Reply-To: <BFEE8FEC12424048AF1805991D65FA91196C613A@IRSMSX105.ger.corp.intel.com>

On Mon, Oct 20, 2014 at 01:29:59PM +0000, Daniel, Thomas wrote:
> > -----Original Message-----
> > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > Sent: Monday, October 20, 2014 2:11 PM
> > To: Daniel, Thomas
> > Cc: intel-gfx@lists.freedesktop.org; shuang.he@linux.intel.com
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/bdw: Clean up execlist queue
> > items in retire_work
> > 
> > On Mon, Oct 20, 2014 at 02:05:55PM +0100, Thomas Daniel wrote:
> > > No longer create a work item to clean each execlist queue item.
> > > Instead, move retired execlist requests to a queue and clean up the
> > > items during retire_requests.
> > >
> > > v2: Fix legacy ring path broken during overzealous cleanup
> > 
> > At some point I presume you noticed that you broke the idle detection...
> This patch doesn't change the idle detection.  Did you want the idle flag to
> now take into account the state of the execlists requests as well?

I guess what Chris means is that execlist broke idle detection (by
retiring in a separate work item, but not integrating with the idle
handling in the existing one). This patch gets us half-way there but still
doesn't really integrate things back fully.

We probably need (again) the s/execlist_item/request/ refactor to make
this happen for real. Since that's what Chris' patch does he featured the
link to it as an endpoint guideline I think.

Or am I completely missing the point again?

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

  reply	other threads:[~2014-10-24  8:18 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-20 12:51 [PATCH] drm/i915/bdw: Clean up execlist queue items in retire_work Thomas Daniel
2014-10-20 13:05 ` Thomas Daniel
2014-10-20 13:11   ` Chris Wilson
2014-10-20 13:29     ` Daniel, Thomas
2014-10-24  8:18       ` Daniel Vetter [this message]
2014-10-20 16:47   ` [PATCH] drm/i915/bdw: Clean up execlist queue items in shuang.he
2014-10-20 13:47 ` shuang.he
2014-10-20 14:20   ` He, Shuang
2014-10-21  8:32 ` shuang.he
2014-10-21 12:13   ` Daniel Vetter
2014-10-21 12:45     ` Daniel, Thomas
2014-10-22  1:00       ` He, Shuang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141024081808.GU26941@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=thomas.daniel@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.