[Intel-gfx] [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
Jeff McGee
jeff.mcgee at intel.com
Fri Aug 1 01:18:45 CEST 2014
On Thu, Jul 31, 2014 at 04:55:58AM -0700, Mcaulay, Alistair wrote:
> Hi Jeff,
>
> Some more comments in the code.
>
> Thanks,
> Alistair.
>
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf Of jeff.mcgee at intel.com
> Sent: Thursday, July 31, 2014 3:00 AM
> To: intel-gfx at lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 2/2] drm/i915/chv: Implement SSEU info for CHV
>
> From: Jeff McGee <jeff.mcgee at intel.com>
>
> Cherryview can have different SSEU configurations within a given PCI ID, so we collect the info from the fuse register.
>
> I don't currently have access to a CHV, much less one with an interesting fuse config. So I have compile-checked this only!
>
> Signed-off-by: Jeff McGee <jeff.mcgee at intel.com>
> ---
> drivers/gpu/drm/i915/Makefile | 3 +-
> drivers/gpu/drm/i915/i915_dma.c | 2 ++
> drivers/gpu/drm/i915/i915_reg.h | 13 ++++++++
> drivers/gpu/drm/i915/intel_sseu.c | 64 +++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_sseu.h | 2 ++
> 5 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/intel_sseu.c
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 91bd167..9a0f411 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -32,7 +32,8 @@ i915-y += i915_cmd_parser.o \
> i915_irq.o \
> i915_trace_points.o \
> intel_ringbuffer.o \
> - intel_uncore.o
> + intel_uncore.o \
> + intel_sseu.o
>
> # autogenerated null render state
> i915-y += intel_renderstate_gen6.o \
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index f581848..384ef65 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1772,6 +1772,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> goto out_gem_unload;
> }
>
> + intel_sseu_init(dev);
> +
> intel_power_domains_init(dev_priv);
>
> if (drm_core_check_feature(dev, DRIVER_MODESET)) { diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 28e21ed..24a2d56 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5624,6 +5624,19 @@ enum punit_power_well {
> #define GEN7_MISCCPCTL (0x9424)
> #define GEN7_DOP_CLOCK_GATE_ENABLE (1<<0)
>
> +/* Fuse readout registers for GT */
> +#define CHV_FUSE_GT 0x182168
> +#define CHV_FUSE_GT_SUBSLICE_DISABLE_SS0 10
> +#define CHV_FUSE_GT_SUBSLICE_DISABLE_SS1 11
>
> These should be:
> #define CHV_FUSE_GT_SUBSLICE_DISABLE_SS0 (1 << 10)
> #define CHV_FUSE_GT_SUBSLICE_DISABLE_SS1 (1 << 11)
>
> +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK (0xf<<16)
> +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT 16
>
> Why not use the shift define here?
> #define CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK (0xf<<CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_SHIFT)
> and for the others.
>
See answer in previous patch
-Jeff
> +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK (0xf<<20)
> +#define CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_SHIFT 20
> +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK (0xf<<24)
> +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_SHIFT 24
> +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK (0xf<<28)
> +#define CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_SHIFT 28
> +
> /* IVYBRIDGE DPF */
> #define GEN7_L3CDERRST1 0xB008 /* L3CD Error Status 1 */
> #define HSW_L3CDERRST11 0xB208 /* L3CD Error Status register 1 slice 1 */
> diff --git a/drivers/gpu/drm/i915/intel_sseu.c b/drivers/gpu/drm/i915/intel_sseu.c
> new file mode 100644
> index 0000000..6ba4830
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_sseu.c
> @@ -0,0 +1,64 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person
> +obtaining a
> + * copy of this software and associated documentation files (the
> +"Software"),
> + * to deal in the Software without restriction, including without
> +limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> +sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom
> +the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> +next
> + * paragraph) shall be included in all copies or substantial portions
> +of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> +SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> +OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> +ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +#include <linux/bitops.h>
> +#include "i915_drv.h"
> +#include "intel_sseu.h"
> +
> +void intel_sseu_init(struct drm_device *dev) {
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_sseu_info sseu_info;
> + u32 gp = 0;
> +
> + /* Collect SSEU info per device */
> + if (IS_CHERRYVIEW(dev)) {
> + u32 fuse, mask_ss, mask_eu;
> +
> + fuse = I915_READ(CHV_FUSE_GT);
> + mask_ss = fuse & (CHV_FUSE_GT_SUBSLICE_DISABLE_SS0 |
> + CHV_FUSE_GT_SUBSLICE_DISABLE_SS1);
> + mask_eu = fuse & (CHV_FUSE_GT_EU_DISABLE_SS0_ROW0_MASK |
> + CHV_FUSE_GT_EU_DISABLE_SS0_ROW1_MASK |
> + CHV_FUSE_GT_EU_DISABLE_SS1_ROW0_MASK |
> + CHV_FUSE_GT_EU_DISABLE_SS1_ROW1_MASK);
> + sseu_info.slice_cnt = 1;
> + sseu_info.subslice_cnt = 2 - hweight32(mask_ss);
> + sseu_info.eu_cnt = 16 - hweight32(mask_eu);
> + sseu_info.threads_per_eu = 7;
> + }
> +
> + /* Pack SSEU info bitfield for I915_PARAM_SSEU_INFO */
> + gp |= (sseu_info.slice_cnt << I915_SSEU_INFO_SLICE_CNT_SHIFT) &
> + I915_SSEU_INFO_SLICE_CNT_MASK;
>
> In theory, there should be no need to use the mask, unless something has gone wrong.
> Maybe:
>
> WARN_ON((sseu_info.slice_cnt << I915_SSEU_INFO_SLICE_CNT_SHIFT) > I915_SSEU_INFO_SLICE_CNT_MASK );
> etc
>
>
I agree. Will make similar change in next version.
-Jeff
> + gp |= (sseu_info.subslice_cnt << I915_SSEU_INFO_SUBSLICE_CNT_SHIFT) &
> + I915_SSEU_INFO_SUBSLICE_CNT_MASK;
> + gp |= (sseu_info.eu_cnt << I915_SSEU_INFO_EU_CNT_SHIFT) &
> + I915_SSEU_INFO_EU_CNT_MASK;
> + gp |= (sseu_info.threads_per_eu << I915_SSEU_INFO_THREADS_PER_EU_SHIFT) &
> + I915_SSEU_INFO_THREADS_PER_EU_MASK;
> + sseu_info.gp_sseu_info = gp;
> +
> + /* Copy SSEU info to the const device info with pointer magic */
> + *(struct intel_sseu_info *)&dev_priv->info.sseu = sseu_info; }
> diff --git a/drivers/gpu/drm/i915/intel_sseu.h b/drivers/gpu/drm/i915/intel_sseu.h
> index 7db7175..0257358 100644
> --- a/drivers/gpu/drm/i915/intel_sseu.h
> +++ b/drivers/gpu/drm/i915/intel_sseu.h
> @@ -37,4 +37,6 @@ struct intel_sseu_info {
> u32 gp_sseu_info;
> };
>
> +extern void intel_sseu_init(struct drm_device *dev);
> +
> #endif
> --
> 2.0.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list