All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Widawsky <bwidawsk@gmail.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 0/8] drm/i915/context
Date: Wed, 2 Feb 2011 15:55:31 -0800	[thread overview]
Message-ID: <20110202235531.GA5345@snipes.kumite> (raw)
In-Reply-To: <0d30dc$kupvbd@orsmga001.jf.intel.com>

On Wed, Feb 02, 2011 at 11:29:37PM +0000, Chris Wilson wrote:
> I don't think we should be pinning contexts for their entire lifetime.
> The page should only be pinned whilst it is being referenced by the
> hardware, and it looks like you already parts of the infrastructure to
> be able to track this. You will also need to track seqno and retire
> contexts...
> 

I have given thought to not pinning the contexts multiple times, and
believe it is ultimately what we will end up doing.  However, the
current implementation is definitely the easier, and more stable way to
go about it. So I'd definitely prefer to keep this as is until the other
parts of the stack are functional.

We need to consider how many clients we will actually have at one time.
I have no idea.  If we're talking a common case of 2 or 3 contexts, the
overhead may not be worth it. Retiring of contexts is very simple the
way it is now too because it only happens voluntarily.

> I'm not convinced by the locking strategy, there seems to be quite a few
> holes left, and is something else that is much easier to make it correct
> from the beginning and incrementally adjust.

Yes, I still need to review this, and I totally agree that it must be
right the first time. Is there something in particular that
bothers you about it?

> 
> And then there are lots of little changes to try and make the code
> tidier... But those are probably best left for another day.
> -Chris

Thanks for the feedback.
Ben

      reply	other threads:[~2011-02-02 23:55 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-02 23:00 [PATCH 0/8] drm/i915/context Ben Widawsky
2011-02-02 23:00 ` [PATCH 1/8] drm/i915/context: basic implementation context ioctls Ben Widawsky
2011-02-02 23:00 ` [PATCH 2/8] drm/i915/context: context initialization/destruction Ben Widawsky
2011-02-02 23:00 ` [PATCH 3/8] drm/i915/context: whitespace cleanup, and warning cleanup Ben Widawsky
2011-02-02 23:00 ` [PATCH 4/8] drm/i915/context: minimal support for contexts in execbuffer2 Ben Widawsky
2011-02-02 23:00 ` [PATCH 5/8] drm/i915/context: context validation for execbuffer2 Ben Widawsky
2011-02-14 20:21   ` Daniel Vetter
2011-02-14 23:10     ` Ben Widawsky
2011-02-02 23:00 ` [PATCH 6/8] drm/i915/context: enable calling context_switch Ben Widawsky
2011-02-02 23:00 ` [PATCH 7/8] drm/i915/context: Insert MI_SET_CONTEXT in ringbuffer context switch Ben Widawsky
2011-02-02 23:00 ` [PATCH 8/8] drm/i915/context: context switch, and PPGTT params Ben Widawsky
2011-02-02 23:29 ` [PATCH 0/8] drm/i915/context Chris Wilson
2011-02-02 23:55   ` Ben Widawsky [this message]

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=20110202235531.GA5345@snipes.kumite \
    --to=bwidawsk@gmail.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /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.