From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eugeni Dodonov Subject: Re: [PATCH] drm/i915: [GEN7] Use HW scheduler for fixed function shaders Date: Sun, 15 Apr 2012 11:55:36 -0300 Message-ID: References: <1334454092-8878-1-git-send-email-ben@bwidawsk.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0455444967==" Return-path: Received: from mail-yw0-f49.google.com (mail-yw0-f49.google.com [209.85.213.49]) by gabe.freedesktop.org (Postfix) with ESMTP id EC5379E73D for ; Sun, 15 Apr 2012 07:56:16 -0700 (PDT) Received: by yhjj52 with SMTP id j52so2405974yhj.36 for ; Sun, 15 Apr 2012 07:56:16 -0700 (PDT) In-Reply-To: <1334454092-8878-1-git-send-email-ben@bwidawsk.net> 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: Ben Widawsky Cc: intel-gfx@lists.freedesktop.org, Bernard Kilarski , stable , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org --===============0455444967== Content-Type: multipart/alternative; boundary=20cf303dd2ee578ae304bdb8e8c5 --20cf303dd2ee578ae304bdb8e8c5 Content-Type: text/plain; charset=ISO-8859-1 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.. -- Eugeni Dodonov --20cf303dd2ee578ae304bdb8e8c5 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
On Sat, Apr 14, 2012 at 22:41, Ben Widawsky <ben= @bwidawsk.net> 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<= br> 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 <bernard.r.kilarski@intel.com>
Cc: stable <= stable@vger.kernel.org>
Reviewed-by (RFC): Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
<= div>
Very nice!

I also suspect that = maybe the initial performance improvement you've seen with previous tes= ting could be related to the occasional turbo disabling we've been seei= ng in other cases as well (e.g.,=A0https://bugs.freedesktop.org/sho= w_bug.cgi?id=3D44006).

But as for this patch, I have just one comment/suggesti= on below, but other than that:

Reviewed-by: Eugeni= Dodonov <= eugeni.dodonov@intel.com>

+static void gen7_setup_fixed= _func_scheduler(struct drm_i915_private *dev_priv)
Perhaps this functions should be named ivybridge_setup_fixed_fun= c_scheduler instead?

Even if those bits are not iv= y bridge-exclusive, this specific explicit setup applies to ivb only..

--
Eugeni Dodonov

--20cf303dd2ee578ae304bdb8e8c5-- --===============0455444967== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============0455444967==--