From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ben Widawsky <ben@bwidawsk.net>
Subject: Re: [PATCH 19/26] drm/i915: add ValleyView specific
 force wake get/put functions
Date: Mon, 26 Mar 2012 11:20:22 -0700
Message-ID: <20120326182022.GB17740@bolo_yeung.jf.intel.com>
References: <1332452348-8814-1-git-send-email-jbarnes@virtuousgeek.org>
	<1332452348-8814-20-git-send-email-jbarnes@virtuousgeek.org>
	<CAC7Lmnsw0o=MKAjoXqfMiw2nbn6YehT+c71_4WzqCv6mpZdPQg@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org>
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 BA4F0A0B6D
	for <intel-gfx@lists.freedesktop.org>;
	Mon, 26 Mar 2012 11:20:30 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <CAC7Lmnsw0o=MKAjoXqfMiw2nbn6YehT+c71_4WzqCv6mpZdPQg@mail.gmail.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=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 <eugeni@dodonov.net>
Cc: intel-gfx@lists.freedesktop.org
List-Id: intel-gfx@lists.freedesktop.org

On Fri, Mar 23, 2012 at 02:20:09PM -0300, Eugeni Dodonov wrote:
> On Thu, Mar 22, 2012 at 18:39, Jesse Barnes <jbarnes@virtuousgeek.org>wrote:
> 
> > ValleyView handles force wake differently than previous chipsets, so add
> > a couple of new functions for it.  But leave it disabled by default
> > until we test it (need a chip with the Punit enabled first).
> >
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  #define NEEDS_FORCE_WAKE(dev_priv, reg) \
> >        (((dev_priv)->info->gen >= 6) && \
> >         ((reg) < 0x40000) &&            \
> > -        ((reg) != FORCEWAKE))
> > +        ((reg) != FORCEWAKE)) && \
> > +       (!IS_VALLEYVIEW((dev_priv)->dev))
> >
> 
> In the spirit of bikeshedding, I think that NEEDS_FORCE_WAKE is becoming a
> bit scary this way. But I don't know if it makes sense to move the gen and
> dev check into a feature flag, to avoid similar issues with possible future
> generations of chips..

Agreed. I think the better approach is to check if the fwake function
pointers are !NULL also (since it will apply to other chipsets
potentially as well).

> 
> -- 
> Eugeni Dodonov
> <http://eugeni.dodonov.net/>

> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx