From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [RFC][PATCH 1/2] drm/i915: Don't increase the GPU frequency from the delayed VLV rps timer Date: Wed, 26 Jun 2013 12:21:31 +0200 Message-ID: <20130626102131.GD18285@phenom.ffwll.local> References: <1372185491-9688-1-git-send-email-ville.syrjala@linux.intel.com> <1372185491-9688-2-git-send-email-ville.syrjala@linux.intel.com> <20130625115328.593abf46@jbarnes-desktop> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ee0-f46.google.com (mail-ee0-f46.google.com [74.125.83.46]) by gabe.freedesktop.org (Postfix) with ESMTP id 145E7E6319 for ; Wed, 26 Jun 2013 03:21:40 -0700 (PDT) Received: by mail-ee0-f46.google.com with SMTP id d41so7168591eek.19 for ; Wed, 26 Jun 2013 03:21:40 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130625115328.593abf46@jbarnes-desktop> 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: Jesse Barnes Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, Jun 25, 2013 at 11:53:28AM -0700, Jesse Barnes wrote: > On Tue, 25 Jun 2013 21:38:10 +0300 > ville.syrjala@linux.intel.com wrote: > = > > From: Ville Syrj=E4l=E4 > > = > > There's little point in increasing the GPU frequency from the delayed > > rps work on VLV. Now when the GPU is idle, the GPU frequency actually > > keeps dropping gradually until it hits the minimum, whereas previously > > it just ping-ponged constantly between RPe and RPe-1. > > = > > Signed-off-by: Ville Syrj=E4l=E4 > > --- > > drivers/gpu/drm/i915/intel_pm.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > = > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/int= el_pm.c > > index 96cfb3e..eaf0fa2 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -3464,7 +3464,8 @@ static void vlv_rps_timer_work(struct work_struct= *work) > > * min freq available. > > */ > > mutex_lock(&dev_priv->rps.hw_lock); > > - valleyview_set_rps(dev_priv->dev, dev_priv->rps.rpe_delay); > > + if (dev_priv->rps.cur_delay > dev_priv->rps.rpe_delay) > > + valleyview_set_rps(dev_priv->dev, dev_priv->rps.rpe_delay); > > mutex_unlock(&dev_priv->rps.hw_lock); > > } > > = > = > Hm that's not what I saw here; I stopped getting the timer interrupts > when the GPU went idle. But that could be explained by punit fw > differences. > = > So this change looks ok to me. > = > Reviewed-by: Jesse Barnes Queued for -next, thanks for the patch. In case you noodle around in here some more, I'd also vote for a s/timer_work/delayed_work/ color change: The execution enviroment of a time is _much_ different than from a work, so sticking to consistent naming helps in reading code. -Daniel -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch