On Mon, 07 Dec 2015, Wayne Boyer <wayne.boyer@xxxxxxxxx> wrote:
> The cherryview device shares many characteristics with the valleyview
> device. When support was added to the driver for cherryview, the
> corresponding device info structure included .is_valleyview = 1.
> This is not correct and leads to some confusion.
>
> This patch changes .is_valleyview to .is_cherryview in the cherryview
> device info structure and simplifies the IS_CHERRYVIEW macro.
> Then where appropriate, instances of IS_VALLEYVIEW are replaced with
> IS_VALLEYVIEW || IS_CHERRYVIEW or equivalent.
>
> v2: Use IS_VALLEYVIEW || IS_CHERRYVIEW instead of defining a new macro.
> Also add followup patches to fix issues discovered during the first
> review. (Ville)
>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Signed-off-by: Wayne Boyer <wayne.boyer@xxxxxxxxx>
I eyeballed through the changes here, comments inline. I didn't check
which places you missed, if any.
BR,
Jani.
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index a8721fc..a9ae642 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1142,8 +1142,34 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
> MEMSTAT_VID_SHIFT);
> 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_GEN9(dev)) {
> + } else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> + u32 freq_sts;
> +
> + mutex_lock(&dev_priv->rps.hw_lock);
> + freq_sts = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> + seq_printf(m, "PUNIT_REG_GPU_FREQ_STS: 0x%08x\n", freq_sts);
> + seq_printf(m, "DDR freq: %d MHz\n", dev_priv->mem_freq);
> +
> + seq_printf(m, "actual GPU freq: %d MHz\n",
> + intel_gpu_freq(dev_priv, (freq_sts >> 8) & 0xff));
> +
> + seq_printf(m, "current GPU freq: %d MHz\n",
> + intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq));
> +
> + seq_printf(m, "max GPU freq: %d MHz\n",
> + intel_gpu_freq(dev_priv, dev_priv->rps.max_freq));
> +
> + seq_printf(m, "min GPU freq: %d MHz\n",
> + intel_gpu_freq(dev_priv, dev_priv->rps.min_freq));
> +
> + seq_printf(m, "idle GPU freq: %d MHz\n",
> + intel_gpu_freq(dev_priv, dev_priv->rps.idle_freq));
> +
> + seq_printf(m,
> + "efficient (RPe) frequency: %d MHz\n",
> + intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq));
> + mutex_unlock(&dev_priv->rps.hw_lock);
> + } else if (IS_GEN6(dev) || IS_BROADWELL(dev) || IS_GEN9(dev)) {
Where did IS_GEN7() go?
> u32 rp_state_limits;
> u32 gt_perf_status;
> u32 rp_state_cap;
> @@ -1284,33 +1310,6 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
> seq_printf(m,
> "efficient (RPe) frequency: %d MHz\n",
> intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq));
> - } else if (IS_VALLEYVIEW(dev)) {
> - u32 freq_sts;
> -
> - mutex_lock(&dev_priv->rps.hw_lock);
> - freq_sts = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> - seq_printf(m, "PUNIT_REG_GPU_FREQ_STS: 0x%08x\n", freq_sts);
> - seq_printf(m, "DDR freq: %d MHz\n", dev_priv->mem_freq);
> -
> - seq_printf(m, "actual GPU freq: %d MHz\n",
> - intel_gpu_freq(dev_priv, (freq_sts >> 8) & 0xff));
> -
> - seq_printf(m, "current GPU freq: %d MHz\n",
> - intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq));
> -
> - seq_printf(m, "max GPU freq: %d MHz\n",
> - intel_gpu_freq(dev_priv, dev_priv->rps.max_freq));
> -
> - seq_printf(m, "min GPU freq: %d MHz\n",
> - intel_gpu_freq(dev_priv, dev_priv->rps.min_freq));
> -
> - seq_printf(m, "idle GPU freq: %d MHz\n",
> - intel_gpu_freq(dev_priv, dev_priv->rps.idle_freq));
> -
> - seq_printf(m,
> - "efficient (RPe) frequency: %d MHz\n",
> - intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq));
> - mutex_unlock(&dev_priv->rps.hw_lock);
> } else {
> seq_puts(m, "no P-state info available\n");
> }
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index a81c766..b156b08e 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -836,7 +836,7 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
>
> static void intel_init_dpio(struct drm_i915_private *dev_priv)
> {
> - if (!IS_VALLEYVIEW(dev_priv))
> + if (!(IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))
Hmm, I think I'd probably end up writing this as:
if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
Matter of style and phase of the moon I guess. But it's nicer
particularly when it's combined with other !platforms.
> return;
>
> /*
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 43761c5..4b1161d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -190,7 +190,7 @@ i915_gem_alloc_context_obj(struct drm_device *dev, size_t size)
> * would make the object snooped which might have a
> * negative performance impact.
> */
> - if (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev)) {
> + if (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)) {
Ahah, you seem to be ambivalent about !(vlv || chv) vs. !vlv && !chv
yourself. ;)
Also makes me think you didn't use sed or coccinelle for the change.
> ret = i915_gem_object_set_cache_level(obj, I915_CACHE_L3_LLC);
> /* Failure shouldn't ever happen this early */
> if (WARN_ON(ret)) {
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 070470f..e384d24 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -357,8 +357,8 @@ parse_general_features(struct drm_i915_private *dev_priv,
> if (general) {
> dev_priv->vbt.int_tv_support = general->int_tv_support;
> /* int_crt_support can't be trusted on earlier platforms */
> - if (bdb->version >= 155 &&
> - (HAS_DDI(dev_priv) || IS_VALLEYVIEW(dev_priv)))
> + if (bdb->version >= 155 && (HAS_DDI(dev_priv) ||
> + IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))
Hmm, is that the same as gen >= 7 && !ivb?
> dev_priv->vbt.int_crt_support = general->int_crt_support;
> dev_priv->vbt.lvds_use_ssc = general->enable_ssc;
> dev_priv->vbt.lvds_ssc_freq =
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
