[Intel-gfx] [PATCH 00/13] drm-intel-collector - review request

[Chris Wilson]

On Tue, Sep 24, 2013 at 12:15:19PM +0200, Daniel Vetter wrote:
> On Mon, Sep 23, 2013 at 05:33:17PM -0300, Rodrigo Vivi wrote:
> > This is another drm-intel-collector push for review:
> > http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=drm-intel-collector
> > 
> > Here goes the list in order for better reviewers assignment:
> > 
> > Patch 01/13 - 816e102 drm/i915: check that the i965g/gm 4G limit is really obeyed - Reviewer: Damien
> > Patch 02/13 - 036f5da drm/i915: Move the conditional seqno query into the tracepoint - Reviewer: Ville
> > Patch 03/13 - f381b05 drm/i915: Add some missing steps to i915_driver_load error path - Reviewer: Ben
> 
> Lazy reviers, tsk ;-)
> 
> > Patch 04/13 - f648dba drm/i915: Asynchronously perform the set-base for a simple modeset - Reviewer: 
> 
> Dunno what to do exactly with this, given that the android guys seem to
> need more vblank waits in the plane ioctls. I'd drop for now, we need a
> more coherent story here ... Or just wait for nuclear flips.

That is irrevelant for that patch. Try it, see how much it improves
switching between fb/X.

> > Patch 05/13 - ec31a06 drm/i915: Align tiled scanouts from stolen memory to 256k in the GTT - Reviewer: 
> 
> I think scanouts in stolen have died for now, too many issues. So I guess
> we can drop this, since there are other issues.
> 
> > Patch 06/13 - 3b2a43a drm/i915: Pair seqno completion tracepoint with its dispatch - Reviewer: 
> 
> There's some bikeshed pending from Thomas Gleixner in the same are to move
> the i915_ring_get_seqno out of the tracepoint. I think if we frob this we
> might as well do it right.

Hah, disagree completely there since that is pessimising the common
case. There is a reason why the expensive operation should only be done
when the tracepoint is active and if he wants to rewrite the tracepoint
API then he is welcome to submit patches...
 
> > Patch 07/13 - 4d98ddd drm/i915: Initialise min/max frequencies before updating RPS registers - Reviewer: 
> 
> Ville had some bikesheds pending, and there's the issue of the delayed rps
> work. I'd drop and wait for a new version.

No, he didn't. The bikesheds were not on the patch but on the
surrounding code.

> > Patch 08/13 - 35983bd drm/i915: Delay the relase of the forcewake by a jiffie - Reviewer: 
> 
> Already reviewed by Ville and needs a bit work it seems, you can drop and
> wait for a new version.

The version I (thought I) sent last has reviewed-by tags. It shouldn't
be dropped.
 
> > Patch 09/13 - 14141ee drm/i915: Add a tracepoint for using a semaphore - Reviewer: 
> 
> Ville.
> 
> > Patch 10/13 - 06d2851 drm/i915: Boost RPS frequency for CPU stalls - Reviewer: 
> > Patch 11/13 - e64dce9 drm/i915: Tweak RPS thresholds to more aggressively downclock - Reviewer: 
> 
> It sounds like Chris has updated versions of these somewhere. Imo you can
> drop these here. Also Jesse should take a look.

These seriously need review, they make a dramatic improvement to certain
benchmarks, and have a clear impact on user experience.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre