From: Ben Widawsky <ben@bwidawsk.net>
To: intel-gfx@lists.freedesktop.org
Subject: forcewake junk, RFC, RFT(test)
Date: Fri, 8 Apr 2011 10:47:26 -0700 [thread overview]
Message-ID: <1302284850-8274-1-git-send-email-ben@bwidawsk.net> (raw)
I am requesting 3 things:
1. code review/request for comments
2. testing on SNB
3. performance regression on < SNB
review
------
The first two patches have nothing to do with the user's ability to
interact with registers. Those patches are about enforcing correctness
within our driver for newer generation products. In other words, if
patch 3 doesn't make sense, don't automatically drop 1, and 2.
review patch 1/2
----------------
The first change is straight forward. It attempts to fold the forcewake stuff
into our standard register read and write functions. Some overhead is added as
a result, but I'd guess it is nothing compared to the UC read about to happen,
and so will not be noticeable.
The existing method for doing the forcewake_get and put requires some
synchronization. For the most part it is protected by struct_mutex and life is
good. Adding a WARN to the get()/put() function is there more as documentation
to future developers, and reminders to the current ones.
To provide an interface to allow user space to use forcewake, I've decided to
change the mechanism that get()/put() operate by introducing a reference count.
The reference count itself must be protected by struct mutex (since we need
synchronization between the initial condition and the destructor). Imagine for
instance: Thread A does a get() and is in the middle of waking the GT (ref has
already been incremented), thread B comes in, thinks the GT is awake,
and incorrectly goes about its business. This does allow users of the
interface to only have to hold struct_mutex while doing the get() and
not for every read and write.
review patch 3
--------------
User space interface is mostly what you'd expect, except the in the case
of trying to get lockless access. This code is a bit meh, but to
remind everyone, it is root only debug code.
testing
-------
The assertion that forcewake is currently properly protected for the
most part, may not be true. People interested should run these patches
on SNB systems with their favorite graphics applications and report the
warnings that occur, they will be in the kernel log, ie. dmesg.
performance
-----------
Looking mostly for regressions on older systems. There is a slight
overhead added to all register reads and writes, which I think shouldn't
be noticeable, but who knows.
Thanks, and let the flames begin!
Ben
next reply other threads:[~2011-04-08 17:47 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-08 17:47 Ben Widawsky [this message]
2011-04-08 17:47 ` [PATCH 1/4] drm/i915: proper use of forcewake Ben Widawsky
2011-04-08 17:47 ` [PATCH 2/4] drm/i915: refcounts for forcewake Ben Widawsky
2011-04-08 17:47 ` [PATCH 3/4] drm/i915: userspace interface to the forcewake refcount Ben Widawsky
2011-04-08 17:47 ` [PATCH 4/4] drm/i915: optional fewer warning patch Ben Widawsky
2011-04-09 20:26 ` forcewake junk, part2 Ben Widawsky
2011-04-09 20:26 ` (no subject) Ben Widawsky
[not found] ` <1302380787-2957-3-git-send-email-ben@bwidawsk.net>
2011-04-09 20:26 ` [PATCH 1/4] drm/i915: proper use of forcewake Ben Widawsky
2011-04-09 20:26 ` [PATCH 2/4] drm/i915: refcounts for forcewake Ben Widawsky
2011-04-09 20:26 ` [PATCH 3/4] drm/i915: userspace interface to the forcewake refcount Ben Widawsky
2011-04-09 20:31 ` forcewake patches (ignore previous, please) Ben Widawsky
2011-04-09 20:31 ` [PATCH 1/4] drm/i915: proper use of forcewake Ben Widawsky
2011-04-09 20:31 ` [PATCH 2/4] drm/i915: refcounts for forcewake Ben Widawsky
2011-04-09 20:31 ` [PATCH 3/4] drm/i915: userspace interface to the forcewake refcount Ben Widawsky
2011-04-09 20:31 ` [PATCH 4/4] drm/i915: optional fewer warning patch Ben Widawsky
2011-04-09 22:31 ` Chris Wilson
2011-04-09 23:32 ` Ben Widawsky
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=1302284850-8274-1-git-send-email-ben@bwidawsk.net \
--to=ben@bwidawsk.net \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).