Re: [PATCH] drm/i915: intel hw has only one gpu write domain

Re: [PATCH] drm/i915: intel hw has only one gpu write domain

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 2398 bytes --]

On Wed, 17 Mar 2010 22:29:27 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Mar 17, 2010 at 02:00:09PM -0700, Eric Anholt wrote:
> > On Sun, 14 Mar 2010 12:18:15 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > It therefore holds that all objects on the gpu_write_domain are
> > > in this single write domain. Drop an if checking for the write
> > > domain in process_flushing_list.
> > > 
> > > For some odd reason retire_commands thinks that the sampler cache
> > > can be written to. Remove this and the assorted logic in do_execbuf.
> > > 
> > > Also check that userspace doesn't try to screw us over by claiming
> > > to write to some strange cache.
> > > 
> > > The idea for this patch emerged from a discussion with Chris Wilson.
> > 
> > The intent of the flush_domains in retire_commands was to note that the
> > read-only sampler cache was flushed and we could clear the read domains
> > so that a later attempt to use it in the sampler cache wouldn't send out
> > a new flush.
> > 
> > You've been looking into this stuff more than I have recently -- do you
> > think that handling that information would be interesting?
> 
> atm we don't handle invalidating read-domains as efficient as
> write-domains. Read: We don't globally update all bo's when doing a
> sampler cache invalidations like we do when flushing the render cache. So
> that flush_domain stuff from retire_commands was pure dead code.
> 
> We could do better of course. Problem is that tracking the buffers that
> might benefit from a read_domains with lists like we're doing for the
> write domain needs NUM_READ_DOMAIN list_heads per object. That's rather
> gross. But we could keep track of the last seqno this bo was dirtied
> (by either the gpu or the cpu, for the cpu just take the latest completed
> seqno) and also keep track of the seqno of the last invalidate for each
> read cache. At execbuf time we could then compare these two seqnos and
> decide whether a invalidate is still needed.
> 
> I haven't implemented this because
> - I don't have the hw that would benefit the most (i965).
> - On different hw, I don't think we'll gain much. At least as long as
>   multi-client drm usage is still uncommon.
> - Currently we still have easier targets for faster performance ;)

OK.  Seems like a fair assessment of the situation.  Applied to -next.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: intel hw has only one gpu write domain

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 2747 bytes --]

On Sun, 18 Apr 2010 17:45:25 -0700, Eric Anholt <eric@anholt.net> wrote:
> On Wed, 17 Mar 2010 22:29:27 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Wed, Mar 17, 2010 at 02:00:09PM -0700, Eric Anholt wrote:
> > > On Sun, 14 Mar 2010 12:18:15 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > > It therefore holds that all objects on the gpu_write_domain are
> > > > in this single write domain. Drop an if checking for the write
> > > > domain in process_flushing_list.
> > > > 
> > > > For some odd reason retire_commands thinks that the sampler cache
> > > > can be written to. Remove this and the assorted logic in do_execbuf.
> > > > 
> > > > Also check that userspace doesn't try to screw us over by claiming
> > > > to write to some strange cache.
> > > > 
> > > > The idea for this patch emerged from a discussion with Chris Wilson.
> > > 
> > > The intent of the flush_domains in retire_commands was to note that the
> > > read-only sampler cache was flushed and we could clear the read domains
> > > so that a later attempt to use it in the sampler cache wouldn't send out
> > > a new flush.
> > > 
> > > You've been looking into this stuff more than I have recently -- do you
> > > think that handling that information would be interesting?
> > 
> > atm we don't handle invalidating read-domains as efficient as
> > write-domains. Read: We don't globally update all bo's when doing a
> > sampler cache invalidations like we do when flushing the render cache. So
> > that flush_domain stuff from retire_commands was pure dead code.
> > 
> > We could do better of course. Problem is that tracking the buffers that
> > might benefit from a read_domains with lists like we're doing for the
> > write domain needs NUM_READ_DOMAIN list_heads per object. That's rather
> > gross. But we could keep track of the last seqno this bo was dirtied
> > (by either the gpu or the cpu, for the cpu just take the latest completed
> > seqno) and also keep track of the seqno of the last invalidate for each
> > read cache. At execbuf time we could then compare these two seqnos and
> > decide whether a invalidate is still needed.
> > 
> > I haven't implemented this because
> > - I don't have the hw that would benefit the most (i965).
> > - On different hw, I don't think we'll gain much. At least as long as
> >   multi-client drm usage is still uncommon.
> > - Currently we still have easier targets for faster performance ;)
> 
> OK.  Seems like a fair assessment of the situation.  Applied to -next.

Actually, having just noticed the "reject user write domains other than
RENDER."  That will break 965 occlusion queries.  I've gone back and
removed this patch from my -next tree.


[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx