Re: [PATCH 3/7] drm/i915: keep freq/opcode conversion function more generic

[Deepak S <deepak.s@linux.intel.com>]

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 <deepak.s@linux.intel.com>
>>>>
>>>> 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 <deepak.s@linux.intel.com>
>>>> ---
>>>>   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