From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH] drm/i915: [GEN7] Use HW scheduler for fixed function shaders Date: Sun, 15 Apr 2012 10:14:24 -0700 Message-ID: <20120415101424.13b830ce@bwidawsk.net> References: <1334454092-8878-1-git-send-email-ben@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from cloud01.chad-versace.us (184-106-247-128.static.cloud-ips.com [184.106.247.128]) by gabe.freedesktop.org (Postfix) with ESMTP id A42949E7B4 for ; Sun, 15 Apr 2012 10:15:49 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Eugeni Dodonov Cc: intel-gfx@lists.freedesktop.org, Bernard Kilarski , stable List-Id: intel-gfx@lists.freedesktop.org On Sun, 15 Apr 2012 11:55:36 -0300 Eugeni Dodonov wrote: > On Sat, Apr 14, 2012 at 22:41, Ben Widawsky wrote: > > > This originally started as a patch from Bernard as a way of simply > > setting the VS scheduler. After submitting the RFC patch, we > > decided to also modify the DS scheduler. To be most explicit, I've > > made the patch explicitly set all scheduler modes, and included the > > defines for other modes (in case someone feels frisky later). > > > > The rest of the story gets a bit weird. The first version of the > > patch showed an almost unbelievable performance improvement. Since > > rebasing my branch it appears the performance improvement has gone, > > unfortunately. But setting these bits seem to be the right thing to > > do given that the docs describe corruption that can occur with the > > default settings. > > > > In summary, I am seeing no more perf improvements (or regressions) > > in my limited testing, but we believe this should be set to prevent > > rendering corruption, therefore cc stable. > > > > v1: Clear bit 4 also (Ken + Eugeni) > > Do a full clear + set of the bits we want (Me). > > > > Cc: Bernard Kilarski > > Cc: stable > > Reviewed-by (RFC): Kenneth Graunke > > Signed-off-by: Ben Widawsky > > > > Very nice! > > I also suspect that maybe the initial performance improvement you've > seen with previous testing could be related to the occasional turbo > disabling we've been seeing in other cases as well (e.g., > https://bugs.freedesktop.org/show_bug.cgi?id=44006). > > But as for this patch, I have just one comment/suggestion below, but > other than that: > > Reviewed-by: Eugeni Dodonov > > +static void gen7_setup_fixed_func_scheduler(struct drm_i915_private > > *dev_priv) > > > > Perhaps this functions should be named > ivybridge_setup_fixed_func_scheduler instead? > > Even if those bits are not ivy bridge-exclusive, this specific > explicit setup applies to ivb only.. > I wasn't sure if we wanted this for VLV or not. In fact, originally the patch did call this in the VLV setup, but since I decided to CC stable (per Ken's idea) I removed the VLV part. If Jesse, or someone could confirm we don't want this for VLV, I agree with your commen, and I'd probably just go back and inline the register write. FWIW, it does *seem* like we don't want to set this on HSW. Anyway, thanks for your review.