All of lore.kernel.org
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Jerome Glisse <j.glisse@gmail.com>
Cc: DRI development list <dri-devel@lists.freedesktop.org>
Subject: Re: TTM and AGP conflicts
Date: Thu, 5 Jan 2012 21:26:20 -0500	[thread overview]
Message-ID: <20120106022620.GA5598@phenom.dumpdata.com> (raw)
In-Reply-To: <20120103225933.GA14711@phenom.dumpdata.com>

On Tue, Jan 03, 2012 at 05:59:33PM -0500, Konrad Rzeszutek Wilk wrote:
> On Tue, Jan 03, 2012 at 05:43:40PM -0500, Jerome Glisse wrote:
> > On Fri, Dec 23, 2011 at 01:19:43AM +0000, James Simmons wrote:
> > > 
> > > > > Hi!
> > > > >
> > > > >        I updated the openchrome tree and while testing on the AGP system
> > > > > discovered some interesting problems with the new TTM changes. The
> > > > > problems center around the ttm_tt_[un]populate which I modeled after the
> > > > > radeon and nouveau driver.
> > > > >        First problem I noticed was on a AGP system that my ttm_tt_populate
> > > > > function would oops. Tracking it down I found the problem was the
> > > > > ttm_agp_tt_create calls ttm_tt_init instead of ttm_dma_tt_init so once my
> > > > > ttm_tt_populate function would attempt to touch the dma_address it would
> > > > > oops. The second issue is the assumption of the cast for struct ttm_tt in
> > > > > both the populate and unpopulate function. For the AGP case the proper
> > > > > case would be to ttm_agp_backend.
> > > > >        At this point my assumption is the ttm_bo_move function has to be
> > > > > rewritten to handle the AGP case to avoid calling ttm_tt_bind and in all
> > > > > cases ttm_tt_bind needs to be avoided. Looking at the radeon and nouveau
> > > > > drivers I don't see that testing happening. Am I just missing something?
> > > > 
> > > > Happens on AGP radeons as well:
> > > > https://bugs.freedesktop.org/show_bug.cgi?id=43719
> > > 
> > > 	So I'm not crazy, so this needs to be fixed. Here is what my 
> > > understanding of the TTM layer is. My impression is that struct ttm_bo_driver
> > > handles multiple domains, AGP, pcie etc and in each method you have to 
> > > handle each specific domain you support. Also *move gives the impression of
> > > moving between these different domains. 
> > > 	Where as for struct ttm_backend_func was more for allocating from
> > > a specific domain. Also I never saw a clear way to handle multiple backends. 
> > > For example my AGP systems can also do pci dma as well. 
> > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> > Attached is patch to fix this, so sorry about that, i must have lost my
> > agp change along the way when working on the patchset. This patch is not
> > extensively tested, i will do more testing tomorrow on more gpu, might
> > even found an nvidia agp i can try. Again sorry for breaking this.
> 
> Hey Jerome,
> 
> Was going to look at this week and see how it performs before (and after)
> the squash ttm bind+populate operation. Any thoughts of what benchmarks I
> should run?

OK, Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>


I ran it on my radeon AGP cards and they performed nicely. I do need to
run it with the nouveau AGP card tomorrow (barring any move_notify issues).

  reply	other threads:[~2012-01-06  2:28 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-22 21:56 TTM and AGP conflicts James Simmons
2011-12-22 22:00 ` Alex Deucher
2011-12-23  1:19   ` James Simmons
2011-12-24  3:44     ` Jerome Glisse
2012-01-03 22:43     ` Jerome Glisse
2012-01-03 22:59       ` Konrad Rzeszutek Wilk
2012-01-06  2:26         ` Konrad Rzeszutek Wilk [this message]
2012-01-04 15:36       ` James Simmons
2012-01-04 15:49         ` Jerome Glisse
2012-01-04 16:04           ` James Simmons
2012-01-04 16:35             ` Jerome Glisse
2012-01-06 15:51               ` James Simmons
2012-01-09  9:07                 ` Thomas Hellstrom
2012-01-09 15:21                   ` Jerome Glisse
2012-01-09 16:46                   ` Konrad Rzeszutek Wilk

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=20120106022620.GA5598@phenom.dumpdata.com \
    --to=konrad.wilk@oracle.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=j.glisse@gmail.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.