From mboxrd@z Thu Jan 1 00:00:00 1970 From: Deepak S Subject: Re: [PATCH 3/7] drm/i915: keep freq/opcode conversion function more generic Date: Fri, 11 Jul 2014 12:20:57 +0530 Message-ID: <53BF8951.8000307@linux.intel.com> References: <1404978387-28180-1-git-send-email-deepak.s@linux.intel.com> <1404978387-28180-4-git-send-email-deepak.s@linux.intel.com> <20140709120355.GR17271@phenom.ffwll.local> <53BF677B.8030200@linux.intel.com> <20140710062835.GZ17271@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 83CCC6E1A6 for ; Wed, 9 Jul 2014 23:55:57 -0700 (PDT) In-Reply-To: <20140710062835.GZ17271@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thursday 10 July 2014 11:58 AM, Daniel Vetter wrote: > On Fri, Jul 11, 2014 at 09:56:35AM +0530, Deepak S wrote: >> On Wednesday 09 July 2014 05:33 PM, Daniel Vetter wrote: >>> On Thu, Jul 10, 2014 at 01:16:23PM +0530, deepak.s@linux.intel.com wrote: >>>> From: Deepak S >>>> >>>> Since freq/encode conversion formula changes from platform to platform, >>>> create a generic wrapper function and having platform check inside this >>>> help to simpilfy adding newer platform freq/opcode conversion. >>>> >>>> Signed-off-by: Deepak S >>>> --- >>>> drivers/gpu/drm/i915/i915_debugfs.c | 14 +++++----- >>>> drivers/gpu/drm/i915/i915_drv.h | 4 +-- >>>> drivers/gpu/drm/i915/i915_sysfs.c | 18 ++++++------- >>>> drivers/gpu/drm/i915/intel_pm.c | 52 +++++++++++++++++++++++++++---------- >>>> 4 files changed, 56 insertions(+), 32 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c >>>> index b3b56c4..dd7078d 100644 >>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c >>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >>>> @@ -1140,14 +1140,14 @@ static int i915_frequency_info(struct seq_file *m, void *unused) >>>> val = valleyview_rps_max_freq(dev_priv); >>>> seq_printf(m, "max GPU freq: %d MHz\n", >>>> - vlv_gpu_freq(dev_priv, val)); >>>> + intel_gpu_freq(dev_priv, val)); >>> intel_ is a bit too generic a prefix for a function which seems to be only >>> used on byt+chv. I'd just add a if (IS_CHERRYVIEW) ... else /* vlv code */ >>> to both functions and not extract further. >>> >>> Aside: Since marketing stopped using vlv and switched to byt we're using >>> vlv for code shared by byt and chv and byt_ for byt-only code. Helps a bit >>> to keep things appart. >>> -Daniel >> Ok. Will it be Ok to use "vlv_gpu_freq" and have BYT and CHV check under >> this function? > Yeah, that's my idea. > >> The reason why i made more generic is it will help us to add conversion >> logic for future platforms > We can look at this again when it happpens. With the current code this > doesn't include desktop rps so the intel_ prefix was a bit confusing. > -Daniel Ok. Thanks for the feedback. I will update and send new patch set