Patches queued to drm-intel-fixes

Daniel Vetter daniel.vetter at ffwll.ch
Tue Dec 13 15:04:26 PST 2011


Hi Keith,

I've noticed that you merged my patch "rm/i915: properly prefault for
pread/pwrite" into your -fixes branch (which I assume is headed for
3.2). Please remove that from your queue again for the following
reasons:

- The right thing to do is to fix up the prefault handlers in pagemap.h
- It's really ugly code (which Chris Wilson later on complained
about), so ugly actually that it confused you while reviewing it.
- It changes the semantics of pread/pwrite in funny ways (something
you actually complained about in review a while ago, too).
- This bug has been lying around for almost half a year already, so I
don't see the need for a rush now.
- It only papers over the underlying issue, the real minimal and
proper fix is queued up (and reviewed) in my-next in my own git repo
for 3.3.
- I actually managed to blow things up while playing with the prefault
stuff, so it's imo not really risk-free.
- But most important this late in the -rc cylce: It doesn't fix a regression.

I've also noticed that you have my patch "drm/i915: check ACTHD of all
rings" queued up in -fixes. I wouldn't have minded this getting merged
a few weeks ago into an early -rc but again I think it's too late for
this one for the following reasons:
- It touches on the hangcheck code, one of the most important pieces
to be able to debug issues and hence support users of our driver, but
also one of the least tested ones (we essentially only test it when
hitting actual hangs).
- A similar patch by Ben Widawsky actually blew things up for Chris Wilson.
- Again it doesn't fix a regression.

Dave, please reject a pull request for 3.2 containing these patches -
I've already embarrassed myself with the vt-d oneliner (which should
imo have been merged about 4 weeks ago, but mea culpa).

Yours, Daniel
-- 
Daniel Vetter
daniel.vetter at ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch


More information about the dri-devel mailing list