On Fri, Feb 06, 2015 at 08:26:37PM +0530, akash.g...@intel.com wrote:
> From: Akash Goel <akash.g...@intel.com>
> 
> Added support for SKL in the 'i915_frequency_info' debugfs function
> 
> Signed-off-by: Akash Goel <akash.g...@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9af17fb..32c62a2 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1089,7 +1089,7 @@ static int i915_frequency_info(struct seq_file *m, void 
> *unused)
>               seq_printf(m, "Current P-state: %d\n",
>                          (rgvstat & MEMSTAT_PSTATE_MASK) >> 
> MEMSTAT_PSTATE_SHIFT);
>       } else if (IS_GEN6(dev) || (IS_GEN7(dev) && !IS_VALLEYVIEW(dev)) ||
> -                IS_BROADWELL(dev)) {
> +                IS_BROADWELL(dev) || IS_GEN9(dev)) {

Can we be optimistic by default (and hope next platform will be no extra
work by having a INTEL_INFO(dev)->gen >= 9)?

>               u32 gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS);
>               u32 rp_state_limits = I915_READ(GEN6_RP_STATE_LIMITS);
>               u32 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> @@ -1109,8 +1109,12 @@ static int i915_frequency_info(struct seq_file *m, 
> void *unused)
>  
>               reqf = I915_READ(GEN6_RPNSWREQ);
>               reqf &= ~GEN6_TURBO_DISABLE;
> +             if (!IS_GEN9(dev))
> +                     reqf &= ~GEN6_TURBO_DISABLE;

It seems like you to remove one masking of bit 31? (can we have >= 9 as
well?).  

Maybe a simpler way to go about it would be:

                if (INTEL_INFO(dev)->gen >= 9)
                        reqf >>= 23;
                else {
                        reqf &= ~GEN6_TURBO_DISABLE;
                        if (IS_HASWELL(dev) || IS_BROADWELL(dev))
                                reqf >>= 24;
                        else
                                reqf >>= 25;
                }

>               if (IS_HASWELL(dev) || IS_BROADWELL(dev))
>                       reqf >>= 24;
> +             else if IS_GEN9(dev)
> +                     reqf >>= 23;
>               else
>                       reqf >>= 25;
>               reqf = intel_gpu_freq(dev_priv, reqf);
> @@ -1128,6 +1132,8 @@ static int i915_frequency_info(struct seq_file *m, void 
> *unused)
>               rpprevdown = I915_READ(GEN6_RP_PREV_DOWN);
>               if (IS_HASWELL(dev) || IS_BROADWELL(dev))
>                       cagf = (rpstat & HSW_CAGF_MASK) >> HSW_CAGF_SHIFT;
> +             else if (IS_GEN9(dev))
> +                     cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
>               else
>                       cagf = (rpstat & GEN6_CAGF_MASK) >> GEN6_CAGF_SHIFT;

cagf (as well as reqf) is(are) used in a printf saying they are Mhz. That looks
wrong.

>               cagf = intel_gpu_freq(dev_priv, cagf);
> @@ -1152,7 +1158,7 @@ static int i915_frequency_info(struct seq_file *m, void 
> *unused)
>                          pm_ier, pm_imr, pm_isr, pm_iir, pm_mask);
>               seq_printf(m, "GT_PERF_STATUS: 0x%08x\n", gt_perf_status);
>               seq_printf(m, "Render p-state ratio: %d\n",
> -                        (gt_perf_status & 0xff00) >> 8);
> +                        (gt_perf_status & (IS_GEN9(dev) ? 0x1ff00 : 0xff00)) 
> >> 8);

Eeek, that's a weird name to say freq. Here the 16.66 unit strikes back, can we
have at least a comment?

>               seq_printf(m, "Render p-state VID: %d\n",
>                          gt_perf_status & 0xff);
>               seq_printf(m, "Render p-state limit: %d\n",
> -- 
> 1.9.2
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to