[Intel-gfx] [PATCH v2] drm/i915: Rework workaround data exporting to debugfs
Daniel Vetter
daniel at ffwll.ch
Tue Sep 2 11:56:39 CEST 2014
On Tue, Sep 02, 2014 at 10:15:31AM +0100, Arun Siluvery wrote:
> Now w/a are organized in an array so we know exactly how many of them
> are applied; use the same array while exporting data to debugfs and
> remove the temporary array we currently have in driver priv structure.
>
> Signed-off-by: Arun Siluvery <arun.siluvery at linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 29 +++++++++++++++++------------
> drivers/gpu/drm/i915/i915_drv.h | 14 --------------
> drivers/gpu/drm/i915/intel_ringbuffer.c | 15 +++++++++++++++
> drivers/gpu/drm/i915/intel_ringbuffer.h | 8 ++++++++
> 4 files changed, 40 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 2727bda..0c1e294 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2465,6 +2465,14 @@ static int i915_wa_registers(struct seq_file *m, void *unused)
> struct drm_info_node *node = (struct drm_info_node *) m->private;
> struct drm_device *dev = node->minor->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_ring_context_rodata ro_data;
> +
> + ret = ring_context_rodata(dev, &ro_data);
> + if (ret) {
> + seq_puts(m, "Workarounds applied: 0\n");
> + DRM_DEBUG_DRIVER("Workaround table not available !!\n");
> + return -EINVAL;
> + }
>
> ret = mutex_lock_interruptible(&dev->struct_mutex);
> if (ret)
> @@ -2472,18 +2480,15 @@ static int i915_wa_registers(struct seq_file *m, void *unused)
>
> intel_runtime_pm_get(dev_priv);
>
> - seq_printf(m, "Workarounds applied: %d\n", dev_priv->num_wa_regs);
> - for (i = 0; i < dev_priv->num_wa_regs; ++i) {
> - u32 addr, mask;
> -
> - addr = dev_priv->intel_wa_regs[i].addr;
> - mask = dev_priv->intel_wa_regs[i].mask;
> - dev_priv->intel_wa_regs[i].value = I915_READ(addr) | mask;
> - if (dev_priv->intel_wa_regs[i].addr)
> - seq_printf(m, "0x%X: 0x%08X, mask: 0x%08X\n",
> - dev_priv->intel_wa_regs[i].addr,
> - dev_priv->intel_wa_regs[i].value,
> - dev_priv->intel_wa_regs[i].mask);
> + seq_printf(m, "Workarounds applied: %d\n", ro_data.num_items);
> + for (i = 0; i < ro_data.num_items; ++i) {
> + u32 addr, mask, value;
> +
> + addr = ro_data.init_context[i].addr;
> + mask = ro_data.init_context[i].mask;
> + value = ro_data.init_context[i].value;
> + seq_printf(m, "0x%X: 0x%08X, mask: 0x%08X\n",
> + addr, value, mask);
> }
>
> intel_runtime_pm_put(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 49b7be7..bcf79f0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1553,20 +1553,6 @@ struct drm_i915_private {
> struct intel_shared_dpll shared_dplls[I915_NUM_PLLS];
> int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
>
> - /*
> - * workarounds are currently applied at different places and
> - * changes are being done to consolidate them so exact count is
> - * not clear at this point, use a max value for now.
> - */
> -#define I915_MAX_WA_REGS 16
> - struct {
> - u32 addr;
> - u32 value;
> - /* bitmask representing WA bits */
> - u32 mask;
> - } intel_wa_regs[I915_MAX_WA_REGS];
> - u32 num_wa_regs;
> -
> /* Reclocking support */
> bool render_reclock_avail;
> bool lvds_downclock_avail;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 3ed0ad5..7262c10 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -769,6 +769,21 @@ static int ring_init_context(struct intel_engine_cs *ring)
> return ret;
> }
>
> +int ring_context_rodata(struct drm_device *dev,
> + struct intel_ring_context_rodata *ro_data)
> +{
> + if (IS_CHERRYVIEW(dev)) {
> + ro_data->init_context = chv_ring_init_context;
> + ro_data->num_items = ARRAY_SIZE(chv_ring_init_context);
> + } else if (IS_BROADWELL(dev)) {
> + ro_data->init_context = bdw_ring_init_context;
> + ro_data->num_items = ARRAY_SIZE(bdw_ring_init_context);
This will kinda break my idea that we could put _all_ static register
w/a into these schem, so also everything that's in the various init_clock
gating functions.
The goal of the test is after all to make sure that we set the w/a bits in
the right place, so if you only check the w/a emitted in the context init
(and this change here kinda bakes this in) then it will not be really
useful.
Maybe you should (just as a prove of concept of these refactorings)
convert the chv or bdw w/a in the init_clock_gating to this infrastructure
too?
Thanks, Daniel
> + } else
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static int init_render_ring(struct intel_engine_cs *ring)
> {
> struct drm_device *dev = ring->dev;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index c9ed06c..33454ce 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -343,6 +343,14 @@ struct intel_wa_reg {
> #define INIT_UNMASKED_WA(_addr, _value) \
> INIT_WA_REG(0, _addr, ~(_value), _value)
>
> +struct intel_ring_context_rodata {
> + u32 num_items;
> + const struct intel_wa_reg *init_context;
> +};
> +
> +int ring_context_rodata(struct drm_device *dev,
> + struct intel_ring_context_rodata *ro_data);
> +
> bool intel_ring_initialized(struct intel_engine_cs *ring);
>
> static inline unsigned
> --
> 2.0.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list