[Intel-gfx] [PATCH v2] drm/i915: Rework workaround data exporting to debugfs

2014-09-02 Thread Arun Siluvery
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.siluv...@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);
+   } 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;
+   

Re: [Intel-gfx] [PATCH v2] drm/i915: Rework workaround data exporting to debugfs

2014-09-02 Thread Daniel Vetter
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.siluv...@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?