Re: [Intel-gfx] [PATCH] drm/i915: Don't just say it, actually force edp vdd

2014-03-02 Thread Chris Wilson
On Sun, Mar 02, 2014 at 01:05:06AM +0100, Patrik Jakobsson wrote:
 This solves the blank screen problem on the MacBook Air 6,2. The
 comments state that we need to force edp vdd so lets put it back. The
 EDP_FORCE_VDD bit was removed in commit:
 
 commit dff392dbd258381a6c3164f38420593f2d291e3b
 drm/i915: don't touch the VDD when disabling the panel
 
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74628
 Cc: Paulo Zanoni paulo.r.zan...@intel.com
 Cc: Chris Wilson ch...@chris-wilson.co.uk
 Signed-off-by: Patrik Jakobsson patrik.r.jakobs...@gmail.com
 ---
  drivers/gpu/drm/i915/intel_dp.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index 57552eb..44de6f7 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -1252,7 +1252,7 @@ void ironlake_edp_panel_off(struct intel_dp *intel_dp)
   pp = ironlake_get_pp_control(intel_dp);
   /* We need to switch off panel power _and_ force vdd, for otherwise some
* panels get very unhappy and cease to work. */
 - pp = ~(POWER_TARGET_ON | PANEL_POWER_RESET | EDP_BLC_ENABLE);
 + pp = ~(POWER_TARGET_ON | EDP_FORCE_VDD | PANEL_POWER_RESET | 
 EDP_BLC_ENABLE);
  
   pp_ctrl_reg = _pp_ctrl_reg(intel_dp);

I'd feel more comfortable with;

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 46d065b..876184f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1872,9 +1872,11 @@ static void intel_disable_dp(struct intel_encoder 
*encoder)
 
/* Make sure the panel is off before trying to change the mode. But also
 * ensure that we have vdd while we switch off the panel. */
+   edp_panel_vdd_on(intel_dp);
intel_edp_backlight_off(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
intel_edp_panel_off(intel_dp);
+   edp_panel_vdd_off(intel_dp, true);
 
/* cpu edp my only be disable _after_ the cpu pipe/plane is disabled. */
if (!(port == PORT_A || IS_VALLEYVIEW(dev)))

Or that vdd_off may have to be post-disable.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: sprinkle static

2014-03-02 Thread Daniel Vetter
Apparently we've missed a few more than what Fengguang's 0-day tester
recently reported in i915_irq.c ... Makes sparse happy again (ignore
some spurious stuff about ksyms of exported functions).

Cc: kbuild test robot fengguang...@intel.com
Cc: Imre Deak imre.d...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_irq.c  |  4 ++--
 drivers/gpu/drm/i915/intel_display.c | 10 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 69f2ebbd6af1..484415bb3497 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -482,7 +482,7 @@ done:
 }
 
 
-void
+static void
 __i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
   u32 enable_mask, u32 status_mask)
 {
@@ -506,7 +506,7 @@ __i915_enable_pipestat(struct drm_i915_private *dev_priv, 
enum pipe pipe,
POSTING_READ(reg);
 }
 
-void
+static void
 __i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
u32 enable_mask, u32 status_mask)
 {
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index c559c58af483..863500893e96 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7692,7 +7692,7 @@ err:
return ERR_PTR(ret);
 }
 
-struct drm_framebuffer *
+static struct drm_framebuffer *
 intel_framebuffer_create(struct drm_device *dev,
 struct drm_mode_fb_cmd2 *mode_cmd,
 struct drm_i915_gem_object *obj)
@@ -10541,10 +10541,10 @@ static const struct drm_framebuffer_funcs 
intel_fb_funcs = {
.create_handle = intel_user_framebuffer_create_handle,
 };
 
-int intel_framebuffer_init(struct drm_device *dev,
-  struct intel_framebuffer *intel_fb,
-  struct drm_mode_fb_cmd2 *mode_cmd,
-  struct drm_i915_gem_object *obj)
+static int intel_framebuffer_init(struct drm_device *dev,
+ struct intel_framebuffer *intel_fb,
+ struct drm_mode_fb_cmd2 *mode_cmd,
+ struct drm_i915_gem_object *obj)
 {
int aligned_height;
int pitch_limit;
-- 
1.8.5.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Always use kref tracking for contexts.

2014-03-02 Thread Ben Widawsky
On Fri, Feb 28, 2014 at 08:06:50PM +, Chris Wilson wrote:
 If we always initialize kref for the context, even if we are using fake
 contexts for hangstats when there is no hw support, we can forgo the
 dance to dereference the ctx-obj and inspect whether we are permitted
 to use kref inside i915_gem_context_reference() and _unreference().
 
 My ulterior motive here is to improve the debugging of a use-after-free
 of ctx-obj. This patch avoids the dereference here and instead forces
 the assertion checks associated with kref.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 ---
  drivers/gpu/drm/i915/i915_drv.h |  6 ++
  drivers/gpu/drm/i915/i915_gem_context.c | 37 
 +
  2 files changed, 26 insertions(+), 17 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 8af8e0dd3943..17a37578053c 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -2380,14 +2380,12 @@ i915_gem_context_get(struct drm_i915_file_private 
 *file_priv, u32 id);
  void i915_gem_context_free(struct kref *ctx_ref);
  static inline void i915_gem_context_reference(struct i915_hw_context *ctx)
  {
 - if (ctx-obj  HAS_HW_CONTEXTS(ctx-obj-base.dev))
 - kref_get(ctx-ref);
 + kref_get(ctx-ref);
  }
  
  static inline void i915_gem_context_unreference(struct i915_hw_context *ctx)
  {
 - if (ctx-obj  HAS_HW_CONTEXTS(ctx-obj-base.dev))
 - kref_put(ctx-ref, i915_gem_context_free);
 + kref_put(ctx-ref, i915_gem_context_free);
  }
  
  static inline bool i915_gem_context_is_default(const struct i915_hw_context 
 *c)
 diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
 b/drivers/gpu/drm/i915/i915_gem_context.c
 index e9f888ea67d6..48dc5f8f21bf 100644
 --- a/drivers/gpu/drm/i915/i915_gem_context.c
 +++ b/drivers/gpu/drm/i915/i915_gem_context.c
 @@ -179,12 +179,9 @@ create_vm_for_ctx(struct drm_device *dev, struct 
 i915_hw_context *ctx)
  }
  
  static struct i915_hw_context *
 -__create_hw_context(struct drm_device *dev,
 -   struct drm_i915_file_private *file_priv)
 +__ctx_alloc(void)
  {
 - struct drm_i915_private *dev_priv = dev-dev_private;
   struct i915_hw_context *ctx;
 - int ret;
  
   ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
   if (ctx == NULL)
 @@ -193,6 +190,21 @@ __create_hw_context(struct drm_device *dev,
   kref_init(ctx-ref);
   INIT_LIST_HEAD(ctx-link);
  
 + return ctx;
 +}
 +
 +static struct i915_hw_context *
 +__create_hw_context(struct drm_device *dev,
 +   struct drm_i915_file_private *file_priv)
 +{
 + struct drm_i915_private *dev_priv = dev-dev_private;
 + struct i915_hw_context *ctx;
 + int ret;
 +
 + ctx = __ctx_alloc();
 + if (IS_ERR(ctx))
 + return ctx;
 +

   ctx-obj = i915_gem_object_create_stolen(dev, 
 dev_priv-hw_context_size);
  How this working for you ^ ?


   if (ctx-obj == NULL)
   ctx-obj = i915_gem_alloc_object(dev, 
 dev_priv-hw_context_size);
 @@ -492,11 +504,9 @@ int i915_gem_context_open(struct drm_device *dev, struct 
 drm_file *file)
  
   if (!HAS_HW_CONTEXTS(dev)) {
   /* Cheat for hang stats */
 - file_priv-private_default_ctx =
 - kzalloc(sizeof(struct i915_hw_context), GFP_KERNEL);
 -
 - if (file_priv-private_default_ctx == NULL)
 - return -ENOMEM;
 + file_priv-private_default_ctx = __ctx_alloc();
 + if (IS_ERR(file_priv-private_default_ctx))
 + return PTR_ERR(file_priv-private_default_ctx);
  
   file_priv-private_default_ctx-vm = dev_priv-gtt.base;
   return 0;
 @@ -521,14 +531,15 @@ void i915_gem_context_close(struct drm_device *dev, 
 struct drm_file *file)
  {
   struct drm_i915_file_private *file_priv = file-driver_priv;
  
 - if (!HAS_HW_CONTEXTS(dev)) {
 - kfree(file_priv-private_default_ctx);
 + if (IS_ERR_OR_NULL(file_priv-private_default_ctx))
   return;
 +
 + if (HAS_HW_CONTEXTS(dev)) {
 + idr_for_each(file_priv-context_idr, context_idr_cleanup, 
 NULL);
 + idr_destroy(file_priv-context_idr);
   }
  
 - idr_for_each(file_priv-context_idr, context_idr_cleanup, NULL);
   i915_gem_context_unreference(file_priv-private_default_ctx);
 - idr_destroy(file_priv-context_idr);
  }

I don't see too much harm in just initializing the idr regardless to
keep the close path simpler. Then stick a WARN in context_idr_cleanup if
it actually does anything, fake_context_idr_cleanup();

I have some recollection of hitting a really tricky thing while
developing PPGTT where the order of the unref in the above sequence
really mattered. Looking again though, I don't see anything familiar
- but my suggestion would put me completely at ease.

In either case:
Reviewed-by: Ben Widawsky 

Re: [Intel-gfx] [PATCH] drm/i915: Convert the forcewake worker into a timer func

2014-03-02 Thread Ben Widawsky
On Fri, Feb 28, 2014 at 06:44:03PM +, Chris Wilson wrote:
 We don't want to suffer scheduling delay when turning off the GPU after
 waking it up to touch registers. Ideally, we only want to keep the GPU
 awake for the register access sequence, with a single forcewake dance on
 the first access and release immediately after the last. We set a timer
 on the first access so that we only dance once and on the next scheduler
 tick, we drop the forcewake again.
 
 This moves the cleanup routine from the common i915 workqueue to a timer
 func so that we don't anger powertop, and drop the forcewake again
 quicker.
 
 v2: Enable the deferred force_wake_put for regular register reads as
 well.

So I went to test this patch today. Oddly this isn't showing up in any
of my profiles on the latest drm-intel-nightly. I just tested today, and
Daniel made it a 3.14-rc4 based thing today. It was still present on
3.13.0. Actually, i915_gem_retire_work_handler is the only one even
showing up in powertop, which is how it should be IMO.

Therefore I'm not sure what we should do with this.

 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Ben Widawsky b...@bwidawsk.net
 Cc: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/i915_drv.h |  2 +-
  drivers/gpu/drm/i915/intel_uncore.c | 34 +++---
  2 files changed, 16 insertions(+), 20 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index b22765192018..8af8e0dd3943 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -508,7 +508,7 @@ struct intel_uncore {
   unsigned fw_rendercount;
   unsigned fw_mediacount;
  
 - struct delayed_work force_wake_work;
 + struct timer_list force_wake_timer;
  };
  
  #define DEV_INFO_FOR_EACH_FLAG(func, sep) \
 diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
 b/drivers/gpu/drm/i915/intel_uncore.c
 index c62841404c82..8ee171178bfe 100644
 --- a/drivers/gpu/drm/i915/intel_uncore.c
 +++ b/drivers/gpu/drm/i915/intel_uncore.c
 @@ -289,10 +289,8 @@ void vlv_force_wake_put(struct drm_i915_private 
 *dev_priv,
   spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
  }
  
 -static void gen6_force_wake_work(struct work_struct *work)
 +static void gen6_force_wake_timer(struct drm_i915_private *dev_priv)
  {
 - struct drm_i915_private *dev_priv =
 - container_of(work, typeof(*dev_priv), 
 uncore.force_wake_work.work);
   unsigned long irqflags;
  
   spin_lock_irqsave(dev_priv-uncore.lock, irqflags);
 @@ -405,9 +403,8 @@ void gen6_gt_force_wake_put(struct drm_i915_private 
 *dev_priv, int fw_engine)
   spin_lock_irqsave(dev_priv-uncore.lock, irqflags);
   if (--dev_priv-uncore.forcewake_count == 0) {
   dev_priv-uncore.forcewake_count++;
 - mod_delayed_work(dev_priv-wq,
 -  dev_priv-uncore.force_wake_work,
 -  1);
 + mod_timer_pinned(dev_priv-uncore.force_wake_timer,
 +  jiffies + 1);
   }
   spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags);
  
 @@ -484,17 +481,15 @@ gen5_read##x(struct drm_i915_private *dev_priv, off_t 
 reg, bool trace) { \
  static u##x \
  gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
   REG_READ_HEADER(x); \
 - if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
 - if (dev_priv-uncore.forcewake_count == 0) \
 - dev_priv-uncore.funcs.force_wake_get(dev_priv, \
 - FORCEWAKE_ALL); \
 - val = __raw_i915_read##x(dev_priv, reg); \
 - if (dev_priv-uncore.forcewake_count == 0) \
 - dev_priv-uncore.funcs.force_wake_put(dev_priv, \
 - FORCEWAKE_ALL); \
 - } else { \
 - val = __raw_i915_read##x(dev_priv, reg); \
 + if (dev_priv-uncore.forcewake_count == 0  \
 + NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
 + dev_priv-uncore.funcs.force_wake_get(dev_priv, \
 +   FORCEWAKE_ALL); \
 + dev_priv-uncore.forcewake_count++; \
 + mod_timer_pinned(dev_priv-uncore.force_wake_timer, \
 +  jiffies + 1); \
   } \
 + val = __raw_i915_read##x(dev_priv, reg); \
   REG_READ_FOOTER; \
  }
  
 @@ -681,8 +676,9 @@ void intel_uncore_init(struct drm_device *dev)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
  
 - INIT_DELAYED_WORK(dev_priv-uncore.force_wake_work,
 -   gen6_force_wake_work);
 + setup_timer(dev_priv-uncore.force_wake_timer,
 + (void (*)(unsigned long))gen6_force_wake_timer,
 + (unsigned long)dev_priv);
  
   if (IS_VALLEYVIEW(dev)) {
   dev_priv-uncore.funcs.force_wake_get = 

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Make num_sprites a per-pipe value

2014-03-02 Thread Zhenyu Wang
On 2014.02.28 16:42:15 +, Damien Lespiau wrote:
 In the future, we need to be able to specify per-pipe number of
 planes/sprites. Let's start today!
 

The number of sprite planes on each pipe is always fixed in our HW.
Do we really need this in the future?

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: Digital signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] sna: Add multiple planes support for Xv video on sprite

2014-03-02 Thread Zhenyu Wang
On VLV there're two sprite planes for each pipe that both can be
used simultaneously for Xv video. This one trys to expose those
capability through multiple Xv ports on sprite video adaptor.

I've tried to use this to validate new two sprite planes for
single pipe on VLV which works fine.

Signed-off-by: Zhenyu Wang zhen...@linux.intel.com
---
 src/sna/sna.h  |   5 ++-
 src/sna/sna_display.c  | 106 ++---
 src/sna/sna_video_sprite.c |  87 -
 3 files changed, 131 insertions(+), 67 deletions(-)

diff --git a/src/sna/sna.h b/src/sna/sna.h
index 4651632..ce51fef 100644
--- a/src/sna/sna.h
+++ b/src/sna/sna.h
@@ -285,6 +285,7 @@ struct sna {
unsigned num_real_crtc;
unsigned num_real_output;
unsigned num_fake;
+   unsigned num_sprite;
} mode;
 
struct sna_dri {
@@ -442,9 +443,9 @@ static inline void sna_dri_close(struct sna *sna, ScreenPtr 
pScreen) { }
 #endif
 void sna_dri_pixmap_update_bo(struct sna *sna, PixmapPtr pixmap);
 
-extern bool sna_crtc_set_sprite_rotation(xf86CrtcPtr crtc, uint32_t rotation);
+extern bool sna_crtc_set_sprite_rotation(xf86CrtcPtr crtc, XvPortPtr port, 
uint32_t rotation);
 extern int sna_crtc_to_pipe(xf86CrtcPtr crtc);
-extern uint32_t sna_crtc_to_sprite(xf86CrtcPtr crtc);
+extern uint32_t sna_crtc_to_sprite(xf86CrtcPtr crtc, XvPortPtr port);
 extern uint32_t sna_crtc_id(xf86CrtcPtr crtc);
 
 CARD32 sna_format_for_depth(int depth);
diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index de08cb9..2e05733 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -100,13 +100,19 @@ struct rotation {
uint32_t current;
 };
 
+struct sna_crtc_sprite {
+   uint32_t id;
+   struct rotation sprite_rotation;
+};
+
 struct sna_crtc {
struct drm_mode_modeinfo kmode;
int dpms_mode;
PixmapPtr scanout_pixmap;
struct kgem_bo *bo, *shadow_bo;
uint32_t cursor;
-   uint32_t sprite;
+   int num_sprite;
+   struct sna_crtc_sprite *sprite;
bool shadow;
bool fallback_shadow;
bool transform;
@@ -115,7 +121,6 @@ struct sna_crtc {
 
uint32_t rotation;
struct rotation primary_rotation;
-   struct rotation sprite_rotation;
 };
 
 struct sna_property {
@@ -200,9 +205,24 @@ int sna_crtc_to_pipe(xf86CrtcPtr crtc)
return to_sna_crtc(crtc)-pipe;
 }
 
-uint32_t sna_crtc_to_sprite(xf86CrtcPtr crtc)
+static int sna_sprite_num(XvPortPtr port)
 {
-   return to_sna_crtc(crtc)-sprite;
+   XvAdaptorPtr adaptor = port-pAdaptor;
+   int i;
+
+   for (i = 0; i  adaptor-nPorts; i++) {
+   if (port == adaptor-pPorts[i])
+   return i;
+   }
+   return 0;
+}
+
+uint32_t sna_crtc_to_sprite(xf86CrtcPtr crtc, XvPortPtr port)
+{
+   struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
+   int num = sna_sprite_num(port);
+
+   return sna_crtc-sprite[num].id;
 }
 
 #ifndef NDEBUG
@@ -674,15 +694,18 @@ rotation_reset(struct rotation *r)
r-current = 0;
 }
 
-bool sna_crtc_set_sprite_rotation(xf86CrtcPtr crtc, uint32_t rotation)
+bool sna_crtc_set_sprite_rotation(xf86CrtcPtr crtc, XvPortPtr port, uint32_t 
rotation)
 {
+   struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
+   int num = sna_sprite_num(port);
+
DBG((%s: CRTC:%d [pipe=%d], sprite=%u set-rotation=%x\n,
 __FUNCTION__,
-to_sna_crtc(crtc)-id, to_sna_crtc(crtc)-pipe, 
to_sna_crtc(crtc)-sprite,
+sna_crtc-id, sna_crtc-pipe, sna_crtc-sprite[num].id,
 rotation));
 
return rotation_set(to_sna(crtc-scrn),
-   to_sna_crtc(crtc)-sprite_rotation,
+   sna_crtc-sprite[num].sprite_rotation,
rotation);
 }
 
@@ -1717,6 +1740,7 @@ sna_crtc_destroy(xf86CrtcPtr crtc)
sna_crtc_hide_cursor(crtc);
gem_close(to_sna(crtc-scrn)-kgem.fd, sna_crtc-cursor);
 
+   free(sna_crtc-sprite);
free(sna_crtc);
crtc-driver_private = NULL;
 }
@@ -1750,32 +1774,39 @@ static const xf86CrtcFuncsRec sna_crtc_funcs = {
 #endif
 };
 
-static int
-sna_crtc_find_sprite(struct sna *sna, int pipe)
+static void
+sna_crtc_find_sprite(struct sna *sna, struct sna_crtc *sna_crtc, int pipe)
 {
 #ifdef DRM_IOCTL_MODE_GETPLANERESOURCES
struct drm_mode_get_plane_res r;
-   uint32_t *planes, id = 0;
+   uint32_t *planes, *id;
int i;
 
VG_CLEAR(r);
r.count_planes = 0;
if (drmIoctl(sna-kgem.fd, DRM_IOCTL_MODE_GETPLANERESOURCES, r))
-   return 0;
+   return;
 
if (!r.count_planes)
-   return 0;
+   return;
 
planes = malloc(sizeof(uint32_t)*r.count_planes);
-   if (planes == NULL)
-   return 0;
+   id = malloc(sizeof(uint32_t)*r.count_planes);
+   if (planes 

[Intel-gfx] [PATCH] drm/i915: Add boot paramter to control rps boost at boot time.

2014-03-02 Thread deepak . s
From: Deepak S deepa...@intel.com

We are adding a module paramter to control rps boost. By default, we
enable the boost for better performace. Based on the need (perf/power)
we can either enable/disable.

Signed-off-by: Deepak S deepa...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h|  1 +
 drivers/gpu/drm/i915/i915_gem.c| 15 ++-
 drivers/gpu/drm/i915/i915_params.c |  8 
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2baeeef..398b101 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1963,6 +1963,7 @@ struct i915_params {
int panel_use_ssc;
int vbt_sdvo_panel_type;
int enable_rc6;
+   int enable_rps_boost;
int enable_fbc;
bool enable_hangcheck;
int enable_ppgtt;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a8a069f..1478ea6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -990,6 +990,17 @@ static bool can_wait_boost(struct drm_i915_file_private 
*file_priv)
return !atomic_xchg(file_priv-rps_wait_boost, true);
 }
 
+static int  intel_enable_rps_boost(struct drm_device *dev)
+{
+   /* No RPS Boost before Ironlake */
+   if (INTEL_INFO(dev)-gen  6)
+   return 0;
+
+   /* Respect the kernel parameter if it is set */
+   return i915.enable_rps_boost;
+
+}
+
 /**
  * __wait_seqno - wait until execution of seqno has finished
  * @ring: the ring expected to report seqno
@@ -1030,7 +1041,9 @@ static int __wait_seqno(struct intel_ring_buffer *ring, 
u32 seqno,
timeout_expire = timeout ? jiffies + 
timespec_to_jiffies_timeout(timeout) : 0;
 
if (dev_priv-info-gen = 6  can_wait_boost(file_priv)) {
-   gen6_rps_boost(dev_priv);
+   if (intel_enable_rps_boost(ring-dev))
+   gen6_rps_boost(dev_priv);
+
if (file_priv)
mod_delayed_work(dev_priv-wq,
 file_priv-mm.idle_work,
diff --git a/drivers/gpu/drm/i915/i915_params.c 
b/drivers/gpu/drm/i915/i915_params.c
index c743057..a19bd7d 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -47,6 +47,7 @@ struct i915_params i915 __read_mostly = {
.prefault_disable = 0,
.reset = true,
.invert_brightness = 0,
+   .enable_rps_boost = 1,
 };
 
 module_param_named(modeset, i915.modeset, int, 0400);
@@ -153,3 +154,10 @@ MODULE_PARM_DESC(invert_brightness,
report PCI device ID, subsystem vendor and subsystem device ID 
to dri-de...@lists.freedesktop.org, if your machine needs it. 
It will then be included in an upcoming module version.);
+
+module_param_named(enable_rps_boost, i915.enable_rps_boost, int, 0600);
+MODULE_PARM_DESC(enable_rps_boost,
+   Enable/Disable boost RPS frequency (default: true));
+
+
+
-- 
1.8.5.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v2] drm/i915/vlv: WA for Turbo and RC6 to work together.

2014-03-02 Thread deepak . s
From: Deepak S deepa...@intel.com

With RC6 enabled, BYT has an HW issue in determining the right
Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide
on increasing/decreasing the freq. This logic will monitor C0
counters of render/media power-wells over EI period and takes
necessary action based on these values

v2: Refactor duplicate code. (ville)

Signed-off-by: Deepak S deepa...@intel.com

---
 drivers/gpu/drm/i915/i915_drv.h |  19 ++
 drivers/gpu/drm/i915/i915_irq.c | 146 ++--
 drivers/gpu/drm/i915/i915_reg.h |  15 +
 drivers/gpu/drm/i915/intel_pm.c |  50 ++
 4 files changed, 213 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 728b9c3..2baeeef 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -957,6 +957,12 @@ struct i915_suspend_saved_registers {
u32 savePCH_PORT_HOTPLUG;
 };
 
+struct intel_rps_ei_calc {
+   u32 cz_ts_ei;
+   u32 render_ei_c0;
+   u32 media_ei_c0;
+};
+
 struct intel_gen6_power_mgmt {
/* work and pm_iir are protected by dev_priv-irq_lock */
struct work_struct work;
@@ -969,10 +975,16 @@ struct intel_gen6_power_mgmt {
u8 rp1_delay;
u8 rp0_delay;
u8 hw_max;
+   u8 hw_min;
 
bool rp_up_masked;
bool rp_down_masked;
 
+   u32 cz_freq;
+   u32 ei_interrupt_count;
+
+   bool use_RC0_residency_for_turbo;
+
int last_adj;
enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
 
@@ -1531,6 +1543,13 @@ typedef struct drm_i915_private {
/* gen6+ rps state */
struct intel_gen6_power_mgmt rps;
 
+   /* rps wa up ei calculation */
+   struct intel_rps_ei_calc rps_up_ei;
+
+   /* rps wa down ei calculation */
+   struct intel_rps_ei_calc rps_down_ei;
+
+
/* ilk-only ips/rps state. Everything in here is protected by the global
 * mchdev_lock in intel_pm.c */
struct intel_ilk_power_mgmt ips;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 56edff3..93b6ebf 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1023,6 +1023,120 @@ void gen6_set_pm_mask(struct drm_i915_private *dev_priv,
}
 }
 
+static u32 vlv_c0_residency(struct drm_i915_private *dev_priv,
+   struct  intel_rps_ei_calc *rps_ei)
+{
+   u32 cz_ts, cz_freq_khz;
+   u32 render_count, media_count;
+   u32 elapsed_render, elapsed_media, elapsed_time;
+   u32 residency = 0;
+
+   cz_ts = vlv_punit_read(dev_priv, PUNIT_REG_CZ_TIMESTAMP);
+   cz_freq_khz = DIV_ROUND_CLOSEST(dev_priv-mem_freq * 1000, 4);
+
+   render_count = I915_READ(VLV_RENDER_C0_COUNT_REG);
+   media_count = I915_READ(VLV_MEDIA_C0_COUNT_REG);
+
+   if (rps_ei-cz_ts_ei == 0) {
+   rps_ei-cz_ts_ei = cz_ts;
+   rps_ei-render_ei_c0 = render_count;
+   rps_ei-media_ei_c0 = media_count;
+
+   return dev_priv-rps.cur_delay;
+   }
+
+   elapsed_time = cz_ts - rps_ei-cz_ts_ei;
+   rps_ei-cz_ts_ei = cz_ts;
+
+   elapsed_render = render_count - rps_ei-render_ei_c0;
+   rps_ei-render_ei_c0 = render_count;
+
+   elapsed_media = media_count - rps_ei-media_ei_c0;
+   rps_ei-media_ei_c0 = media_count;
+
+   /* Convert all the counters into common unit of milli sec */
+   elapsed_time /= VLV_CZ_CLOCK_TO_MILLI_SEC;
+   elapsed_render /=  cz_freq_khz;
+   elapsed_media /= cz_freq_khz;
+
+   /* Calculate overall C0 residency percentage only
+   * if elapsed time is non zero
+   */
+   if (elapsed_time) {
+   residency =
+   ((max(elapsed_render, elapsed_media) * 100)
+   / elapsed_time);
+   }
+
+   return residency;
+}
+
+
+/**
+ * vlv_calc_delay_from_C0_counters - Increase/Decrease freq based on GPU
+ * busy-ness calculated from C0 counters of render  media power wells
+ * @dev_priv: DRM device private
+ *
+ */
+static u32 vlv_calc_delay_from_C0_counters(struct drm_i915_private *dev_priv)
+{
+   u32 residency_C0_up = 0, residency_C0_down = 0;
+   u8 new_delay;
+
+   dev_priv-rps.ei_interrupt_count++;
+
+   WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock));
+
+
+   if (dev_priv-rps_up_ei.cz_ts_ei == 0) {
+   vlv_c0_residency(dev_priv, dev_priv-rps_up_ei);
+   vlv_c0_residency(dev_priv, dev_priv-rps_down_ei);
+   return dev_priv-rps.cur_delay;
+   }
+
+
+   /* To down throttle, C0 residency should be less than down threshold
+   * for continous EI intervals. So calculate down EI counters
+   * once in VLV_INT_COUNT_FOR_DOWN_EI
+   */
+   if (dev_priv-rps.ei_interrupt_count == VLV_INT_COUNT_FOR_DOWN_EI) {
+
+   dev_priv-rps.ei_interrupt_count = 0;
+
+ 

Re: [Intel-gfx] [PATCH] drm/i915: Convert the forcewake worker into a timer func

2014-03-02 Thread Chris Wilson
On Sun, Mar 02, 2014 at 05:01:26PM -0800, Ben Widawsky wrote:
 On Fri, Feb 28, 2014 at 06:44:03PM +, Chris Wilson wrote:
  We don't want to suffer scheduling delay when turning off the GPU after
  waking it up to touch registers. Ideally, we only want to keep the GPU
  awake for the register access sequence, with a single forcewake dance on
  the first access and release immediately after the last. We set a timer
  on the first access so that we only dance once and on the next scheduler
  tick, we drop the forcewake again.
  
  This moves the cleanup routine from the common i915 workqueue to a timer
  func so that we don't anger powertop, and drop the forcewake again
  quicker.
  
  v2: Enable the deferred force_wake_put for regular register reads as
  well.
 
 So I went to test this patch today. Oddly this isn't showing up in any
 of my profiles on the latest drm-intel-nightly. I just tested today, and
 Daniel made it a 3.14-rc4 based thing today. It was still present on
 3.13.0. Actually, i915_gem_retire_work_handler is the only one even
 showing up in powertop, which is how it should be IMO.
 
 Therefore I'm not sure what we should do with this.

From one perspective, that sounds promising - if it doesn't cause
powertop to start a panic. This patch is what I wanted for the
forcewake-off delay in the first place, so it is worth
investigating as it is more likely to have an effect. There are still
the regular dropped mmio writes being reported in downstream bugzillas.
:|
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Always use kref tracking for contexts.

2014-03-02 Thread Chris Wilson
On Sun, Mar 02, 2014 at 03:58:09PM -0800, Ben Widawsky wrote:
 On Fri, Feb 28, 2014 at 08:06:50PM +, Chris Wilson wrote:
  ctx-obj = i915_gem_object_create_stolen(dev, 
  dev_priv-hw_context_size);
 How this working for you ^ ?

Fine. It helps detect cases where we try to load uninitialised contexts,
but other than that I have not noticed any difference.

  +   if (HAS_HW_CONTEXTS(dev)) {
  +   idr_for_each(file_priv-context_idr, context_idr_cleanup, 
  NULL);
  +   idr_destroy(file_priv-context_idr);
  }
   
  -   idr_for_each(file_priv-context_idr, context_idr_cleanup, NULL);
  i915_gem_context_unreference(file_priv-private_default_ctx);
  -   idr_destroy(file_priv-context_idr);
   }
 
 I don't see too much harm in just initializing the idr regardless to
 keep the close path simpler. Then stick a WARN in context_idr_cleanup if
 it actually does anything, fake_context_idr_cleanup();

Agreed I didn't see any harm in making that change as well. I was just
trying to keep the set of changes small, and targetted towards removing
that ctx-obj dereference.
 
 I have some recollection of hitting a really tricky thing while
 developing PPGTT where the order of the unref in the above sequence
 really mattered. Looking again though, I don't see anything familiar
 - but my suggestion would put me completely at ease.

Sure, but I think you've squashed that bug already. :)
 
 In either case:
 Reviewed-by: Ben Widawsky b...@bwidawsk.net

Ok, let's go with a second patch to converge the fake context with the
hw context later.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx