From: Daniel Vetter <daniel@ffwll.ch>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Daniel Vetter <daniel@ffwll.ch>,
Patrik Jakobsson <patrik.r.jakobsson@gmail.com>,
Dave Airlie <airlied@redhat.com>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
greg@kroah.com
Subject: Re: [PATCH 14/15] gma500: nuke the PSB debug stuff
Date: Mon, 13 Jun 2011 21:35:32 +0200 [thread overview]
Message-ID: <20110613193532.GB2761@phenom.ffwll.local> (raw)
In-Reply-To: <20110613164403.4cb74675@lxorguk.ukuu.org.uk>
On Mon, Jun 13, 2011 at 04:44:03PM +0100, Alan Cox wrote:
> > Given that I've mucked around in drm_mm quite a bit I'd be very interested
> > in your opinion about what's weird in it (and presumably what could be
> > improved). Can you elaborate?
>
> Mostly the API, which is also somewhat poorly documented. I've not dug
> into the internals beyond trying to figure out how to use it. Looking at
> the users the API is non-obvious, the interface involves a mix of calls
> and digging deep into struct internals.
Well, I've rewritten the api but that transition is not yet complete, i.e.
the new interface is there, but no driver is converted yet. The old api
has a two-step allocation (search_free + get_block) and frees allocated
ranges with put_block. The new api has just insert_node and remove_node,
struct drm_mm_node must be allocated by the caller (to get rid of the racy
preallocation scheme and allow embedding of drm_mm_node).
There's also the lru scanning helpers (only used by intel). Under specific
conditions (should be all documented in comments) this can be used as an
eviction roaster.
Generally only node->start should be used by drivers, otherwise
drm_mm_node should be opaque. At least that's the idea.
btw, old conversion patches for i915 to the new interface are at:
http://cgit.freedesktop.org/~danvet/drm/log/?h=embed-gtt-space
When creating the new interfaces I've tried to document them. Ideas to
improve that highly welcome.
> I'd have expected an API that had allocate/free type methods and exposed
> the needed information directly or very easily not one that has drivers
> doing. drm_sman seems to be attempt in that direction but its also rather
> odd with interfaces like
Should be fixed, see above ;-)
[snip]
> Possibly drm_sman and friends should just wrap whatever simple native
> allocator exists on the OS ?
Well, drm_mm has some graphics specific magic (range restricted scans and
the eviction helpers). But simpler stuff like sman is imo just "drm likes
to reinvent the wheel". Unfortunately I don't have the hw, so I haven't
dared to touch it (the snippet you've posted is a prime example why).
Yours, Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
next prev parent reply other threads:[~2011-06-13 19:35 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-08 10:10 [PATCH 00/15] GMA500 KMS and GEM Alan Cox
2011-06-08 10:11 ` [PATCH 01/15] gma500: fix warnings Alan Cox
2011-06-08 10:11 ` [PATCH 02/15] gma500: Skip bogus LVDS VBT mode and check for LVDS before adding backlight Alan Cox
2011-06-08 10:11 ` [PATCH 03/15] gma500: Make GTT pages uncached Alan Cox
2011-06-08 10:12 ` [PATCH 04/15] gma500: Ensure the frame buffer has a linear virtual mapping Alan Cox
2011-06-08 10:12 ` [PATCH 05/15] gma500: Set the correct bits according to the pipe Alan Cox
2011-06-08 10:12 ` [PATCH 06/15] staging/gma500: get control from firmware framebuffer if conflicts Alan Cox
2011-06-08 10:13 ` [PATCH 07/15] gma500: Fix uninitialized variable and style issues Alan Cox
2011-06-08 10:13 ` [PATCH 08/15] gma500: revamp frame buffer creation and handling Alan Cox
2011-06-08 10:13 ` [PATCH 09/15] gma500: Do sane FB cleanup Alan Cox
2011-06-08 10:14 ` [PATCH 10/15] gma500: trim some of the debug Alan Cox
2011-06-08 10:14 ` [PATCH 11/15] gma500: polish for completion of this phase Alan Cox
2011-06-08 10:14 ` [PATCH 12/15] gma500: 2D acceleration tidying Alan Cox
2011-06-08 10:15 ` [PATCH 13/15] gma500: nuke the last bits of TTM code Alan Cox
2011-06-08 10:15 ` [PATCH 14/15] gma500: nuke the PSB debug stuff Alan Cox
2011-06-09 1:10 ` Patrik Jakobsson
2011-06-09 8:11 ` Alan Cox
2011-06-09 10:36 ` Dave Airlie
2011-06-09 11:45 ` Patrik Jakobsson
2011-06-09 12:04 ` Alan Cox
2011-06-12 19:02 ` Daniel Vetter
2011-06-13 15:44 ` Alan Cox
2011-06-13 19:35 ` Daniel Vetter [this message]
2011-06-09 11:55 ` Alan Cox
2011-06-08 10:15 ` [PATCH 15/15] gma500: Kill spare kref Alan Cox
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=20110613193532.GB2761@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=airlied@redhat.com \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=dri-devel@lists.freedesktop.org \
--cc=greg@kroah.com \
--cc=linux-kernel@vger.kernel.org \
--cc=patrik.r.jakobsson@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox