[Intel-gfx] [PATCH 3/9] drm/i915: Use plane->state->fb instead of plane->fb in intel_plane_restore()

[ville . syrjala]

From: Ville Syrjälä 

plane->fb is not as reliable as plane->state->fb so let's convert
intel_plane_restore() over the the new way of thinking as well.

Cc: Matt Roper 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_sprite.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 7051da7..a828736 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1361,10 +1361,10 @@ out_unlock:
 
 int intel_plane_restore(struct drm_plane *plane)
 {
-   if (!plane->crtc || !plane->fb)
+   if (!plane->crtc || !plane->state->fb)
return 0;
 
-   return plane->funcs->update_plane(plane, plane->crtc, plane->fb,
+   return plane->funcs->update_plane(plane, plane->crtc, plane->state->fb,
  plane->state->crtc_x, plane->state->crtc_y,
  plane->state->crtc_w, plane->state->crtc_h,
  plane->state->src_x, plane->state->src_y,
-- 
2.0.5

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

[Intel-gfx] [PATCH 4/9] drm/i915: Make derived plane state correct after crtc_enable

[ville . syrjala]

From: Ville Syrjälä 

We need to call the plane .atomic_check() hook when enabling the crtc
to make sure all the derived state (eg. visible, clipped src/dst
coordinates) are up to date when we enable the plane. This allows us to
trust this derived state everywhere.

We also take the opportunity to rewrite the plane enable sequence to
perform it as a single atomic update, which is a bit closer to where we
want to end up. Obviously this is a bit of a hack as we can't deal with
errors from .atomic_check(), so we just paper over that by making sure
visible=false so that we don't try to enable the plane with potentially
garbage coordinates and whatnot.

We also abuse the atomic code a bit by not making another copy of the
plane state and just frobbing directly with the plane->state.

Cc: Matt Roper 
Cc: Sonika Jindal 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_drv.h  |   3 +
 drivers/gpu/drm/i915/intel_display.c | 222 ++-
 2 files changed, 88 insertions(+), 137 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5462cbf..b16c0a7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -238,6 +238,9 @@ enum hpd_pin {
 #define for_each_intel_crtc(dev, intel_crtc) \
list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list, base.head)
 
+#define for_each_intel_plane(dev, intel_plane) \
+   list_for_each_entry(intel_plane, &dev->mode_config.plane_list, 
base.head)
+
 #define for_each_intel_encoder(dev, intel_encoder) \
list_for_each_entry(intel_encoder,  \
&(dev)->mode_config.encoder_list,   \
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 0da3abf..fdc83f1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2124,66 +2124,6 @@ void intel_flush_primary_plane(struct drm_i915_private 
*dev_priv,
POSTING_READ(reg);
 }
 
-/**
- * intel_enable_primary_hw_plane - enable the primary plane on a given pipe
- * @plane:  plane to be enabled
- * @crtc: crtc for the plane
- *
- * Enable @plane on @crtc, making sure that the pipe is running first.
- */
-static void intel_enable_primary_hw_plane(struct drm_plane *plane,
- struct drm_crtc *crtc)
-{
-   struct drm_device *dev = plane->dev;
-   struct drm_i915_private *dev_priv = dev->dev_private;
-   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-   /* If the pipe isn't enabled, we can't pump pixels and may hang */
-   assert_pipe_enabled(dev_priv, intel_crtc->pipe);
-
-   if (intel_crtc->primary_enabled)
-   return;
-
-   intel_crtc->primary_enabled = true;
-
-   dev_priv->display.update_primary_plane(crtc, plane->fb,
-  crtc->x, crtc->y);
-
-   /*
-* BDW signals flip done immediately if the plane
-* is disabled, even if the plane enable is already
-* armed to occur at the next vblank :(
-*/
-   if (IS_BROADWELL(dev))
-   intel_wait_for_vblank(dev, intel_crtc->pipe);
-}
-
-/**
- * intel_disable_primary_hw_plane - disable the primary hardware plane
- * @plane: plane to be disabled
- * @crtc: crtc for the plane
- *
- * Disable @plane on @crtc, making sure that the pipe is running first.
- */
-static void intel_disable_primary_hw_plane(struct drm_plane *plane,
-  struct drm_crtc *crtc)
-{
-   struct drm_device *dev = plane->dev;
-   struct drm_i915_private *dev_priv = dev->dev_private;
-   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-   if (WARN_ON(!intel_crtc->active))
-   return;
-
-   if (!intel_crtc->primary_enabled)
-   return;
-
-   intel_crtc->primary_enabled = false;
-
-   dev_priv->display.update_primary_plane(crtc, plane->fb,
-  crtc->x, crtc->y);
-}
-
 static bool need_vtd_wa(struct drm_device *dev)
 {
 #ifdef CONFIG_INTEL_IOMMU
@@ -2895,7 +2835,10 @@ static void skylake_update_primary_plane(struct drm_crtc 
*crtc,
POSTING_READ(PLANE_SURF(pipe, 0));
 }
 
-/* Assume fb object is pinned & idle & fenced and just update base pointers */
+/*
+ * Assume fb object is big enough & pinned & idle & fenced,
+ * and just update base pointers
+ */
 static int
 intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
   int x, int y, enum mode_set_atomic state)
@@ -2906,6 +2849,7 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct 
drm_framebuffer *fb,
if (dev_priv->display.disable_fbc)
dev_priv->display.disable_fbc(dev);
 
+   to_intel_crtc(crtc)->primary_enabled = true;
dev_priv->display.update_primary_plane(crtc, fb, x, y);
 
return 0;
@@ -2933,

[Intel-gfx] [PATCH 2/9] drm/i915: Reduce clutter by using the local plane pointer

[ville . syrjala]

From: Ville Syrjälä 

No need to go dig throguh intel_crtc->base.cursor when we already have
the same thing as 'plane' local variable.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 1d5107e..0da3abf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12299,7 +12299,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
 
 finish:
if (intel_crtc->active) {
-   if (intel_crtc->base.cursor->state->crtc_w != 
state->base.crtc_w)
+   if (plane->state->crtc_w != state->base.crtc_w)
intel_crtc->atomic.update_wm = true;
 
intel_crtc->atomic.fb_bits |=
-- 
2.0.5

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

[Intel-gfx] [PATCH 1/9] drm/i915: Remove debug prints from primary plane update funcs

[ville . syrjala]

From: Ville Syrjälä 

These are now called from the plane commit hooks, so they really need to
be fast or else we risk atomic update failures. So kill the debug prints
which are slowing things down massively.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_display.c | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index c3a5888..1d5107e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2658,9 +2658,6 @@ static void i9xx_update_primary_plane(struct drm_crtc 
*crtc,
 
I915_WRITE(reg, dspcntr);
 
-   DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
- i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
- fb->pitches[0]);
I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
if (INTEL_INFO(dev)->gen >= 4) {
I915_WRITE(DSPSURF(plane),
@@ -2762,9 +2759,6 @@ static void ironlake_update_primary_plane(struct drm_crtc 
*crtc,
 
I915_WRITE(reg, dspcntr);
 
-   DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
- i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
- fb->pitches[0]);
I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
I915_WRITE(DSPSURF(plane),
   i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset);
@@ -2890,11 +2884,6 @@ static void skylake_update_primary_plane(struct drm_crtc 
*crtc,
 
I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
 
-   DRM_DEBUG_KMS("Writing base %08lX %d,%d,%d,%d pitch=%d\n",
- i915_gem_obj_ggtt_offset(obj),
- x, y, fb->width, fb->height,
- fb->pitches[0]);
-
I915_WRITE(PLANE_POS(pipe, 0), 0);
I915_WRITE(PLANE_OFFSET(pipe, 0), (y << 16) | x);
I915_WRITE(PLANE_SIZE(pipe, 0),
-- 
2.0.5

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

[Intel-gfx] [PATCH 6/9] drm/i915: Pass the primary plane position to .update_primary_plane()

[ville . syrjala]

From: Ville Syrjälä 

In preparation for changing the primary plane position pass the clipped
position to .update_primary_plane().

Cc: Sonika Jindal 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_display.c | 20 +++-
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e99eef0..8d8b9a1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -579,6 +579,7 @@ struct drm_i915_display_funcs {
void (*update_primary_plane)(struct drm_crtc *crtc,
 struct drm_framebuffer *fb,
 int x, int y,
+int crtc_x, int crtc_y,
 int crtc_w, int crtc_h);
void (*hpd_irq_setup)(struct drm_device *dev);
/* clock updates for mode set */
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 1a789f0..33680a8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2483,6 +2483,7 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
 static void i9xx_update_primary_plane(struct drm_crtc *crtc,
  struct drm_framebuffer *fb,
  int x, int y,
+ int crtc_x, int crtc_y,
  int crtc_w, int crtc_h)
 {
struct drm_device *dev = crtc->dev;
@@ -2519,13 +2520,14 @@ static void i9xx_update_primary_plane(struct drm_crtc 
*crtc,
if (intel_crtc->pipe == PIPE_B)
dspcntr |= DISPPLANE_SEL_PIPE_B;
I915_WRITE(DSPSIZE(plane), ((crtc_h - 1) << 16) | (crtc_w - 1));
-   I915_WRITE(DSPPOS(plane), 0);
+   I915_WRITE(DSPPOS(plane), (crtc_y << 16) | crtc_x);
} else if (IS_CHERRYVIEW(dev) && plane == PLANE_B) {
I915_WRITE(PRIMSIZE(plane), ((crtc_h - 1) << 16) | (crtc_w - 
1));
-   I915_WRITE(PRIMPOS(plane), 0);
+   I915_WRITE(PRIMPOS(plane), (crtc_y << 16) | crtc_x);
I915_WRITE(PRIMCNSTALPHA(plane), 0);
} else {
-   WARN_ONCE(crtc_w != intel_crtc->config->pipe_src_w ||
+   WARN_ONCE(crtc_x != 0 || crtc_y != 0 ||
+ crtc_w != intel_crtc->config->pipe_src_w ||
  crtc_h != intel_crtc->config->pipe_src_h,
  "primary plane size doesn't match pipe size\n");
}
@@ -2609,6 +2611,7 @@ static void i9xx_update_primary_plane(struct drm_crtc 
*crtc,
 static void ironlake_update_primary_plane(struct drm_crtc *crtc,
  struct drm_framebuffer *fb,
  int x, int y,
+ int crtc_x, int crtc_y,
  int crtc_w, int crtc_h)
 {
struct drm_device *dev = crtc->dev;
@@ -2641,7 +2644,8 @@ static void ironlake_update_primary_plane(struct drm_crtc 
*crtc,
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
 
-   WARN_ONCE(crtc_w != intel_crtc->config->pipe_src_w ||
+   WARN_ONCE(crtc_x != 0 || crtc_y != 0 ||
+ crtc_w != intel_crtc->config->pipe_src_w ||
  crtc_h != intel_crtc->config->pipe_src_h,
  "primary plane size doesn't match pipe size\n");
 
@@ -2750,6 +2754,7 @@ u32 intel_fb_stride_alignment(struct drm_device *dev, 
uint64_t fb_modifier,
 static void skylake_update_primary_plane(struct drm_crtc *crtc,
 struct drm_framebuffer *fb,
 int x, int y,
+int crtc_x, int crtc_y,
 int crtc_w, int crtc_h)
 {
struct drm_device *dev = crtc->dev;
@@ -2827,7 +2832,7 @@ static void skylake_update_primary_plane(struct drm_crtc 
*crtc,
 
I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
 
-   I915_WRITE(PLANE_POS(pipe, 0), 0);
+   I915_WRITE(PLANE_POS(pipe, 0), (crtc_y << 16) | crtc_x);
I915_WRITE(PLANE_OFFSET(pipe, 0), (y << 16) | x);
I915_WRITE(PLANE_SIZE(pipe, 0), ((crtc_h - 1) << 16) | (crtc_w - 1));
I915_WRITE(PLANE_STRIDE(pipe, 0), fb->pitches[0] / stride_div);
@@ -2854,6 +2859,7 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct 
drm_framebuffer *fb,
/* FIXME: this will go badly if the fb isn't big enough */
to_intel_crtc(crtc)->primary_enabled = true;
dev_priv->display.update_primary_plane(crtc, fb, x, y,
+  0, 0,
   intel_crtc->config->pipe_src_w,

[Intel-gfx] [PATCH 5/9] drm/i915: Pass primary plane size to .update_primary_plane()

[ville . syrjala]

From: Ville Syrjälä 

In preparation to movable/resizable primary planes pass the clipped
plane size to .update_primary_plane().

Cc: Sonika Jindal 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_drv.h  |  3 +-
 drivers/gpu/drm/i915/intel_display.c | 63 
 2 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b16c0a7..e99eef0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -578,7 +578,8 @@ struct drm_i915_display_funcs {
  uint32_t flags);
void (*update_primary_plane)(struct drm_crtc *crtc,
 struct drm_framebuffer *fb,
-int x, int y);
+int x, int y,
+int crtc_w, int crtc_h);
void (*hpd_irq_setup)(struct drm_device *dev);
/* clock updates for mode set */
/* cursor updates */
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index fdc83f1..1a789f0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2482,7 +2482,8 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
 
 static void i9xx_update_primary_plane(struct drm_crtc *crtc,
  struct drm_framebuffer *fb,
- int x, int y)
+ int x, int y,
+ int crtc_w, int crtc_h)
 {
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2517,20 +2518,16 @@ static void i9xx_update_primary_plane(struct drm_crtc 
*crtc,
if (INTEL_INFO(dev)->gen < 4) {
if (intel_crtc->pipe == PIPE_B)
dspcntr |= DISPPLANE_SEL_PIPE_B;
-
-   /* pipesrc and dspsize control the size that is scaled from,
-* which should always be the user's requested size.
-*/
-   I915_WRITE(DSPSIZE(plane),
-  ((intel_crtc->config->pipe_src_h - 1) << 16) |
-  (intel_crtc->config->pipe_src_w - 1));
+   I915_WRITE(DSPSIZE(plane), ((crtc_h - 1) << 16) | (crtc_w - 1));
I915_WRITE(DSPPOS(plane), 0);
} else if (IS_CHERRYVIEW(dev) && plane == PLANE_B) {
-   I915_WRITE(PRIMSIZE(plane),
-  ((intel_crtc->config->pipe_src_h - 1) << 16) |
-  (intel_crtc->config->pipe_src_w - 1));
+   I915_WRITE(PRIMSIZE(plane), ((crtc_h - 1) << 16) | (crtc_w - 
1));
I915_WRITE(PRIMPOS(plane), 0);
I915_WRITE(PRIMCNSTALPHA(plane), 0);
+   } else {
+   WARN_ONCE(crtc_w != intel_crtc->config->pipe_src_w ||
+ crtc_h != intel_crtc->config->pipe_src_h,
+ "primary plane size doesn't match pipe size\n");
}
 
switch (fb->pixel_format) {
@@ -2586,14 +2583,14 @@ static void i9xx_update_primary_plane(struct drm_crtc 
*crtc,
if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180)) {
dspcntr |= DISPPLANE_ROTATE_180;
 
-   x += (intel_crtc->config->pipe_src_w - 1);
-   y += (intel_crtc->config->pipe_src_h - 1);
+   x += (crtc_w - 1);
+   y += (crtc_h - 1);
 
/* Finding the last pixel of the last line of the display
data and adding to linear_offset*/
linear_offset +=
-   (intel_crtc->config->pipe_src_h - 1) * fb->pitches[0] +
-   (intel_crtc->config->pipe_src_w - 1) * pixel_size;
+   (crtc_h - 1) * fb->pitches[0] +
+   (crtc_w - 1) * pixel_size;
}
 
I915_WRITE(reg, dspcntr);
@@ -2611,7 +2608,8 @@ static void i9xx_update_primary_plane(struct drm_crtc 
*crtc,
 
 static void ironlake_update_primary_plane(struct drm_crtc *crtc,
  struct drm_framebuffer *fb,
- int x, int y)
+ int x, int y,
+ int crtc_w, int crtc_h)
 {
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2643,6 +2641,10 @@ static void ironlake_update_primary_plane(struct 
drm_crtc *crtc,
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
 
+   WARN_ONCE(crtc_w != intel_crtc->config->pipe_src_w ||
+ crtc_h != intel_crtc->config->pipe_src_h,
+ "primary plane size doesn't match pipe size\n");
+
switch (fb->pixel_format) {
case DRM_FORMAT_C8:
dspcn

[Intel-gfx] [PATCH 8/9] drm/i915: Use state->visible in wm calculation

[ville . syrjala]

From: Ville Syrjälä 

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_pm.c | 24 +---
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a06a2c7..499e054 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1955,6 +1955,7 @@ static void ilk_compute_wm_parameters(struct drm_crtc 
*crtc,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
enum pipe pipe = intel_crtc->pipe;
struct drm_plane *plane;
+   const struct intel_plane_state *state;
 
if (!intel_crtc->active)
return;
@@ -1962,13 +1963,22 @@ static void ilk_compute_wm_parameters(struct drm_crtc 
*crtc,
p->active = true;
p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
-   p->pri.bytes_per_pixel = crtc->primary->state->fb->bits_per_pixel / 8;
-   p->cur.bytes_per_pixel = 4;
-   p->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
-   p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;
-   /* TODO: for now, assume primary and cursor planes are always enabled. 
*/
-   p->pri.enabled = true;
-   p->cur.enabled = true;
+
+   state = to_intel_plane_state(crtc->primary->state);
+   if (state->visible) {
+   p->pri.enabled = true;
+   p->pri.bytes_per_pixel =
+   drm_format_plane_cpp(state->base.fb->pixel_format, 0);
+   p->pri.horiz_pixels = drm_rect_width(&state->dst);
+   }
+
+   state = to_intel_plane_state(crtc->cursor->state);
+   if (state->visible) {
+   p->cur.enabled = true;
+   p->cur.bytes_per_pixel =
+   drm_format_plane_cpp(state->base.fb->pixel_format, 0);
+   p->cur.horiz_pixels = state->base.crtc_w;
+   }
 
drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
struct intel_plane *intel_plane = to_intel_plane(plane);
-- 
2.0.5

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

[Intel-gfx] [PATCH 0/9] drm/i915: Update derived plane state at crtc enable/disable

[ville . syrjala]

From: Ville Syrjälä 

Here's a quick series that tries to make our .crtc_{enable,disable}()
update the derived plane state correctly. The upside of doing this is
that we can actually trust the 'visible' and the clipped src/dst
coordinates for all planes.

I also took the opportunity to make the plane enable/disable happen
atomically when the crtc is enabled/disabled.

And now that the derived plane state is correct I added a few patchs to
use that information in the wm code, and also to prepare for primary plane
windowing.

Ville Syrjälä (9):
  drm/i915: Remove debug prints from primary plane update funcs
  drm/i915: Reduce clutter by using the local plane pointer
  drm/i915: Use plane->state->fb instead of plane->fb in
intel_plane_restore()
  drm/i915: Make derived plane state correct after crtc_enable
  drm/i915: Pass primary plane size to .update_primary_plane()
  drm/i915: Pass the primary plane position to .update_primary_plane()
  drm/i915: Update watermarks after the derived plane state is uptodate
  drm/i915: Use state->visible in wm calculation
  drm/i915: Don't re-enable an explicitly disabled primary plane due to
sprite coverage changes

 drivers/gpu/drm/i915/i915_drv.h  |   7 +-
 drivers/gpu/drm/i915/intel_display.c | 310 +++
 drivers/gpu/drm/i915/intel_pm.c  |  24 ++-
 drivers/gpu/drm/i915/intel_sprite.c  |   8 +-
 4 files changed, 163 insertions(+), 186 deletions(-)

-- 
2.0.5

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

[Intel-gfx] [PATCH 7/9] drm/i915: Update watermarks after the derived plane state is uptodate

[ville . syrjala]

From: Ville Syrjälä 

When enabling planes during .crtc_enable() we currently want to update
the watermarks before enabling the planes. We already do it once just
before enabling the pipe, but at that point out derived plane state is
still out of whack, so we need to do it again after the .atomic_check()
hooks have been called.

What this means is now we could actually start to trust the derived
plane state (clipped size, 'visible', etc.) in the watermark code.

The pre pipe enable watermark update is supposed to be just make sure
the other pipes are ready to have their FIFOs potentially reduced, so we
need to keep it there as well.

Since we don't yet have proper two-part watermark update leave the
watermakrs alone in the plane disable case. This way they'll get updated
only after the planes and pipe have all been turned off.

Cc: Matt Roper 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_display.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 33680a8..a9201a7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4167,6 +4167,8 @@ static void _intel_crtc_enable_planes(struct intel_crtc 
*crtc)
state->visible = false;
}
 
+   intel_update_watermarks(&crtc->base);
+
crtc_funcs->atomic_begin(&crtc->base);
 
for_each_intel_plane(dev, plane) {
@@ -4213,6 +4215,8 @@ static void _intel_crtc_disable_planes(struct intel_crtc 
*crtc)
}
 
crtc_funcs->atomic_flush(&crtc->base);
+
+   /* we'll defer watermark update to after the pipe has been disabled */
 }
 
 void hsw_enable_ips(struct intel_crtc *crtc)
-- 
2.0.5

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

[Intel-gfx] [PATCH 9/9] drm/i915: Don't re-enable an explicitly disabled primary plane due to sprite coverage changes

[ville . syrjala]

From: Ville Syrjälä 

When the sprite is covering the entire pipe (and color keying is not
enabled) we currently try to automagically disable the primary plane
which is fully covered by the sprite.

Now that .crtc_disable() will simply disable planes by setting the
state->visible to false, the sprite code will actually end up
re-enabling the primary after it got disabled, and then we proceed to
turn off the pipe and are greeted with some warnings from
assert_plane_disabled().

The code doing the automagic disable of covered planes needs to
rewritten to do things correctly as part of the atomic update (or simply
removed), but in the meantime add a a bit of duct tape and simply have
the sprite code check the primary plane's state->visible before
re-enabling it.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_sprite.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index a828736..7286497 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1287,7 +1287,9 @@ intel_commit_sprite_plane(struct drm_plane *plane,
intel_plane->obj = obj;
 
if (intel_crtc->active) {
-   intel_crtc->primary_enabled = !state->hides_primary;
+   intel_crtc->primary_enabled =
+   to_intel_plane_state(crtc->primary->state)->visible &&
+   !state->hides_primary;
 
if (state->visible) {
crtc_x = state->dst.x1;
-- 
2.0.5

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

Re: [Intel-gfx] [PATCH v6 05/32] drm/i915: Track GEN6 page table usage

[Mika Kuoppala]

Mika Kuoppala  writes:

> Michel Thierry  writes:
>
>> From: Ben Widawsky 
>>
>> Instead of implementing the full tracking + dynamic allocation, this
>> patch does a bit less than half of the work, by tracking and warning on
>> unexpected conditions. The tracking itself follows which PTEs within a
>> page table are currently being used for objects. The next patch will
>> modify this to actually allocate the page tables only when necessary.
>>
>> With the current patch there isn't much in the way of making a gen
>> agnostic range allocation function. However, in the next patch we'll add
>> more specificity which makes having separate functions a bit easier to
>> manage.
>>
>> One important change introduced here is that DMA mappings are
>> created/destroyed at the same page directories/tables are
>> allocated/deallocated.
>>
>> Notice that aliasing PPGTT is not managed here. The patch which actually
>> begins dynamic allocation/teardown explains the reasoning for this.
>>
>> v2: s/pdp.page_directory/pdp.page_directorys
>> Make a scratch page allocation helper
>>
>> v3: Rebase and expand commit message.
>>
>> v4: Allocate required pagetables only when it is needed, _bind_to_vm
>> instead of bind_vma (Daniel).
>>
>> v5: Rebased to remove the unnecessary noise in the diff, also:
>>  - PDE mask is GEN agnostic, renamed GEN6_PDE_MASK to I915_PDE_MASK.
>>  - Removed unnecessary checks in gen6_alloc_va_range.
>>  - Changed map/unmap_px_single macros to use dma functions directly and
>>be part of a static inline function instead.
>>  - Moved drm_device plumbing through page tables operation to its own
>>patch.
>>  - Moved allocate/teardown_va_range calls until they are fully
>>implemented (in subsequent patch).
>>  - Merged pt and scratch_pt unmap_and_free path.
>>  - Moved scratch page allocator helper to the patch that will use it.
>>
>> v6: Reduce complexity by not tearing down pagetables dynamically, the
>> same can be achieved while freeing empty vms. (Daniel)
>>
>> v7: s/i915_dma_map_px_single/i915_dma_map_single
>> s/gen6_write_pdes/gen6_write_pde
>> Prevent a NULL case when only GGTT is available. (Mika)
>>
>> v8: Rebased after s/page_tables/page_table/.
>>
>> Cc: Daniel Vetter 
>> Cc: Mika Kuoppala 
>> Signed-off-by: Ben Widawsky 
>> Signed-off-by: Michel Thierry  (v3+)
>> ---
>>  drivers/gpu/drm/i915/i915_gem_gtt.c | 198 
>> +---
>>  drivers/gpu/drm/i915/i915_gem_gtt.h |  75 ++
>>  2 files changed, 211 insertions(+), 62 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index e05488e..f9354c7 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -278,29 +278,88 @@ static gen6_gtt_pte_t iris_pte_encode(dma_addr_t addr,
>>  return pte;
>>  }
>>  
>> -static void unmap_and_free_pt(struct i915_page_table_entry *pt, struct 
>> drm_device *dev)
>> +#define i915_dma_unmap_single(px, dev) \
>> +__i915_dma_unmap_single((px)->daddr, dev)
>> +
>> +static inline void __i915_dma_unmap_single(dma_addr_t daddr,
>> +struct drm_device *dev)
>> +{
>> +struct device *device = &dev->pdev->dev;
>> +
>> +dma_unmap_page(device, daddr, 4096, PCI_DMA_BIDIRECTIONAL);
>> +}
>> +
>> +/**
>> + * i915_dma_map_single() - Create a dma mapping for a page table/dir/etc.
>> + * @px: Page table/dir/etc to get a DMA map for
>> + * @dev:drm device
>> + *
>> + * Page table allocations are unified across all gens. They always require a
>> + * single 4k allocation, as well as a DMA mapping. If we keep the structs
>> + * symmetric here, the simple macro covers us for every page table type.
>> + *
>> + * Return: 0 if success.
>> + */
>> +#define i915_dma_map_single(px, dev) \
>> +i915_dma_map_page_single((px)->page, (dev), &(px)->daddr)
>> +
>> +static inline int i915_dma_map_page_single(struct page *page,
>> +   struct drm_device *dev,
>> +   dma_addr_t *daddr)
>> +{
>> +struct device *device = &dev->pdev->dev;
>> +
>> +*daddr = dma_map_page(device, page, 0, 4096, PCI_DMA_BIDIRECTIONAL);
>> +return dma_mapping_error(device, *daddr);
>> +}
>> +
>> +static void unmap_and_free_pt(struct i915_page_table_entry *pt,
>> +   struct drm_device *dev)
>>  {
>>  if (WARN_ON(!pt->page))
>>  return;
>> +
>> +i915_dma_unmap_single(pt, dev);
>>  __free_page(pt->page);
>> +kfree(pt->used_ptes);
>>  kfree(pt);
>>  }
>>  
>>  static struct i915_page_table_entry *alloc_pt_single(struct drm_device *dev)
>>  {
>>  struct i915_page_table_entry *pt;
>> +const size_t count = INTEL_INFO(dev)->gen >= 8 ?
>> +GEN8_PTES_PER_PAGE : I915_PPGTT_PT_ENTRIES;
>> +int ret = -ENOMEM;
>>  
>>  pt = kzalloc(sizeof(*pt), GFP_KERNEL);
>>  if (!pt)
>>  return ERR_PTR(-ENOMEM);
>> 

Re: [Intel-gfx] [PATCH v2 09/12] drm/i915: Rewrite VLV/CHV watermark code

[Ville Syrjälä]

On Tue, Mar 10, 2015 at 11:26:29AM +0100, Daniel Vetter wrote:
> On Thu, Mar 05, 2015 at 09:19:49PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > Assuming the PND deadline mechanism works reasonably we should do
> > memory requests as early as possible so that PND has schedule the
> > requests more intelligently. Currently we're still calculating
> > the watermarks as if VLV/CHV are identical to g4x, which isn't
> > the case.
> > 
> > The current code also seems to calculate insufficient watermarks
> > and hence we're seeing some underruns, especially on high resolution
> > displays.
> > 
> > To fix it just rip out the current code and replace is with something
> > that tries to utilize PND as efficiently as possible.
> > 
> > We now calculate the WM watermark to trigger when the FIFO still has
> > 256us worth of data. 256us is the maximum deadline value supoorted by
> > PND, so issuing memory requests earlier would mean we probably couldn't
> > utilize the full FIFO as PND would attempt to return the data at
> > least in at least 256us. We also clamp the watermark to at least 8
> > cachelines as that's the magic watermark that enabling trickle feed
> > would also impose. I'm assuming it matches some burst size.
> > 
> > In theory we could just enable trickle feed and ignore the WM values,
> > except trickle feed doesn't work with max fifo mode anyway, so we'd
> > still need to calculate the SR watermarks. It seems cleaner to just
> > disable trickle feed and calculate all watermarks the same way. Also
> > trickle feed wouldn't account for the 256us max deadline value, thoguh
> > that may be a moot point in non-max fifo mode sicne the FIFOs are fairly
> > small.
> > 
> > On VLV max fifo mode can be used with either primary or sprite planes.
> > So the code now also checks all the planes (apart from the cursor)
> > when calculating the SR plane watermark.
> > 
> > We don't have to worry about the WM1 watermarks since we're using the
> > PND deadline scheme which means the hardware ignores WM1 values.
> > 
> > v2: Use plane->state->fb instead of plane->fb
> > 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h |  11 ++
> >  drivers/gpu/drm/i915/i915_reg.h |   4 +-
> >  drivers/gpu/drm/i915/intel_pm.c | 330 
> > +---
> >  3 files changed, 188 insertions(+), 157 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 5de69a0..b191b12 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1516,6 +1516,17 @@ struct ilk_wm_values {
> >  
> >  struct vlv_wm_values {
> > struct {
> > +   uint16_t primary;
> > +   uint16_t sprite[2];
> > +   uint8_t cursor;
> > +   } pipe[3];
> > +
> > +   struct {
> > +   uint16_t plane;
> > +   uint8_t cursor;
> > +   } sr;
> > +
> > +   struct {
> > uint8_t cursor;
> > uint8_t sprite[2];
> > uint8_t primary;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 8178610..9f98384 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4127,7 +4127,7 @@ enum skl_disp_power_wells {
> >  /* vlv/chv high order bits */
> >  #define DSPHOWM(VLV_DISPLAY_BASE + 0x70064)
> >  #define   DSPFW_SR_HI_SHIFT24
> > -#define   DSPFW_SR_HI_MASK (1<<24)
> > +#define   DSPFW_SR_HI_MASK (3<<24) /* 2 bits for chv, 1 for vlv */
> >  #define   DSPFW_SPRITEF_HI_SHIFT   23
> >  #define   DSPFW_SPRITEF_HI_MASK(1<<23)
> >  #define   DSPFW_SPRITEE_HI_SHIFT   22
> > @@ -4148,7 +4148,7 @@ enum skl_disp_power_wells {
> >  #define   DSPFW_PLANEA_HI_MASK (1<<0)
> >  #define DSPHOWM1   (VLV_DISPLAY_BASE + 0x70068)
> >  #define   DSPFW_SR_WM1_HI_SHIFT24
> > -#define   DSPFW_SR_WM1_HI_MASK (1<<24)
> > +#define   DSPFW_SR_WM1_HI_MASK (3<<24) /* 2 bits for chv, 1 
> > for vlv */
> >  #define   DSPFW_SPRITEF_WM1_HI_SHIFT   23
> >  #define   DSPFW_SPRITEF_WM1_HI_MASK(1<<23)
> >  #define   DSPFW_SPRITEE_WM1_HI_SHIFT   22
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c 
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index bdb0f5d..497847c 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -778,6 +778,55 @@ static void vlv_write_wm_values(struct intel_crtc 
> > *crtc,
> >(wm->ddl[pipe].sprite[0] << DDL_SPRITE_SHIFT(0)) |
> >(wm->ddl[pipe].primary << DDL_PLANE_SHIFT));
> >  
> > +   I915_WRITE(DSPFW1,
> > +  ((wm->sr.plane << DSPFW_SR_SHIFT) & DSPFW_SR_MASK) |
> > +  ((wm->pipe[PIPE_B].cursor << DSPFW_CURSORB_SHIFT) & 
> > DSPFW_CURSORB_MASK) |
> > +  ((wm->pipe[PIPE_B].primary << DSPFW_PLANEB_SHIFT) & 
> > DSPFW_PLANEB_MA

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for flips

[Tvrtko Ursulin]

On 02/16/2015 02:31 PM, Chris Wilson wrote:

intel_user_framebuffer_destroy() requires the struct_mutex for its
object bookkeeping, so this means that all calls to
drm_framebuffer_reference must be held without that lock.


Maybe "drm_framebuffer_unreference must be made without that lock", hm, 
actually "requires the struct_mutex" is misleading, should be "will 
grab/take struct_mutex".


But code itself:

Reviewed-by: Tvrtko Ursulin 

Regards,

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover

[Tvrtko Ursulin]

Hi,

On 02/16/2015 02:31 PM, Chris Wilson wrote:

intel_user_framebuffer_destroy() requires the struct_mutex for its
object bookkeeping, so this means that all calls to
drm_framebuffer_reference must be held without that lock.


Same comment on the commit message as 1/2.


References: https://bugs.freedesktop.org/show_bug.cgi?id=89166
Signed-off-by: Chris Wilson 
---
  drivers/gpu/drm/i915/intel_display.c | 13 ++---
  1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 6e1da7da5cca..aba36662d511 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13672,6 +13672,8 @@ void intel_modeset_gem_init(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *c;
struct drm_i915_gem_object *obj;
+   struct drm_plane *unused[I915_MAX_PIPES];
+   int n_unused = 0;

mutex_lock(&dev->struct_mutex);
intel_init_gt_powersave(dev);
@@ -13707,13 +13709,18 @@ void intel_modeset_gem_init(struct drm_device *dev)
   NULL)) {
DRM_ERROR("failed to pin boot fb on pipe %d\n",
  to_intel_crtc(c)->pipe);
-   drm_framebuffer_unreference(c->primary->fb);
-   c->primary->fb = NULL;
-   update_state_fb(c->primary);
+   unused[n_unused++] = c->primary;
}
}
mutex_unlock(&dev->struct_mutex);

+   while (n_unused--) {
+   struct drm_plane *p = unused[n_unused];
+   drm_framebuffer_unreference(p->fb);
+   p->fb = NULL;
+   update_state_fb(p);
+   }
+


For this one I am not sure. Should c->primary->fb = NULL remain under 
the locked loop? If not what is the the mutex protecting then?


Regards,

Tvrtko

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover

[Chris Wilson]

On Tue, Mar 10, 2015 at 12:02:28PM +, Tvrtko Ursulin wrote:
> >@@ -13707,13 +13709,18 @@ void intel_modeset_gem_init(struct drm_device *dev)
> >NULL)) {
> > DRM_ERROR("failed to pin boot fb on pipe %d\n",
> >   to_intel_crtc(c)->pipe);
> >-drm_framebuffer_unreference(c->primary->fb);
> >-c->primary->fb = NULL;
> >-update_state_fb(c->primary);
> >+unused[n_unused++] = c->primary;
> > }
> > }
> > mutex_unlock(&dev->struct_mutex);
> >
> >+while (n_unused--) {
> >+struct drm_plane *p = unused[n_unused];
> >+drm_framebuffer_unreference(p->fb);
> >+p->fb = NULL;
> >+update_state_fb(p);
> >+}
> >+
> 
> For this one I am not sure. Should c->primary->fb = NULL remain
> under the locked loop? If not what is the the mutex protecting then?

It's a dummy mutex that only exists to keep the WARNs quiet. This phase
of initialisation is explicitly single-threaded.
-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 2/2] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover

[Tvrtko Ursulin]

On 03/10/2015 12:19 PM, Chris Wilson wrote:

On Tue, Mar 10, 2015 at 12:02:28PM +, Tvrtko Ursulin wrote:

@@ -13707,13 +13709,18 @@ void intel_modeset_gem_init(struct drm_device *dev)
   NULL)) {
DRM_ERROR("failed to pin boot fb on pipe %d\n",
  to_intel_crtc(c)->pipe);
-   drm_framebuffer_unreference(c->primary->fb);
-   c->primary->fb = NULL;
-   update_state_fb(c->primary);
+   unused[n_unused++] = c->primary;
}
}
mutex_unlock(&dev->struct_mutex);

+   while (n_unused--) {
+   struct drm_plane *p = unused[n_unused];
+   drm_framebuffer_unreference(p->fb);
+   p->fb = NULL;
+   update_state_fb(p);
+   }
+


For this one I am not sure. Should c->primary->fb = NULL remain
under the locked loop? If not what is the the mutex protecting then?


It's a dummy mutex that only exists to keep the WARNs quiet. This phase
of initialisation is explicitly single-threaded.


Would it be a simpler fix then to move the mutex only around 
pin_and_fence_fb_obj?


Regards,

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

[Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept

[Ander Conselvan de Oliveira]

Remove the global modeset resource function that would disable the
bifurcation bit, and instead enable/disable it when enabling or
disabling the pch transcoder. The checks before the mode set should
ensure that the configuration is valid, so it should be safe to do it
so.

Signed-off-by: Ander Conselvan de Oliveira 

---

On Mon, 2015-03-09 at 17:21 +0100, Daniel Vetter wrote:
> With atomic we need to probably look at crtc_state->mode_changed. As an
> interim solution we have the same information available in the
> modeset_pipes bitmask. I think replacing the check for intel_crtc->active
> with !(modeset_pipes & (1dev;
+   struct intel_crtc *crtc =
+   to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
uint32_t reg, val;
 
/* FDI relies on the transcoder */
@@ -1980,6 +1984,9 @@ static void ironlake_disable_pch_transcoder(struct 
drm_i915_private *dev_priv,
val &= ~TRANS_CHICKEN2_TIMING_OVERRIDE;
I915_WRITE(reg, val);
}
+
+   if (IS_IVYBRIDGE(dev))
+   ivybridge_update_fdi_bc_bifurcation(crtc, false);
 }
 
 static void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv)
@@ -3153,32 +3160,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc *crtc)
return crtc->base.state->enable && crtc->config->has_pch_encoder;
 }
 
-static void ivb_modeset_global_resources(struct drm_device *dev)
-{
-   struct drm_i915_private *dev_priv = dev->dev_private;
-   struct intel_crtc *pipe_B_crtc =
-   to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
-   struct intel_crtc *pipe_C_crtc =
-   to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]);
-   uint32_t temp;
-
-   /*
-* When everything is off disable fdi C so that we could enable fdi B
-* with all lanes. Note that we don't care about enabled pipes without
-* an enabled pch encoder.
-*/
-   if (!pipe_has_enabled_pch(pipe_B_crtc) &&
-   !pipe_has_enabled_pch(pipe_C_crtc)) {
-   WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
-   WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
-
-   temp = I915_READ(SOUTH_CHICKEN1);
-   temp &= ~FDI_BC_BIFURCATION_SELECT;
-   DRM_DEBUG_KMS("disabling fdi C rx\n");
-   I915_WRITE(SOUTH_CHICKEN1, temp);
-   }
-}
-
 /* The FDI link training functions for ILK/Ibexpeak. */
 static void ironlake_fdi_link_train(struct drm_crtc *crtc)
 {
@@ -3834,41 +3815,44 @@ static void ironlake_pch_transcoder_set_timings(struct 
intel_crtc *crtc,
   I915_READ(VSYNCSHIFT(cpu_transcoder)));
 }
 
-static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
+static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev, bool enable)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t temp;
 
temp = I915_READ(SOUTH_CHICKEN1);
-   if (temp & FDI_BC_BIFURCATION_SELECT)
+   if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable)
return;
 
WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
 
-   temp |= FDI_BC_BIFURCATION_SELECT;
-   DRM_DEBUG_KMS("enabling fdi C rx\n");
+   temp &= ~FDI_BC_BIFURCATION_SELECT;
+   if (enable)
+   temp |= FDI_BC_BIFURCATION_SELECT;
+
+   DRM_DEBUG_KMS("%sabling fdi C rx\n", enable ? "en" : "dis");
I915_WRITE(SOUTH_CHICKEN1, temp);
POSTING_READ(SOUTH_CHICKEN1);
 }
 
-static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
+static void ivybridge_update_fdi_bc_bi

[Intel-gfx] [PATCH] tests: Add test for pipe B and C interactions in IVB

[Ander Conselvan de Oliveira]

This actually only works if the machine is setup properly. It requires
that:

- the hardware to have at least two connected outpus;
- the defatult mode on those outputs needs to be big enough;
- the attached monitors need to support 10 bpc.

This might generate spurius results if the connected output doesn't support
10bpc or a large enough mode.

---
I used this to test the changes that affect pipe B and C interactions on
IVB. I'm not really sure how to turn into a proper igt test though, because
of the requirements on the outputs in use.


---
 tests/Makefile.sources|   1 +
 tests/kms_pipe_b_c_dpms.c | 173 ++
 2 files changed, 174 insertions(+)
 create mode 100644 tests/kms_pipe_b_c_dpms.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 9ab0ff4..9e43a64 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -73,6 +73,7 @@ TESTS_progs_M = \
kms_nuclear \
kms_pipe_crc_basic \
kms_plane \
+   kms_pipe_b_c_dpms \
kms_psr_sink_crc \
kms_render \
kms_rotation_crc \
diff --git a/tests/kms_pipe_b_c_dpms.c b/tests/kms_pipe_b_c_dpms.c
new file mode 100644
index 000..a5ed761
--- /dev/null
+++ b/tests/kms_pipe_b_c_dpms.c
@@ -0,0 +1,173 @@
+/*
+ * Copyright © 2015 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.
+ *
+ * Authors:
+ *   Ander Conselvan de Oliveira 
+ */
+
+#include "drmtest.h"
+#include "igt_kms.h"
+#include "intel_chipset.h"
+
+typedef struct {
+   int drm_fd;
+   igt_display_t display;
+} data_t;
+
+static int
+set_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output, int bpc)
+{
+   igt_plane_t *primary;
+   drmModeModeInfo *mode;
+   struct igt_fb fb;
+   int fb_id;
+   uint32_t format;
+
+   if (bpc == 8)
+   format = DRM_FORMAT_XRGB;
+   else if (bpc == 10)
+   format = DRM_FORMAT_XRGB2101010;
+   else
+   igt_fail(1);
+
+   igt_output_set_pipe(output, pipe);
+
+   /* FIXME: we need a big mode */
+   mode = igt_output_get_mode(output);
+   primary = igt_output_get_plane(output, 0);
+
+   fb_id = igt_create_color_fb(data->drm_fd,
+   mode->hdisplay, mode->vdisplay,
+   format, I915_TILING_NONE,
+   1.0, 1.0, 1.0, &fb);
+   igt_assert(fb_id >= 0);
+
+   igt_plane_set_fb(primary, &fb);
+   return igt_display_try_commit2(&data->display, COMMIT_LEGACY);
+}
+
+static int
+set_big_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
+{
+   return set_mode_on_pipe(data, pipe, output, 10);
+}
+
+static int
+set_normal_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
+{
+   return set_mode_on_pipe(data, pipe, output, 8);
+}
+
+static void
+find_outputs(data_t *data, igt_output_t **output1, igt_output_t **output2)
+{
+   int count = 0;
+   igt_output_t *output;
+
+   *output1 = NULL;
+   *output2 = NULL;
+
+   for_each_connected_output(&data->display, output) {
+   if (!(*output1))
+   *output1 = output;
+   else if (!(*output2))
+   *output2 = output;
+
+   igt_output_set_pipe(output, PIPE_ANY);
+   count++;
+   }
+
+   igt_skip_on_f(count < 2, "Not enough connected outputs\n");
+}
+
+static void
+test_dpms(data_t *data)
+{
+   igt_output_t *output1, *output2;
+   int ret;
+
+   find_outputs(data, &output1, &output2);
+
+   igt_info("Pipe %s will use connector %s\n",
+kmstest_pipe_name(PIPE_B), igt_output_name(output1));
+   igt_info("Pipe %s will use connector %s\n",
+kmstest_pipe_name(PIPE_C), igt_output_name(output2));
+
+   ret = set_big_mode_on_pipe(data, PIPE_B, output1);

Re: [Intel-gfx] [PATCH v2] drm/i915: Prevent TLB error on first execution on SNB

[Daniel Vetter]

On Tue, Mar 10, 2015 at 10:35:41AM +, Chris Wilson wrote:
> On Tue, Mar 10, 2015 at 11:31:04AM +0100, Daniel Vetter wrote:
> > On Fri, Feb 13, 2015 at 02:35:59PM +, Chris Wilson wrote:
> > > Long ago I found that I was getting sporadic errors when booting SNB,
> > > with the symptom being that the first batch died with IPEHR != *ACTHD,
> > > typically caused by the TLB being invalid. These magically disappeared
> > > if I held the forcewake during the entire ring initialisation sequence.
> > > (It can probably be shortened to a short critical section, but the whole
> > > initialisation is full of register writes and so we would be taking and
> > > releasing forcewake almost continually, and so holding it over the
> > > entire sequence will probably be a net win!)
> > > 
> > > Note some of the kernels I encounted the issue already had the deferred
> > > forcewake release, so it is still relevant.
> > > 
> > > I know that there have been a few other reports with similar failure
> > > conditions on SNB, I think such as
> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=80913
> > > 
> > > v2: Wrap i915_gem_init_hw() with its own security blanket as we take
> > > that path following resume and reset.
> > > 
> > > Signed-off-by: Chris Wilson 
> > > ---
> > >  drivers/gpu/drm/i915/i915_gem.c | 18 --
> > >  1 file changed, 16 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> > > b/drivers/gpu/drm/i915/i915_gem.c
> > > index 8d15c8110962..08450922f373 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -4783,6 +4783,9 @@ i915_gem_init_hw(struct drm_device *dev)
> > >   if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
> > >   return -EIO;
> > >  
> > > + /* Double layer security blanket, see i915_gem_init() */
> > > + intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> > > +
> > >   if (dev_priv->ellc_size)
> > >   I915_WRITE(HSW_IDICR, I915_READ(HSW_IDICR) | IDIHASHMSK(0xf));
> > >  
> > > @@ -4815,7 +4818,7 @@ i915_gem_init_hw(struct drm_device *dev)
> > >   for_each_ring(ring, dev_priv, i) {
> > >   ret = ring->init_hw(ring);
> > >   if (ret)
> > > - return ret;
> > > + goto out;
> > >   }
> > >  
> > >   for (i = 0; i < NUM_L3_SLICES(dev); i++)
> > > @@ -4832,9 +4835,11 @@ i915_gem_init_hw(struct drm_device *dev)
> > >   DRM_ERROR("Context enable failed %d\n", ret);
> > >   i915_gem_cleanup_ringbuffer(dev);
> > >  
> > > - return ret;
> > > + goto out;
> > >   }
> > >  
> > > +out:
> > > + intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> > >   return ret;
> > >  }
> > >  
> > > @@ -4868,6 +4873,14 @@ int i915_gem_init(struct drm_device *dev)
> > >   dev_priv->gt.stop_ring = intel_logical_ring_stop;
> > >   }
> > >  
> > > + /* This is just a security blanket to placate dragons.
> > > +  * On some systems, we very sporadically observe that the first TLBs
> > > +  * used by the CS may be stale, despite us poking the TLB reset. If
> > > +  * we hold the forcewake during initialisation these problems
> > > +  * just magically go away.
> > > +  */
> > > + intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> > 
> > gem_init shouldn't ever touch the hw except through gem_init_hw. Do we
> > really need the double-layer here?
> 
> There are register accesses before, so yes since that's how I tested
> it...
> 
> > Also the forcewake hack in the ring
> > init code should now be redundant, too.
> 
> I am of the opinion that they still serve documentary value. Unless you
> have an assert_force_wake() handy.

Ok, count me convinced.

Reviewed-by: Daniel Vetter 

And I guess this is for Jani + cc: stable.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for flips

[Daniel Vetter]

On Tue, Mar 10, 2015 at 11:43:10AM +, Tvrtko Ursulin wrote:
> On 02/16/2015 02:31 PM, Chris Wilson wrote:
> >intel_user_framebuffer_destroy() requires the struct_mutex for its
> >object bookkeeping, so this means that all calls to
> >drm_framebuffer_reference must be held without that lock.
> 
> Maybe "drm_framebuffer_unreference must be made without that lock", hm,
> actually "requires the struct_mutex" is misleading, should be "will
> grab/take struct_mutex".

I've changed the commit message to "... must not hold that lock."
> 
> But code itself:
> 
> Reviewed-by: Tvrtko Ursulin 

Both merged, thanks.
-Daniel

> 
> Regards,
> 
> Tvrtko
> ___
> Intel-gfx mailing list
> Intel-gfx@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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept

[Ville Syrjälä]

On Tue, Mar 10, 2015 at 02:32:34PM +0200, Ander Conselvan de Oliveira wrote:
> Remove the global modeset resource function that would disable the
> bifurcation bit, and instead enable/disable it when enabling or
> disabling the pch transcoder. The checks before the mode set should
> ensure that the configuration is valid, so it should be safe to do it
> so.
> 
> Signed-off-by: Ander Conselvan de Oliveira 
> 
> ---
> 
> On Mon, 2015-03-09 at 17:21 +0100, Daniel Vetter wrote:
> > With atomic we need to probably look at crtc_state->mode_changed. As an
> > interim solution we have the same information available in the
> > modeset_pipes bitmask. I think replacing the check for intel_crtc->active
> > with !(modeset_pipes & (1 
> I looked into that solution, but involves passing modeset_pipes way down
> into the call stack, so I started looking for a different solution. Do you
> see any caveat with this approach?
> 
> Thanks,
> Ander
> 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 60 
> +---
>  1 file changed, 21 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 4008bf4..eca5a41 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -101,6 +101,8 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
>   const struct intel_crtc_state *pipe_config);
>  static void intel_begin_crtc_commit(struct drm_crtc *crtc);
>  static void intel_finish_crtc_commit(struct drm_crtc *crtc);
> +static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc 
> *intel_crtc,
> + bool pipe_active);
>  
>  static struct intel_encoder *intel_find_encoder(struct intel_connector 
> *connector, int pipe)
>  {
> @@ -1956,6 +1958,8 @@ static void ironlake_disable_pch_transcoder(struct 
> drm_i915_private *dev_priv,
>   enum pipe pipe)
>  {
>   struct drm_device *dev = dev_priv->dev;
> + struct intel_crtc *crtc =
> + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
>   uint32_t reg, val;
>  
>   /* FDI relies on the transcoder */
> @@ -1980,6 +1984,9 @@ static void ironlake_disable_pch_transcoder(struct 
> drm_i915_private *dev_priv,
>   val &= ~TRANS_CHICKEN2_TIMING_OVERRIDE;
>   I915_WRITE(reg, val);
>   }
> +
> + if (IS_IVYBRIDGE(dev))
> + ivybridge_update_fdi_bc_bifurcation(crtc, false);
>  }
>  
>  static void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv)
> @@ -3153,32 +3160,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc 
> *crtc)
>   return crtc->base.state->enable && crtc->config->has_pch_encoder;
>  }
>  
> -static void ivb_modeset_global_resources(struct drm_device *dev)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *pipe_B_crtc =
> - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
> - struct intel_crtc *pipe_C_crtc =
> - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]);
> - uint32_t temp;
> -
> - /*
> -  * When everything is off disable fdi C so that we could enable fdi B
> -  * with all lanes. Note that we don't care about enabled pipes without
> -  * an enabled pch encoder.
> -  */
> - if (!pipe_has_enabled_pch(pipe_B_crtc) &&
> - !pipe_has_enabled_pch(pipe_C_crtc)) {
> - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
> - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
> -
> - temp = I915_READ(SOUTH_CHICKEN1);
> - temp &= ~FDI_BC_BIFURCATION_SELECT;
> - DRM_DEBUG_KMS("disabling fdi C rx\n");
> - I915_WRITE(SOUTH_CHICKEN1, temp);
> - }
> -}
> -
>  /* The FDI link training functions for ILK/Ibexpeak. */
>  static void ironlake_fdi_link_train(struct drm_crtc *crtc)
>  {
> @@ -3834,41 +3815,44 @@ static void 
> ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc,
>  I915_READ(VSYNCSHIFT(cpu_transcoder)));
>  }
>  
> -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
> +static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev, bool 
> enable)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   uint32_t temp;
>  
>   temp = I915_READ(SOUTH_CHICKEN1);
> - if (temp & FDI_BC_BIFURCATION_SELECT)
> + if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable)
>   return;
>  
>   WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
>   WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
>  
> - temp |= FDI_BC_BIFURCATION_SELECT;
> - DRM_DEBUG_KMS("enabling fdi C rx\n");
> + temp &= ~FDI_BC_BIFURCATION_SELECT;
> + if (enable)
> + temp |= FDI_BC_BIFURCATION_SELECT;
> +
> + DRM_DEBUG_KMS("%sabling fdi C 

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Don't assume primary & cursor are always on for wm calculation (v4)

[Daniel Vetter]

On Tue, Mar 10, 2015 at 10:51:34AM +, Tvrtko Ursulin wrote:
> 
> On 03/09/2015 06:06 PM, Matt Roper wrote:
> >Current ILK-style watermark code assumes the primary plane and cursor
> >plane are always enabled.  This assumption, along with the combination
> >of two independent commits that got merged at the same time, results in
> >a NULL dereference.  The offending commits are:
> >
> > commit fd2d61341bf39d1054256c07d6eddd624ebc4241
> > Author: Matt Roper 
> > Date:   Fri Feb 27 10:12:01 2015 -0800
> >
> > drm/i915: Use plane->state->fb in watermark code (v2)
> >
> >and
> >
> > commit 0fda65680e92545caea5be7805a7f0a617fb6c20
> > Author: Tvrtko Ursulin 
> > Date:   Fri Feb 27 15:12:35 2015 +
> >
> > drm/i915/skl: Update watermarks for Y tiling
> >
> >The first commit causes us to use the FB from plane->state->fb rather
> >than the legacy plane->fb, which is updated a bit later in the process.
> >
> >The second commit includes a change that now triggers watermark
> >reprogramming on primary plane enable/disable where we didn't have one
> >before (which wasn't really correct, but we had been getting lucky
> >because we always calculated as if the primary plane was on).
> >
> >Together, these two commits cause the watermark calculation to
> >(properly) see plane->state->fb = NULL when we're in the process of
> >disabling the primary plane.  However the existing watermark code
> >assumes there's always a primary fb and tries to dereference it to find
> >out pixel format / bpp information.
> >
> >The fix is to make ILK-style watermark calculation actually check the
> >true status of primary & cursor planes and adjust our watermark logic
> >accordingly.
> >
> >v2: Update unchecked uses of state->fb for other platforms (pnv, skl,
> > etc.).  Note that this is just a temporary fix.  Ultimately the
> > useful information is going to be computed at check time and stored
> > right in the state structures so that we don't have to figure this
> > all out while we're supposed to be programming the watermarks.
> > (caught by Tvrtko)
> >
> >v3: Fix a couple copy/paste mistakes in SKL code. (Tvrtko)
> >
> >v4: Only add FB checks for ILK/SKL codepaths.  Older platforms still use
> > intel_crtc_active() and will shortcircuit out of watermark
> > calculations before ever trying to dereference the primary plane's
> > framebuffer.
> >
> >Cc: Tvrtko Ursulin 
> >Reported-by: Michael Leuchtenburg 
> >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89388
> >Signed-off-by: Matt Roper 
> >---
> >  drivers/gpu/drm/i915/intel_pm.c | 62 
> > ++---
> >  1 file changed, 39 insertions(+), 23 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_pm.c 
> >b/drivers/gpu/drm/i915/intel_pm.c
> >index a06a2c7..7566cec 100644
> >--- a/drivers/gpu/drm/i915/intel_pm.c
> >+++ b/drivers/gpu/drm/i915/intel_pm.c
> >@@ -1962,13 +1962,25 @@ static void ilk_compute_wm_parameters(struct 
> >drm_crtc *crtc,
> > p->active = true;
> > p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
> > p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
> >-p->pri.bytes_per_pixel = crtc->primary->state->fb->bits_per_pixel / 8;
> >-p->cur.bytes_per_pixel = 4;
> >+
> >+if (crtc->primary->state->fb) {
> >+p->pri.enabled = true;
> >+p->pri.bytes_per_pixel =
> >+crtc->primary->state->fb->bits_per_pixel / 8;
> >+} else {
> >+p->pri.enabled = false;
> >+p->pri.bytes_per_pixel = 0;
> >+}
> >+
> >+if (crtc->cursor->state->fb) {
> >+p->cur.enabled = true;
> >+p->cur.bytes_per_pixel = 4;
> >+} else {
> >+p->cur.enabled = false;
> >+p->cur.bytes_per_pixel = 0;
> >+}
> > p->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
> > p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;
> >-/* TODO: for now, assume primary and cursor planes are always enabled. 
> >*/
> >-p->pri.enabled = true;
> >-p->cur.enabled = true;
> >
> > drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
> > struct intel_plane *intel_plane = to_intel_plane(plane);
> >@@ -2734,27 +2746,31 @@ static void skl_compute_wm_pipe_parameters(struct 
> >drm_crtc *crtc,
> > p->pipe_htotal = 
> > intel_crtc->config->base.adjusted_mode.crtc_htotal;
> > p->pixel_rate = skl_pipe_pixel_rate(intel_crtc->config);
> >
> >-/*
> >- * For now, assume primary and cursor planes are always enabled.
> >- */
> >-p->plane[0].enabled = true;
> >-p->plane[0].bytes_per_pixel =
> >-crtc->primary->state->fb->bits_per_pixel / 8;
> >-p->plane[0].horiz_pixels = intel_crtc->config->pipe_src_w;
> >-p->plane[0].vert_pixels = intel_crtc->config->pi

Re: [Intel-gfx] [PATCH v2] drm/i915: Prevent TLB error on first execution on SNB

[Jani Nikula]

On Tue, 10 Mar 2015, Daniel Vetter  wrote:
> On Tue, Mar 10, 2015 at 10:35:41AM +, Chris Wilson wrote:
>> On Tue, Mar 10, 2015 at 11:31:04AM +0100, Daniel Vetter wrote:
>> > On Fri, Feb 13, 2015 at 02:35:59PM +, Chris Wilson wrote:
>> > > Long ago I found that I was getting sporadic errors when booting SNB,
>> > > with the symptom being that the first batch died with IPEHR != *ACTHD,
>> > > typically caused by the TLB being invalid. These magically disappeared
>> > > if I held the forcewake during the entire ring initialisation sequence.
>> > > (It can probably be shortened to a short critical section, but the whole
>> > > initialisation is full of register writes and so we would be taking and
>> > > releasing forcewake almost continually, and so holding it over the
>> > > entire sequence will probably be a net win!)
>> > > 
>> > > Note some of the kernels I encounted the issue already had the deferred
>> > > forcewake release, so it is still relevant.
>> > > 
>> > > I know that there have been a few other reports with similar failure
>> > > conditions on SNB, I think such as
>> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=80913
>> > > 
>> > > v2: Wrap i915_gem_init_hw() with its own security blanket as we take
>> > > that path following resume and reset.
>> > > 
>> > > Signed-off-by: Chris Wilson 
>> > > ---
>> > >  drivers/gpu/drm/i915/i915_gem.c | 18 --
>> > >  1 file changed, 16 insertions(+), 2 deletions(-)
>> > > 
>> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c 
>> > > b/drivers/gpu/drm/i915/i915_gem.c
>> > > index 8d15c8110962..08450922f373 100644
>> > > --- a/drivers/gpu/drm/i915/i915_gem.c
>> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
>> > > @@ -4783,6 +4783,9 @@ i915_gem_init_hw(struct drm_device *dev)
>> > >  if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
>> > >  return -EIO;
>> > >  
>> > > +/* Double layer security blanket, see i915_gem_init() */
>> > > +intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>> > > +
>> > >  if (dev_priv->ellc_size)
>> > >  I915_WRITE(HSW_IDICR, I915_READ(HSW_IDICR) | 
>> > > IDIHASHMSK(0xf));
>> > >  
>> > > @@ -4815,7 +4818,7 @@ i915_gem_init_hw(struct drm_device *dev)
>> > >  for_each_ring(ring, dev_priv, i) {
>> > >  ret = ring->init_hw(ring);
>> > >  if (ret)
>> > > -return ret;
>> > > +goto out;
>> > >  }
>> > >  
>> > >  for (i = 0; i < NUM_L3_SLICES(dev); i++)
>> > > @@ -4832,9 +4835,11 @@ i915_gem_init_hw(struct drm_device *dev)
>> > >  DRM_ERROR("Context enable failed %d\n", ret);
>> > >  i915_gem_cleanup_ringbuffer(dev);
>> > >  
>> > > -return ret;
>> > > +goto out;
>> > >  }
>> > >  
>> > > +out:
>> > > +intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>> > >  return ret;
>> > >  }
>> > >  
>> > > @@ -4868,6 +4873,14 @@ int i915_gem_init(struct drm_device *dev)
>> > >  dev_priv->gt.stop_ring = intel_logical_ring_stop;
>> > >  }
>> > >  
>> > > +/* This is just a security blanket to placate dragons.
>> > > + * On some systems, we very sporadically observe that the first 
>> > > TLBs
>> > > + * used by the CS may be stale, despite us poking the TLB 
>> > > reset. If
>> > > + * we hold the forcewake during initialisation these problems
>> > > + * just magically go away.
>> > > + */
>> > > +intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>> > 
>> > gem_init shouldn't ever touch the hw except through gem_init_hw. Do we
>> > really need the double-layer here?
>> 
>> There are register accesses before, so yes since that's how I tested
>> it...
>> 
>> > Also the forcewake hack in the ring
>> > init code should now be redundant, too.
>> 
>> I am of the opinion that they still serve documentary value. Unless you
>> have an assert_force_wake() handy.
>
> Ok, count me convinced.
>
> Reviewed-by: Daniel Vetter 
>
> And I guess this is for Jani + cc: stable.

Pushed to drm-intel-fixes, thanks for the patch and review.

BR,
Jani.


> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] kernel BUG at drivers/gpu/drm/drm_mm.c:305!

[Krzysztof Kolasa]

System ( 32bit, Intel 945GM ) hangs after some short time, oops:

[ cut here ]
kernel BUG at drivers/gpu/drm/drm_mm.c:305!
invalid opcode:  [#1] SMP
Modules linked in: arc4 md4 cfg80211 8192cu(O) pci_stub vboxpci(O) 
vboxnetadp(O) vboxnetflt(O) vboxdrv(O) snd_hda_codec_si3054 
snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel 
snd_hda_controller snd_hda_codec snd_hwdep snd_pcm microcode 
snd_seq_midi snd_seq_midi_event snd_rawmidi joydev serio_raw snd_seq 
snd_timer snd_seq_device rfcomm bnep bluetooth i915 snd lpc_ich 
drm_kms_helper soundcore drm i2c_algo_bit parport_pc ppdev video mac_hid 
coretemp lp parport nls_utf8 cifs usb_storage psmouse 8139too 8139cp mii
CPU: 0 PID: 1165 Comm: Xorg Tainted: G   O 
4.0.0-rc3-winsoft-x86+ #11
Hardware name: FUJITSU SIEMENS AMILO Pro Edition V3405/AMILO Pro 
Edition V3405, BIOS R01-B0E03/20/2007

task: f64e0db0 ti: f4566000 task.ti: f4566000
EIP: 0060:[] EFLAGS: 00213206 CPU: 0
EIP is at drm_mm_insert_node_in_range_generic+0x432/0x4d0 [drm]
EAX: 0100 EBX: 00c78000 ECX: f6d4c908 EDX: 
ESI: f6d4c900 EDI: f6d4c900 EBP: f4567c78 ESP: f4567bf8
DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
CR0: 8005003b CR2: b4a02000 CR3: 345f2000 CR4: 07f0
Stack:
 f4567c64  0080  0080 f6d4c900  f4882700
    00a88000  1000 00478000 
    00f0    f472c33c
Call Trace:
 [] i915_gem_object_pin_view+0x3ce/0x840 [i915]
 [] ? i915_gem_obj_lookup_or_create_vma_view+0x41/0x160 [i915]
 [] i915_gem_execbuffer_reserve_vma.isra.11+0x62/0x100 [i915]
 [] i915_gem_execbuffer_reserve+0x291/0x2e0 [i915]
 [] i915_gem_do_execbuffer.isra.16+0x4f0/0xd10 [i915]
 [] ? poll_select_copy_remaining+0x100/0x100
 [] ? __kmalloc+0x4d/0x190
 [] i915_gem_execbuffer2+0x8b/0x2c0 [i915]
 [] ? i915_gem_execbuffer+0x4e0/0x4e0 [i915]
 [] drm_ioctl+0x1b7/0x510 [drm]
 [] ? i915_gem_execbuffer+0x4e0/0x4e0 [i915]
 [] ? ktime_get_ts64+0x52/0x1a0
 [] ? drm_getmap+0xb0/0xb0 [drm]
 [] do_vfs_ioctl+0x30a/0x530
 [] ? _copy_to_user+0x26/0x30
 [] ? ktime_get_ts64+0x52/0x1a0
 [] ? __fget_light+0x22/0x60
 [] SyS_ioctl+0x60/0x90
 [] sysenter_do_call+0x12/0x12
Code: ff ff 3b 55 e8 8d 74 26 00 77 10 73 04 0f 0b 66 90 3b 45 e4 90 8d 
74 26 00 72 f2 8b 75 94 03 46 20 13 56 24 3b 55 f0 72 0d 76 06 <0f> 0b 
8d 74 26 00 3b 45 ec 77 f5 39 55 c4 77 10 73 04 0f 0b 66
EIP: [] drm_mm_insert_node_in_range_generic+0x432/0x4d0 [drm] 
SS:ESP 0068:f4567bf8

---[ end trace 4648f53699b9eb32 ]---

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

[Intel-gfx] [PATCH] drm/i915: Add polish to VLV WM shift+mask operations

[ville . syrjala]

From: Ville Syrjälä 

Wrap the FW register value shift+mask operations into a macro to hide
the ugliness a bit. Also might avoid bugs due to typos.

Also rename all the primary/sprite plane low order bit masks to have the
_VLV suffix, so that we can use the FW_WM_VLV() macro instead of the
FW_WM() macro for them in a consistent manner. Cursor and all the high
order bits are left to use the FW_WM() macro as there's no real
confusion with them.

Cc: Daniel Vetter 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_reg.h | 10 +++---
 drivers/gpu/drm/i915/intel_pm.c | 74 +++--
 2 files changed, 46 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 495b22b..8ff039d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4161,25 +4161,25 @@ enum skl_disp_power_wells {
 #define   DSPFW_SPRITED_WM1_SHIFT  24
 #define   DSPFW_SPRITED_WM1_MASK   (0xff<<24)
 #define   DSPFW_SPRITED_SHIFT  16
-#define   DSPFW_SPRITED_MASK   (0xff<<16)
+#define   DSPFW_SPRITED_MASK_VLV   (0xff<<16)
 #define   DSPFW_SPRITEC_WM1_SHIFT  8
 #define   DSPFW_SPRITEC_WM1_MASK   (0xff<<8)
 #define   DSPFW_SPRITEC_SHIFT  0
-#define   DSPFW_SPRITEC_MASK   (0xff<<0)
+#define   DSPFW_SPRITEC_MASK_VLV   (0xff<<0)
 #define DSPFW8_CHV (VLV_DISPLAY_BASE + 0x700b8)
 #define   DSPFW_SPRITEF_WM1_SHIFT  24
 #define   DSPFW_SPRITEF_WM1_MASK   (0xff<<24)
 #define   DSPFW_SPRITEF_SHIFT  16
-#define   DSPFW_SPRITEF_MASK   (0xff<<16)
+#define   DSPFW_SPRITEF_MASK_VLV   (0xff<<16)
 #define   DSPFW_SPRITEE_WM1_SHIFT  8
 #define   DSPFW_SPRITEE_WM1_MASK   (0xff<<8)
 #define   DSPFW_SPRITEE_SHIFT  0
-#define   DSPFW_SPRITEE_MASK   (0xff<<0)
+#define   DSPFW_SPRITEE_MASK_VLV   (0xff<<0)
 #define DSPFW9_CHV (VLV_DISPLAY_BASE + 0x7007c) /* wtf #2? */
 #define   DSPFW_PLANEC_WM1_SHIFT   24
 #define   DSPFW_PLANEC_WM1_MASK(0xff<<24)
 #define   DSPFW_PLANEC_SHIFT   16
-#define   DSPFW_PLANEC_MASK(0xff<<16)
+#define   DSPFW_PLANEC_MASK_VLV(0xff<<16)
 #define   DSPFW_CURSORC_WM1_SHIFT  8
 #define   DSPFW_CURSORC_WM1_MASK   (0x3f<<16)
 #define   DSPFW_CURSORC_SHIFT  0
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0e84558..8ac358d0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -835,6 +835,11 @@ static bool g4x_compute_srwm(struct drm_device *dev,
  display, cursor);
 }
 
+#define FW_WM(value, plane) \
+   (((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK)
+#define FW_WM_VLV(value, plane) \
+   (((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK_VLV)
+
 static void vlv_write_wm_values(struct intel_crtc *crtc,
const struct vlv_wm_values *wm)
 {
@@ -848,50 +853,50 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
   (wm->ddl[pipe].primary << DDL_PLANE_SHIFT));
 
I915_WRITE(DSPFW1,
-  ((wm->sr.plane << DSPFW_SR_SHIFT) & DSPFW_SR_MASK) |
-  ((wm->pipe[PIPE_B].cursor << DSPFW_CURSORB_SHIFT) & 
DSPFW_CURSORB_MASK) |
-  ((wm->pipe[PIPE_B].primary << DSPFW_PLANEB_SHIFT) & 
DSPFW_PLANEB_MASK_VLV) |
-  ((wm->pipe[PIPE_A].primary << DSPFW_PLANEA_SHIFT) & 
DSPFW_PLANEA_MASK_VLV));
+  FW_WM(wm->sr.plane, SR) |
+  FW_WM(wm->pipe[PIPE_B].cursor, CURSORB) |
+  FW_WM_VLV(wm->pipe[PIPE_B].primary, PLANEB) |
+  FW_WM_VLV(wm->pipe[PIPE_A].primary, PLANEA));
I915_WRITE(DSPFW2,
-  ((wm->pipe[PIPE_A].sprite[1] << DSPFW_SPRITEB_SHIFT) & 
DSPFW_SPRITEB_MASK_VLV) |
-  ((wm->pipe[PIPE_A].cursor << DSPFW_CURSORA_SHIFT) & 
DSPFW_CURSORA_MASK) |
-  ((wm->pipe[PIPE_A].sprite[0] << DSPFW_SPRITEA_SHIFT) & 
DSPFW_SPRITEA_MASK_VLV));
+  FW_WM_VLV(wm->pipe[PIPE_A].sprite[1], SPRITEB) |
+  FW_WM(wm->pipe[PIPE_A].cursor, CURSORA) |
+  FW_WM_VLV(wm->pipe[PIPE_A].sprite[0], SPRITEA));
I915_WRITE(DSPFW3,
-  ((wm->sr.cursor << DSPFW_CURSOR_SR_SHIFT) & 
DSPFW_CURSOR_SR_MASK));
+  FW_WM(wm->sr.cursor, CURSOR_SR));
 
if (IS_CHERRYVIEW(dev_priv)) {
I915_WRITE(DSPFW7_CHV,
-  ((wm->pipe[PIPE_B].sprite[1] << DSPFW_SPRITED_SHIFT) 
& DSPFW_SPRITED_MASK) |
-  ((wm->pipe[PIPE_B].sprite[0] << DSPFW_SPRITEC_SHIFT) 
& DSPFW_SPRITEC_MASK));
+  FW_WM_VLV(wm->pipe[PIPE_B].sprite[1], SPRITED) |
+  FW_WM_VLV(wm->pipe[PIPE_B].sprite[0], SPRITEC));
I915_WRITE(DSPFW8_CHV,
-   

[Intel-gfx] [PATCH] drm/i915: Use FW_WM() macro for older gmch platforms too

[ville . syrjala]

From: Ville Syrjälä 

Use the FW_WM() macro from the VLV wm code to polish up the wm
code for older gmch platforms.

Cc: Daniel Vetter 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_reg.h |  4 ++--
 drivers/gpu/drm/i915/intel_pm.c | 42 +
 2 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8ff039d..793ed63 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4121,8 +4121,8 @@ enum skl_disp_power_wells {
 #define   DSPFW_SPRITEB_MASK_VLV   (0xff<<16) /* vlv/chv */
 #define   DSPFW_CURSORA_SHIFT  8
 #define   DSPFW_CURSORA_MASK   (0x3f<<8)
-#define   DSPFW_PLANEC_SHIFT_OLD   0
-#define   DSPFW_PLANEC_MASK_OLD(0x7f<<0) /* pre-gen4 sprite C 
*/
+#define   DSPFW_PLANEC_OLD_SHIFT   0
+#define   DSPFW_PLANEC_OLD_MASK(0x7f<<0) /* pre-gen4 sprite C 
*/
 #define   DSPFW_SPRITEA_SHIFT  0
 #define   DSPFW_SPRITEA_MASK   (0x7f<<0) /* g4x */
 #define   DSPFW_SPRITEA_MASK_VLV   (0xff<<0) /* vlv/chv */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8ac358d0..9ac9a2f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -301,6 +301,9 @@ static void chv_set_memory_pm5(struct drm_i915_private 
*dev_priv, bool enable)
mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
+#define FW_WM(value, plane) \
+   (((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK)
+
 void intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable)
 {
struct drm_device *dev = dev_priv->dev;
@@ -661,7 +664,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
pixel_size, latency->display_sr);
reg = I915_READ(DSPFW1);
reg &= ~DSPFW_SR_MASK;
-   reg |= wm << DSPFW_SR_SHIFT;
+   reg |= FW_WM(wm, SR);
I915_WRITE(DSPFW1, reg);
DRM_DEBUG_KMS("DSPFW1 register is %x\n", reg);
 
@@ -671,7 +674,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
pixel_size, latency->cursor_sr);
reg = I915_READ(DSPFW3);
reg &= ~DSPFW_CURSOR_SR_MASK;
-   reg |= (wm & 0x3f) << DSPFW_CURSOR_SR_SHIFT;
+   reg |= FW_WM(wm, CURSOR_SR);
I915_WRITE(DSPFW3, reg);
 
/* Display HPLL off SR */
@@ -680,7 +683,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
pixel_size, 
latency->display_hpll_disable);
reg = I915_READ(DSPFW3);
reg &= ~DSPFW_HPLL_SR_MASK;
-   reg |= wm & DSPFW_HPLL_SR_MASK;
+   reg |= FW_WM(wm, HPLL_SR);
I915_WRITE(DSPFW3, reg);
 
/* cursor HPLL off SR */
@@ -689,7 +692,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
pixel_size, 
latency->cursor_hpll_disable);
reg = I915_READ(DSPFW3);
reg &= ~DSPFW_HPLL_CURSOR_MASK;
-   reg |= (wm & 0x3f) << DSPFW_HPLL_CURSOR_SHIFT;
+   reg |= FW_WM(wm, HPLL_CURSOR);
I915_WRITE(DSPFW3, reg);
DRM_DEBUG_KMS("DSPFW3 register is %x\n", reg);
 
@@ -835,8 +838,6 @@ static bool g4x_compute_srwm(struct drm_device *dev,
  display, cursor);
 }
 
-#define FW_WM(value, plane) \
-   (((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK)
 #define FW_WM_VLV(value, plane) \
(((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK_VLV)
 
@@ -904,7 +905,6 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
dev_priv->wm.vlv = *wm;
 }
 
-#undef FW_WM
 #undef FW_WM_VLV
 
 static uint8_t vlv_compute_drain_latency(struct drm_crtc *crtc,
@@ -1163,17 +1163,17 @@ static void g4x_update_wm(struct drm_crtc *crtc)
  plane_sr, cursor_sr);
 
I915_WRITE(DSPFW1,
-  (plane_sr << DSPFW_SR_SHIFT) |
-  (cursorb_wm << DSPFW_CURSORB_SHIFT) |
-  (planeb_wm << DSPFW_PLANEB_SHIFT) |
-  (planea_wm << DSPFW_PLANEA_SHIFT));
+  FW_WM(plane_sr, SR) |
+  FW_WM(cursorb_wm, CURSORB) |
+  FW_WM(planeb_wm, PLANEB) |
+  FW_WM(planea_wm, PLANEA));
I915_WRITE(DSPFW2,
   (I915_READ(DSPFW2) & ~DSPFW_CURSORA_MASK) |
-  (cursora_wm << DSPFW_CURSORA_SHIFT));
+  FW_WM(cursora_wm, CURSORA));
/* HPLL off in SR has some issues on G4x... disable it */
I915_WRITE(DSPFW3,
   (I915_READ(DSPFW3) & ~(DSPFW_HPLL_SR_EN | 
DSPFW_CURSOR_SR_MASK)) |
-  (cursor_sr << DSPFW

Re: [Intel-gfx] [PATCH 1/9] drm/i915: Remove debug prints from primary plane update funcs

[Jesse Barnes]

On 03/10/2015 04:15 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> These are now called from the plane commit hooks, so they really need to
> be fast or else we risk atomic update failures. So kill the debug prints
> which are slowing things down massively.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 ---
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index c3a5888..1d5107e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2658,9 +2658,6 @@ static void i9xx_update_primary_plane(struct drm_crtc 
> *crtc,
>  
>   I915_WRITE(reg, dspcntr);
>  
> - DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
> -   i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
> -   fb->pitches[0]);
>   I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
>   if (INTEL_INFO(dev)->gen >= 4) {
>   I915_WRITE(DSPSURF(plane),
> @@ -2762,9 +2759,6 @@ static void ironlake_update_primary_plane(struct 
> drm_crtc *crtc,
>  
>   I915_WRITE(reg, dspcntr);
>  
> - DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
> -   i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
> -   fb->pitches[0]);
>   I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
>   I915_WRITE(DSPSURF(plane),
>  i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset);
> @@ -2890,11 +2884,6 @@ static void skylake_update_primary_plane(struct 
> drm_crtc *crtc,
>  
>   I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
>  
> - DRM_DEBUG_KMS("Writing base %08lX %d,%d,%d,%d pitch=%d\n",
> -   i915_gem_obj_ggtt_offset(obj),
> -   x, y, fb->width, fb->height,
> -   fb->pitches[0]);
> -
>   I915_WRITE(PLANE_POS(pipe, 0), 0);
>   I915_WRITE(PLANE_OFFSET(pipe, 0), (y << 16) | x);
>   I915_WRITE(PLANE_SIZE(pipe, 0),
> 

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

[Intel-gfx] [PATCH] drm/i915: Optimistically spin for the request completion

[Chris Wilson]

This provides a nice boost to mesa in swap bound scenarios (as mesa
throttles itself to the previous frame and given the scenario that will
complete shortly). It will also provide a good boost to systems running
with semaphores disabled and so frequently waiting on the GPU as it
switches rings.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_gem.c | 43 +++--
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index de15bd319bd0..2aaa3cab2be4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1191,6 +1191,28 @@ static bool missed_irq(struct drm_i915_private *dev_priv,
return test_bit(ring->id, &dev_priv->gpu_error.missed_irq_rings);
 }
 
+static int __i915_spin_request(struct drm_i915_gem_request *req)
+{
+   struct intel_engine_cs *ring = i915_gem_request_get_ring(req);
+   struct drm_i915_private *dev_priv = to_i915(ring->dev);
+   int ret = -EBUSY;
+
+   intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+   while (!need_resched()) {
+   if (i915_gem_request_completed(req, true)) {
+   ret = 0;
+   goto out;
+   }
+
+   cpu_relax_lowlatency();
+   }
+   if (i915_gem_request_completed(req, false))
+   ret = 0;
+out:
+   intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+   return ret;
+}
+
 /**
  * __i915_wait_request - wait until execution of request has finished
  * @req: duh!
@@ -1235,12 +1257,20 @@ int __i915_wait_request(struct drm_i915_gem_request 
*req,
if (ring->id == RCS && INTEL_INFO(dev)->gen >= 6)
gen6_rps_boost(dev_priv, file_priv);
 
-   if (!irq_test_in_progress && WARN_ON(!ring->irq_get(ring)))
-   return -ENODEV;
-
/* Record current time in case interrupted by signal, or wedged */
trace_i915_gem_request_wait_begin(req);
before = ktime_get_raw_ns();
+
+   /* Optimistic spin before touching IRQs */
+   ret = __i915_spin_request(req);
+   if (ret == 0)
+   goto out;
+
+   if (!irq_test_in_progress && WARN_ON(!ring->irq_get(ring))) {
+   ret = -ENODEV;
+   goto out;
+   }
+
for (;;) {
struct timer_list timer;
 
@@ -1289,14 +1319,15 @@ int __i915_wait_request(struct drm_i915_gem_request 
*req,
destroy_timer_on_stack(&timer);
}
}
-   now = ktime_get_raw_ns();
-   trace_i915_gem_request_wait_end(req);
-
if (!irq_test_in_progress)
ring->irq_put(ring);
 
finish_wait(&ring->irq_queue, &wait);
 
+out:
+   now = ktime_get_raw_ns();
+   trace_i915_gem_request_wait_end(req);
+
if (timeout) {
s64 tres = *timeout - (now - before);
 
-- 
2.1.4

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

Re: [Intel-gfx] [PATCH] drm/i915: Optimistically spin for the request completion

[Chris Wilson]

On Tue, Mar 10, 2015 at 04:06:19PM +, Chris Wilson wrote:
> @@ -1235,12 +1257,20 @@ int __i915_wait_request(struct drm_i915_gem_request 
> *req,
>   if (ring->id == RCS && INTEL_INFO(dev)->gen >= 6)
>   gen6_rps_boost(dev_priv, file_priv);
>  
> - if (!irq_test_in_progress && WARN_ON(!ring->irq_get(ring)))
> - return -ENODEV;
> -
>   /* Record current time in case interrupted by signal, or wedged */
>   trace_i915_gem_request_wait_begin(req);
>   before = ktime_get_raw_ns();
> +
> + /* Optimistic spin before touching IRQs */
Perhaps iff timeout == NULL, or pass it along and add a

if (timeout && timeout_after_eq(jiffies, timeout))
break;

before the cpu_relax()?

> + ret = __i915_spin_request(req);
> + if (ret == 0)
> + goto out;

-- 
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] [Beignet] [PATCH i-g-t 2/2] configure: Bump required libdrm version to 2.4.60

[Jeff McGee]

On Tue, Mar 10, 2015 at 08:37:30AM +0100, Daniel Vetter wrote:
> On Mon, Mar 09, 2015 at 04:41:02PM -0700, jeff.mc...@intel.com wrote:
> > From: Jeff McGee 
> > 
> > tests/core_getparams needs the new libdrm interfaces for
> > querying subslice and EU counts.
> > 
> > For: VIZ-4636
> > Signed-off-by: Jeff McGee 
> > ---
> >  configure.ac | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 16d6a2e..88a1c3d 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -82,7 +82,7 @@ if test "x$GCC" = "xyes"; then
> >  fi
> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
> >  
> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.52 libdrm])
> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.60 libdrm])
> 
> Please don't and instead copypaste the new structs/defines with a local_
> prefix like we do it for all the other new igt testcases. Forcing libdrm
> to get updated for igt all the time can get annoying fast.
> -Daniel
> 
In this case I'm trying to exercise new API functions in libdrm which
wrap the GETPARAM ioctl. Would you rather me bypass the wrapper to
avoid requiring updated libdrm? I can do that, but it fails to test the
complete path that client would use.
-Jeff

> >  PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
> >  PKG_CHECK_MODULES(OVERLAY_XVLIB, [xv x11 xext dri2proto >= 2.6], 
> > enable_overlay_xvlib=yes, enable_overlay_xvlib=no)
> >  PKG_CHECK_MODULES(OVERLAY_XLIB, [cairo-xlib dri2proto >= 2.6], 
> > enable_overlay_xlib=yes, enable_overlay_xlib=no)
> > -- 
> > 2.3.0
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@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
> ___
> Beignet mailing list
> beig...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Do not use ggtt_view with (aliasing) PPGTT

[Tvrtko Ursulin]

Hi,

On the high level I don't like the explosion of _ppgtt_ functions and 
the name in the first place. As Daniel commented a lot of those (if not 
all) should not care between ppgtt and ggtt.


I do like the goal of having _ggtt_ versions which take the view, and 
normal/non-suffixed/legacy ones which work with any VMA (ppgtt/ggtt) and 
assume normal view in the ggtt case.


If that basically means just renaming _view to _ggtt (more or less) then 
it is not that great, but if you can get some more 
unification/clarification/consistency in the process then it is good.


And I am not sure that there is place for both _ggtt_ and _view prefix 
in the final goal - that looks even more messy.


How many places would you have to change for partial views if you didn't 
do this cleanup first? If not too much maybe do that first and then see 
if there is scope for a cleanup?


Regards,

Tvrtko

On 03/09/2015 02:34 PM, Joonas Lahtinen wrote:

GGTT views are only applicable when dealing with GGTT. Change the code to
reject ggtt_view where it should not be used and require it when it should
be.

Signed-off-by: Joonas Lahtinen 
---
  drivers/gpu/drm/i915/i915_drv.h |  91 
  drivers/gpu/drm/i915/i915_gem.c | 134 
  drivers/gpu/drm/i915/i915_gem_gtt.c |  41 ---
  3 files changed, 184 insertions(+), 82 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dd7651b..6bc67fb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2608,20 +2608,16 @@ void i915_gem_vma_destroy(struct i915_vma *vma);
  #define PIN_GLOBAL 0x4
  #define PIN_OFFSET_BIAS 0x8
  #define PIN_OFFSET_MASK (~4095)
-int __must_check i915_gem_object_pin_view(struct drm_i915_gem_object *obj,
- struct i915_address_space *vm,
- uint32_t alignment,
- uint64_t flags,
- const struct i915_ggtt_view *view);
-static inline
-int __must_check i915_gem_object_pin(struct drm_i915_gem_object *obj,
-struct i915_address_space *vm,
-uint32_t alignment,
-uint64_t flags)
-{
-   return i915_gem_object_pin_view(obj, vm, alignment, flags,
-   &i915_ggtt_view_normal);
-}
+int __must_check
+i915_gem_object_pin(struct drm_i915_gem_object *obj,
+   struct i915_address_space *vm,
+   uint32_t alignment,
+   uint64_t flags);
+int __must_check
+i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
+const struct i915_ggtt_view *view,
+uint32_t alignment,
+uint64_t flags);

  int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
  u32 flags);
@@ -2785,41 +2781,55 @@ struct dma_buf *i915_gem_prime_export(struct drm_device 
*dev,

  void i915_gem_restore_fences(struct drm_device *dev);

-unsigned long i915_gem_obj_offset_view(struct drm_i915_gem_object *o,
-  struct i915_address_space *vm,
-  enum i915_ggtt_view_type view);
-static inline
-unsigned long i915_gem_obj_offset(struct drm_i915_gem_object *o,
- struct i915_address_space *vm)
+static inline bool i915_is_ggtt(struct i915_address_space *vm);
+
+unsigned long
+i915_gem_obj_ppgtt_offset(struct drm_i915_gem_object *o,
+ struct i915_address_space *vm);
+unsigned long
+i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o,
+ enum i915_ggtt_view_type view);
+static inline unsigned long
+i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *o)
  {
-   return i915_gem_obj_offset_view(o, vm, I915_GGTT_VIEW_NORMAL);
+   return i915_gem_obj_ggtt_offset_view(o, I915_GGTT_VIEW_NORMAL);
  }
-bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o);
-bool i915_gem_obj_bound_view(struct drm_i915_gem_object *o,
-struct i915_address_space *vm,
-enum i915_ggtt_view_type view);
-static inline
-bool i915_gem_obj_bound(struct drm_i915_gem_object *o,
-   struct i915_address_space *vm)
+
+static inline unsigned long
+i915_gem_obj_offset(struct drm_i915_gem_object *o,
+   struct i915_address_space *vm)
  {
-   return i915_gem_obj_bound_view(o, vm, I915_GGTT_VIEW_NORMAL);
+   if (i915_is_ggtt(vm))
+   return i915_gem_obj_ggtt_offset(o);
+   return i915_gem_obj_ppgtt_offset(o, vm);
  }

+bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o);
+bool i915_gem_obj_bound(struct drm_i915_gem_object *o,
+   struct i915_address_space *vm);

Re: [Intel-gfx] [PATCH 4/9] drm/i915: Make derived plane state correct after crtc_enable

[Matt Roper]

On Tue, Mar 10, 2015 at 01:15:24PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> We need to call the plane .atomic_check() hook when enabling the crtc
> to make sure all the derived state (eg. visible, clipped src/dst
> coordinates) are up to date when we enable the plane. This allows us to
> trust this derived state everywhere.
> 
> We also take the opportunity to rewrite the plane enable sequence to
> perform it as a single atomic update, which is a bit closer to where we
> want to end up. Obviously this is a bit of a hack as we can't deal with
> errors from .atomic_check(), so we just paper over that by making sure
> visible=false so that we don't try to enable the plane with potentially
> garbage coordinates and whatnot.
> 
> We also abuse the atomic code a bit by not making another copy of the
> plane state and just frobbing directly with the plane->state.
> 
> Cc: Matt Roper 
> Cc: Sonika Jindal 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |   3 +
>  drivers/gpu/drm/i915/intel_display.c | 222 
> ++-
>  2 files changed, 88 insertions(+), 137 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5462cbf..b16c0a7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -238,6 +238,9 @@ enum hpd_pin {
>  #define for_each_intel_crtc(dev, intel_crtc) \
>   list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list, base.head)
>  
> +#define for_each_intel_plane(dev, intel_plane) \
> + list_for_each_entry(intel_plane, &dev->mode_config.plane_list, 
> base.head)
> +
>  #define for_each_intel_encoder(dev, intel_encoder)   \
>   list_for_each_entry(intel_encoder,  \
>   &(dev)->mode_config.encoder_list,   \
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 0da3abf..fdc83f1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2124,66 +2124,6 @@ void intel_flush_primary_plane(struct drm_i915_private 
> *dev_priv,
>   POSTING_READ(reg);
>  }
>  
> -/**
> - * intel_enable_primary_hw_plane - enable the primary plane on a given pipe
> - * @plane:  plane to be enabled
> - * @crtc: crtc for the plane
> - *
> - * Enable @plane on @crtc, making sure that the pipe is running first.
> - */
> -static void intel_enable_primary_hw_plane(struct drm_plane *plane,
> -   struct drm_crtc *crtc)
> -{
> - struct drm_device *dev = plane->dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -
> - /* If the pipe isn't enabled, we can't pump pixels and may hang */
> - assert_pipe_enabled(dev_priv, intel_crtc->pipe);
> -
> - if (intel_crtc->primary_enabled)
> - return;
> -
> - intel_crtc->primary_enabled = true;
> -
> - dev_priv->display.update_primary_plane(crtc, plane->fb,
> -crtc->x, crtc->y);
> -
> - /*
> -  * BDW signals flip done immediately if the plane
> -  * is disabled, even if the plane enable is already
> -  * armed to occur at the next vblank :(
> -  */
> - if (IS_BROADWELL(dev))
> - intel_wait_for_vblank(dev, intel_crtc->pipe);
> -}
> -
> -/**
> - * intel_disable_primary_hw_plane - disable the primary hardware plane
> - * @plane: plane to be disabled
> - * @crtc: crtc for the plane
> - *
> - * Disable @plane on @crtc, making sure that the pipe is running first.
> - */
> -static void intel_disable_primary_hw_plane(struct drm_plane *plane,
> -struct drm_crtc *crtc)
> -{
> - struct drm_device *dev = plane->dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -
> - if (WARN_ON(!intel_crtc->active))
> - return;
> -
> - if (!intel_crtc->primary_enabled)
> - return;
> -
> - intel_crtc->primary_enabled = false;
> -
> - dev_priv->display.update_primary_plane(crtc, plane->fb,
> -crtc->x, crtc->y);
> -}
> -
>  static bool need_vtd_wa(struct drm_device *dev)
>  {
>  #ifdef CONFIG_INTEL_IOMMU
> @@ -2895,7 +2835,10 @@ static void skylake_update_primary_plane(struct 
> drm_crtc *crtc,
>   POSTING_READ(PLANE_SURF(pipe, 0));
>  }
>  
> -/* Assume fb object is pinned & idle & fenced and just update base pointers 
> */
> +/*
> + * Assume fb object is big enough & pinned & idle & fenced,
> + * and just update base pointers
> + */
>  static int
>  intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  int x, int y, enum mode_set_atomic state)
> @@ -2906,6 +2849,7 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, 
> stru

Re: [Intel-gfx] [PATCH 3/9] drm/i915: Use plane->state->fb instead of plane->fb in intel_plane_restore()

[Matt Roper]

On Tue, Mar 10, 2015 at 01:15:23PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> plane->fb is not as reliable as plane->state->fb so let's convert
> intel_plane_restore() over the the new way of thinking as well.
> 
> Cc: Matt Roper 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 7051da7..a828736 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1361,10 +1361,10 @@ out_unlock:
>  
>  int intel_plane_restore(struct drm_plane *plane)
>  {
> - if (!plane->crtc || !plane->fb)
> + if (!plane->crtc || !plane->state->fb)
>   return 0;
>  
> - return plane->funcs->update_plane(plane, plane->crtc, plane->fb,
> + return plane->funcs->update_plane(plane, plane->crtc, plane->state->fb,

While we're at it, should we s/plane->crtc/plane->state->crtc/ as well?

Otherwise, 1-3 are

Reviewed-by: Matt Roper 

> plane->state->crtc_x, plane->state->crtc_y,
> plane->state->crtc_w, plane->state->crtc_h,
> plane->state->src_x, plane->state->src_y,
> -- 
> 2.0.5
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 7/9] drm/i915: Update watermarks after the derived plane state is uptodate

[Matt Roper]

On Tue, Mar 10, 2015 at 01:15:27PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> When enabling planes during .crtc_enable() we currently want to update
> the watermarks before enabling the planes. We already do it once just
> before enabling the pipe, but at that point out derived plane state is
> still out of whack, so we need to do it again after the .atomic_check()
> hooks have been called.
> 
> What this means is now we could actually start to trust the derived
> plane state (clipped size, 'visible', etc.) in the watermark code.
> 
> The pre pipe enable watermark update is supposed to be just make sure
> the other pipes are ready to have their FIFOs potentially reduced, so we
> need to keep it there as well.
> 
> Since we don't yet have proper two-part watermark update leave the
> watermakrs alone in the plane disable case. This way they'll get updated
> only after the planes and pipe have all been turned off.
> 
> Cc: Matt Roper 
> Signed-off-by: Ville Syrjälä 

Reviewed-by: Matt Roper 

> ---
>  drivers/gpu/drm/i915/intel_display.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 33680a8..a9201a7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4167,6 +4167,8 @@ static void _intel_crtc_enable_planes(struct intel_crtc 
> *crtc)
>   state->visible = false;
>   }
>  
> + intel_update_watermarks(&crtc->base);
> +
>   crtc_funcs->atomic_begin(&crtc->base);
>  
>   for_each_intel_plane(dev, plane) {
> @@ -4213,6 +4215,8 @@ static void _intel_crtc_disable_planes(struct 
> intel_crtc *crtc)
>   }
>  
>   crtc_funcs->atomic_flush(&crtc->base);
> +
> + /* we'll defer watermark update to after the pipe has been disabled */
>  }
>  
>  void hsw_enable_ips(struct intel_crtc *crtc)
> -- 
> 2.0.5
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 8/9] drm/i915: Use state->visible in wm calculation

[Matt Roper]

On Tue, Mar 10, 2015 at 01:15:28PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Signed-off-by: Ville Syrjälä 

The actual diff here needs to be regenerated since my patch got merged
and changed this area of the code, but your final solution looks right
to me.  The main difference between your patch here and what got merged
yesterday is that your patch starts using state->visible rather than
state->fb, which is the right move to make.

So assuming you rebase your changes onto the latest code,
Reviewed-by: Matt Roper 

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 24 +---
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a06a2c7..499e054 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1955,6 +1955,7 @@ static void ilk_compute_wm_parameters(struct drm_crtc 
> *crtc,
>   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   enum pipe pipe = intel_crtc->pipe;
>   struct drm_plane *plane;
> + const struct intel_plane_state *state;
>  
>   if (!intel_crtc->active)
>   return;
> @@ -1962,13 +1963,22 @@ static void ilk_compute_wm_parameters(struct drm_crtc 
> *crtc,
>   p->active = true;
>   p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
>   p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
> - p->pri.bytes_per_pixel = crtc->primary->state->fb->bits_per_pixel / 8;
> - p->cur.bytes_per_pixel = 4;
> - p->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
> - p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;
> - /* TODO: for now, assume primary and cursor planes are always enabled. 
> */
> - p->pri.enabled = true;
> - p->cur.enabled = true;
> +
> + state = to_intel_plane_state(crtc->primary->state);
> + if (state->visible) {
> + p->pri.enabled = true;
> + p->pri.bytes_per_pixel =
> + drm_format_plane_cpp(state->base.fb->pixel_format, 0);
> + p->pri.horiz_pixels = drm_rect_width(&state->dst);
> + }
> +
> + state = to_intel_plane_state(crtc->cursor->state);
> + if (state->visible) {
> + p->cur.enabled = true;
> + p->cur.bytes_per_pixel =
> + drm_format_plane_cpp(state->base.fb->pixel_format, 0);
> + p->cur.horiz_pixels = state->base.crtc_w;
> + }
>  
>   drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
>   struct intel_plane *intel_plane = to_intel_plane(plane);
> -- 
> 2.0.5
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/9] drm/i915: Use plane->state->fb instead of plane->fb in intel_plane_restore()

[Ville Syrjälä]

On Tue, Mar 10, 2015 at 10:01:47AM -0700, Matt Roper wrote:
> On Tue, Mar 10, 2015 at 01:15:23PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > plane->fb is not as reliable as plane->state->fb so let's convert
> > intel_plane_restore() over the the new way of thinking as well.
> > 
> > Cc: Matt Roper 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/intel_sprite.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index 7051da7..a828736 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1361,10 +1361,10 @@ out_unlock:
> >  
> >  int intel_plane_restore(struct drm_plane *plane)
> >  {
> > -   if (!plane->crtc || !plane->fb)
> > +   if (!plane->crtc || !plane->state->fb)
> > return 0;
> >  
> > -   return plane->funcs->update_plane(plane, plane->crtc, plane->fb,
> > +   return plane->funcs->update_plane(plane, plane->crtc, plane->state->fb,
> 
> While we're at it, should we s/plane->crtc/plane->state->crtc/ as well?

I tried to make that change everywhere and it blew up. But I think that
was simply because I changed it some .commit hook as well, and currently
we don't have the old state around there, so the 'crtc ? crtc : state->crtc'
just ended up as 'crtc' effectively and that of course didn't work as well
as I'd hoped ;)

But yeah maybe we should make that change. Would just need to pass the
old state to commit instead of the new state, I think.

> 
> Otherwise, 1-3 are
> 
> Reviewed-by: Matt Roper 
> 
> >   plane->state->crtc_x, plane->state->crtc_y,
> >   plane->state->crtc_w, plane->state->crtc_h,
> >   plane->state->src_x, plane->state->src_y,
> > -- 
> > 2.0.5
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 8/9] drm/i915: Use state->visible in wm calculation

[Ville Syrjälä]

On Tue, Mar 10, 2015 at 10:19:56AM -0700, Matt Roper wrote:
> On Tue, Mar 10, 2015 at 01:15:28PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > Signed-off-by: Ville Syrjälä 
> 
> The actual diff here needs to be regenerated since my patch got merged
> and changed this area of the code, but your final solution looks right
> to me.  The main difference between your patch here and what got merged
> yesterday is that your patch starts using state->visible rather than
> state->fb, which is the right move to make.
> 
> So assuming you rebase your changes onto the latest code,
> Reviewed-by: Matt Roper 

Yep, I was expecting a slight conflict here since you were working in the
same vicinity. So I'll respin this one on top of the latest stuff.

And I should change the vlv/chv code while I'm at it...

> 
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 24 +---
> >  1 file changed, 17 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c 
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index a06a2c7..499e054 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -1955,6 +1955,7 @@ static void ilk_compute_wm_parameters(struct drm_crtc 
> > *crtc,
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > enum pipe pipe = intel_crtc->pipe;
> > struct drm_plane *plane;
> > +   const struct intel_plane_state *state;
> >  
> > if (!intel_crtc->active)
> > return;
> > @@ -1962,13 +1963,22 @@ static void ilk_compute_wm_parameters(struct 
> > drm_crtc *crtc,
> > p->active = true;
> > p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
> > p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
> > -   p->pri.bytes_per_pixel = crtc->primary->state->fb->bits_per_pixel / 8;
> > -   p->cur.bytes_per_pixel = 4;
> > -   p->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
> > -   p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;
> > -   /* TODO: for now, assume primary and cursor planes are always enabled. 
> > */
> > -   p->pri.enabled = true;
> > -   p->cur.enabled = true;
> > +
> > +   state = to_intel_plane_state(crtc->primary->state);
> > +   if (state->visible) {
> > +   p->pri.enabled = true;
> > +   p->pri.bytes_per_pixel =
> > +   drm_format_plane_cpp(state->base.fb->pixel_format, 0);
> > +   p->pri.horiz_pixels = drm_rect_width(&state->dst);
> > +   }
> > +
> > +   state = to_intel_plane_state(crtc->cursor->state);
> > +   if (state->visible) {
> > +   p->cur.enabled = true;
> > +   p->cur.bytes_per_pixel =
> > +   drm_format_plane_cpp(state->base.fb->pixel_format, 0);
> > +   p->cur.horiz_pixels = state->base.crtc_w;
> > +   }
> >  
> > drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
> > struct intel_plane *intel_plane = to_intel_plane(plane);
> > -- 
> > 2.0.5
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 9/9] drm/i915: Don't re-enable an explicitly disabled primary plane due to sprite coverage changes

[shuang . he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: 
shuang...@intel.com)
Task id: 5924
-Summary-
Platform  Delta  drm-intel-nightly  Series Applied
PNV  282/282  282/282
ILK  308/308  308/308
SNB  307/307  307/307
IVB  375/375  375/375
BYT  294/294  294/294
HSW  385/385  385/385
BDW  315/315  315/315
-Detailed-
Platform  Testdrm-intel-nightly  Series 
Applied
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [Beignet] [PATCH i-g-t 2/2] configure: Bump required libdrm version to 2.4.60

[Rob Clark]

On Tue, Mar 10, 2015 at 2:34 PM, Jeff McGee  wrote:
> On Tue, Mar 10, 2015 at 01:58:52PM -0400, Rob Clark wrote:
>> On Tue, Mar 10, 2015 at 12:59 PM, Jeff McGee  wrote:
>> > On Tue, Mar 10, 2015 at 08:37:30AM +0100, Daniel Vetter wrote:
>> >> On Mon, Mar 09, 2015 at 04:41:02PM -0700, jeff.mc...@intel.com wrote:
>> >> > From: Jeff McGee 
>> >> >
>> >> > tests/core_getparams needs the new libdrm interfaces for
>> >> > querying subslice and EU counts.
>> >> >
>> >> > For: VIZ-4636
>> >> > Signed-off-by: Jeff McGee 
>> >> > ---
>> >> >  configure.ac | 2 +-
>> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/configure.ac b/configure.ac
>> >> > index 16d6a2e..88a1c3d 100644
>> >> > --- a/configure.ac
>> >> > +++ b/configure.ac
>> >> > @@ -82,7 +82,7 @@ if test "x$GCC" = "xyes"; then
>> >> >  fi
>> >> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
>> >> >
>> >> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.52 libdrm])
>> >> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.60 libdrm])
>> >>
>> >> Please don't and instead copypaste the new structs/defines with a local_
>> >> prefix like we do it for all the other new igt testcases. Forcing libdrm
>> >> to get updated for igt all the time can get annoying fast.
>> >> -Daniel
>> >>
>> > In this case I'm trying to exercise new API functions in libdrm which
>> > wrap the GETPARAM ioctl. Would you rather me bypass the wrapper to
>> > avoid requiring updated libdrm? I can do that, but it fails to test the
>> > complete path that client would use.
>>
>>
>> Am I missing something, or does 2.4.60 not exist yet?
>>
>> That said dependency bumps for igt seem like less of an issue than
>> dependency bumps for mesa..  I mean if you are using igt you are
>> probably on the latest anyways..  I'm not sure why Daniel is so
>> concerned about that..
>>
>> (but dependency bumps to something that doesn't exist yet should
>> perhaps be avoided)
>>
>> BR,
>> -R
>>
>
> Hi Rob. This igt change is contigent upon my libdrm changes which
> would in fact bump that version to 2.4.60 after adding an API. That
> change is also posted and waiting review. I guess I should have stated
> that dependency here to begin with.
>
> http://lists.freedesktop.org/archives/intel-gfx/2015-March/061101.html
>

ahh, my bad.. I hadn't read all of the threads.. sorry for the noise ;-)

BR,
-R

> Jeff
>>
>> > -Jeff
>> >
>> >> >  PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
>> >> >  PKG_CHECK_MODULES(OVERLAY_XVLIB, [xv x11 xext dri2proto >= 2.6], 
>> >> > enable_overlay_xvlib=yes, enable_overlay_xvlib=no)
>> >> >  PKG_CHECK_MODULES(OVERLAY_XLIB, [cairo-xlib dri2proto >= 2.6], 
>> >> > enable_overlay_xlib=yes, enable_overlay_xlib=no)
>> >> > --
>> >> > 2.3.0
>> >> >
>> >> > ___
>> >> > Intel-gfx mailing list
>> >> > Intel-gfx@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
>> >> ___
>> >> Beignet mailing list
>> >> beig...@lists.freedesktop.org
>> >> http://lists.freedesktop.org/mailman/listinfo/beignet
>> > ___
>> > dri-devel mailing list
>> > dri-de...@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>> ___
>> dri-devel mailing list
>> dri-de...@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [Beignet] [PATCH i-g-t 2/2] configure: Bump required libdrm version to 2.4.60

[Daniel Vetter]

On Tue, Mar 10, 2015 at 01:58:52PM -0400, Rob Clark wrote:
> On Tue, Mar 10, 2015 at 12:59 PM, Jeff McGee  wrote:
> > On Tue, Mar 10, 2015 at 08:37:30AM +0100, Daniel Vetter wrote:
> >> On Mon, Mar 09, 2015 at 04:41:02PM -0700, jeff.mc...@intel.com wrote:
> >> > From: Jeff McGee 
> >> >
> >> > tests/core_getparams needs the new libdrm interfaces for
> >> > querying subslice and EU counts.
> >> >
> >> > For: VIZ-4636
> >> > Signed-off-by: Jeff McGee 
> >> > ---
> >> >  configure.ac | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/configure.ac b/configure.ac
> >> > index 16d6a2e..88a1c3d 100644
> >> > --- a/configure.ac
> >> > +++ b/configure.ac
> >> > @@ -82,7 +82,7 @@ if test "x$GCC" = "xyes"; then
> >> >  fi
> >> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
> >> >
> >> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.52 libdrm])
> >> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.60 libdrm])
> >>
> >> Please don't and instead copypaste the new structs/defines with a local_
> >> prefix like we do it for all the other new igt testcases. Forcing libdrm
> >> to get updated for igt all the time can get annoying fast.
> >> -Daniel
> >>
> > In this case I'm trying to exercise new API functions in libdrm which
> > wrap the GETPARAM ioctl. Would you rather me bypass the wrapper to
> > avoid requiring updated libdrm? I can do that, but it fails to test the
> > complete path that client would use.
> 
> 
> Am I missing something, or does 2.4.60 not exist yet?
> 
> That said dependency bumps for igt seem like less of an issue than
> dependency bumps for mesa..  I mean if you are using igt you are
> probably on the latest anyways..  I'm not sure why Daniel is so
> concerned about that..
> 
> (but dependency bumps to something that doesn't exist yet should
> perhaps be avoided)

I'd like to avoid massive depency loops for igt tests so that I can merge
the testcase right when the patches land in -nightly. Otherwise there's
always a small delay involved where regression can creep in. Also if I
have to update libdrm every time I update igt that's annoying since
without that I don't have to install/update anything at all - I run igt
in-place. And we've used the LOCAL_ prefixes for pretty much every abi
addition in igt tests thus far.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] tests: Add test for pipe B and C interactions in IVB

[Daniel Vetter]

On Tue, Mar 10, 2015 at 02:35:56PM +0200, Ander Conselvan de Oliveira wrote:
> This actually only works if the machine is setup properly. It requires
> that:
> 
> - the hardware to have at least two connected outpus;
> - the defatult mode on those outputs needs to be big enough;
> - the attached monitors need to support 10 bpc.
> 
> This might generate spurius results if the connected output doesn't support
> 10bpc or a large enough mode.
> 
> ---
> I used this to test the changes that affect pipe B and C interactions on
> IVB. I'm not really sure how to turn into a proper igt test though, because
> of the requirements on the outputs in use.

Sprinkle lots of igt_require(condition) over the testcase. In this case a
bit tricky since we'd need to look at the edid to check that there's a
10bpc screeen. Maybe use as an alternative plan just a higher resolution
and only check for the fdi per-lane dotclock limit? That should be more
reliable since for external outputs we start out with at least 8bpc
everywhere before deciding to dither down.
-Daniel

> 
> 
> ---
>  tests/Makefile.sources|   1 +
>  tests/kms_pipe_b_c_dpms.c | 173 
> ++
>  2 files changed, 174 insertions(+)
>  create mode 100644 tests/kms_pipe_b_c_dpms.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 9ab0ff4..9e43a64 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -73,6 +73,7 @@ TESTS_progs_M = \
>   kms_nuclear \
>   kms_pipe_crc_basic \
>   kms_plane \
> + kms_pipe_b_c_dpms \
>   kms_psr_sink_crc \
>   kms_render \
>   kms_rotation_crc \
> diff --git a/tests/kms_pipe_b_c_dpms.c b/tests/kms_pipe_b_c_dpms.c
> new file mode 100644
> index 000..a5ed761
> --- /dev/null
> +++ b/tests/kms_pipe_b_c_dpms.c
> @@ -0,0 +1,173 @@
> +/*
> + * Copyright © 2015 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.
> + *
> + * Authors:
> + *   Ander Conselvan de Oliveira 
> + */
> +
> +#include "drmtest.h"
> +#include "igt_kms.h"
> +#include "intel_chipset.h"
> +
> +typedef struct {
> + int drm_fd;
> + igt_display_t display;
> +} data_t;
> +
> +static int
> +set_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output, int bpc)
> +{
> + igt_plane_t *primary;
> + drmModeModeInfo *mode;
> + struct igt_fb fb;
> + int fb_id;
> + uint32_t format;
> +
> + if (bpc == 8)
> + format = DRM_FORMAT_XRGB;
> + else if (bpc == 10)
> + format = DRM_FORMAT_XRGB2101010;
> + else
> + igt_fail(1);
> +
> + igt_output_set_pipe(output, pipe);
> +
> + /* FIXME: we need a big mode */
> + mode = igt_output_get_mode(output);
> + primary = igt_output_get_plane(output, 0);
> +
> + fb_id = igt_create_color_fb(data->drm_fd,
> + mode->hdisplay, mode->vdisplay,
> + format, I915_TILING_NONE,
> + 1.0, 1.0, 1.0, &fb);
> + igt_assert(fb_id >= 0);
> +
> + igt_plane_set_fb(primary, &fb);
> + return igt_display_try_commit2(&data->display, COMMIT_LEGACY);
> +}
> +
> +static int
> +set_big_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
> +{
> + return set_mode_on_pipe(data, pipe, output, 10);
> +}
> +
> +static int
> +set_normal_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
> +{
> + return set_mode_on_pipe(data, pipe, output, 8);
> +}
> +
> +static void
> +find_outputs(data_t *data, igt_output_t **output1, igt_output_t **output2)
> +{
> + int count = 0;
> + igt_output_t *output;
> +
> + *output1 = NULL;
> + *output2 = NULL;
> +
> + for_each_connected_output(&data->display, output) {
> + if (!(*output1))
> + *output1 = output;
> + else if (!(*o

Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept

[Daniel Vetter]

On Tue, Mar 10, 2015 at 02:32:34PM +0200, Ander Conselvan de Oliveira wrote:
> Remove the global modeset resource function that would disable the
> bifurcation bit, and instead enable/disable it when enabling or
> disabling the pch transcoder. The checks before the mode set should
> ensure that the configuration is valid, so it should be safe to do it
> so.
> 
> Signed-off-by: Ander Conselvan de Oliveira 
> 
> ---
> 
> On Mon, 2015-03-09 at 17:21 +0100, Daniel Vetter wrote:
> > With atomic we need to probably look at crtc_state->mode_changed. As an
> > interim solution we have the same information available in the
> > modeset_pipes bitmask. I think replacing the check for intel_crtc->active
> > with !(modeset_pipes & (1 
> I looked into that solution, but involves passing modeset_pipes way down
> into the call stack, so I started looking for a different solution. Do you
> see any caveat with this approach?

Moving things to the disable hook would be great since that's yet another
special case remove. It wasnt' possible back when I've done this, and I
think the reason was that we still had a ->mode_set callback before the
crtc_enable hook. But that's all gone now, so as long as the bifurcate
update is done early enough I think this'll work.

Maybe discuss this with Ville locally - he provided all the feedback for
my original patch too? Random bikesheds below, I definitely need to think
about this more.

Cheers, Daniel

> 
> Thanks,
> Ander
> 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 60 
> +---
>  1 file changed, 21 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 4008bf4..eca5a41 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -101,6 +101,8 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
>   const struct intel_crtc_state *pipe_config);
>  static void intel_begin_crtc_commit(struct drm_crtc *crtc);
>  static void intel_finish_crtc_commit(struct drm_crtc *crtc);
> +static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc 
> *intel_crtc,
> + bool pipe_active);

Imo just move the functions instead of forward declarations.

>  
>  static struct intel_encoder *intel_find_encoder(struct intel_connector 
> *connector, int pipe)
>  {
> @@ -1956,6 +1958,8 @@ static void ironlake_disable_pch_transcoder(struct 
> drm_i915_private *dev_priv,
>   enum pipe pipe)
>  {
>   struct drm_device *dev = dev_priv->dev;
> + struct intel_crtc *crtc =
> + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
>   uint32_t reg, val;
>  
>   /* FDI relies on the transcoder */
> @@ -1980,6 +1984,9 @@ static void ironlake_disable_pch_transcoder(struct 
> drm_i915_private *dev_priv,
>   val &= ~TRANS_CHICKEN2_TIMING_OVERRIDE;
>   I915_WRITE(reg, val);
>   }
> +
> + if (IS_IVYBRIDGE(dev))
> + ivybridge_update_fdi_bc_bifurcation(crtc, false);
>  }
>  
>  static void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv)
> @@ -3153,32 +3160,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc 
> *crtc)
>   return crtc->base.state->enable && crtc->config->has_pch_encoder;
>  }
>  
> -static void ivb_modeset_global_resources(struct drm_device *dev)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *pipe_B_crtc =
> - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
> - struct intel_crtc *pipe_C_crtc =
> - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]);
> - uint32_t temp;
> -
> - /*
> -  * When everything is off disable fdi C so that we could enable fdi B
> -  * with all lanes. Note that we don't care about enabled pipes without
> -  * an enabled pch encoder.
> -  */
> - if (!pipe_has_enabled_pch(pipe_B_crtc) &&
> - !pipe_has_enabled_pch(pipe_C_crtc)) {
> - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
> - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
> -
> - temp = I915_READ(SOUTH_CHICKEN1);
> - temp &= ~FDI_BC_BIFURCATION_SELECT;
> - DRM_DEBUG_KMS("disabling fdi C rx\n");
> - I915_WRITE(SOUTH_CHICKEN1, temp);
> - }
> -}
> -
>  /* The FDI link training functions for ILK/Ibexpeak. */
>  static void ironlake_fdi_link_train(struct drm_crtc *crtc)
>  {
> @@ -3834,41 +3815,44 @@ static void 
> ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc,
>  I915_READ(VSYNCSHIFT(cpu_transcoder)));
>  }
>  
> -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
> +static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev, bool 
> enable)

enable feels misnamed now. Maybe s/enable/set/ instead?

>  {

Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept

[Daniel Vetter]

On Tue, Mar 10, 2015 at 03:03:59PM +0200, Ville Syrjälä wrote:
> On Tue, Mar 10, 2015 at 02:32:34PM +0200, Ander Conselvan de Oliveira wrote:
> > Remove the global modeset resource function that would disable the
> > bifurcation bit, and instead enable/disable it when enabling or
> > disabling the pch transcoder. The checks before the mode set should
> > ensure that the configuration is valid, so it should be safe to do it
> > so.
> > 
> > Signed-off-by: Ander Conselvan de Oliveira 
> > 
> > ---
> > 
> > On Mon, 2015-03-09 at 17:21 +0100, Daniel Vetter wrote:
> > > With atomic we need to probably look at crtc_state->mode_changed. As an
> > > interim solution we have the same information available in the
> > > modeset_pipes bitmask. I think replacing the check for intel_crtc->active
> > > with !(modeset_pipes & (1 > 
> > I looked into that solution, but involves passing modeset_pipes way down
> > into the call stack, so I started looking for a different solution. Do you
> > see any caveat with this approach?
> > 
> > Thanks,
> > Ander
> > 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 60 
> > +---
> >  1 file changed, 21 insertions(+), 39 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 4008bf4..eca5a41 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -101,6 +101,8 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
> > const struct intel_crtc_state *pipe_config);
> >  static void intel_begin_crtc_commit(struct drm_crtc *crtc);
> >  static void intel_finish_crtc_commit(struct drm_crtc *crtc);
> > +static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc 
> > *intel_crtc,
> > +   bool pipe_active);
> >  
> >  static struct intel_encoder *intel_find_encoder(struct intel_connector 
> > *connector, int pipe)
> >  {
> > @@ -1956,6 +1958,8 @@ static void ironlake_disable_pch_transcoder(struct 
> > drm_i915_private *dev_priv,
> > enum pipe pipe)
> >  {
> > struct drm_device *dev = dev_priv->dev;
> > +   struct intel_crtc *crtc =
> > +   to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> > uint32_t reg, val;
> >  
> > /* FDI relies on the transcoder */
> > @@ -1980,6 +1984,9 @@ static void ironlake_disable_pch_transcoder(struct 
> > drm_i915_private *dev_priv,
> > val &= ~TRANS_CHICKEN2_TIMING_OVERRIDE;
> > I915_WRITE(reg, val);
> > }
> > +
> > +   if (IS_IVYBRIDGE(dev))
> > +   ivybridge_update_fdi_bc_bifurcation(crtc, false);
> >  }
> >  
> >  static void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv)
> > @@ -3153,32 +3160,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc 
> > *crtc)
> > return crtc->base.state->enable && crtc->config->has_pch_encoder;
> >  }
> >  
> > -static void ivb_modeset_global_resources(struct drm_device *dev)
> > -{
> > -   struct drm_i915_private *dev_priv = dev->dev_private;
> > -   struct intel_crtc *pipe_B_crtc =
> > -   to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
> > -   struct intel_crtc *pipe_C_crtc =
> > -   to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]);
> > -   uint32_t temp;
> > -
> > -   /*
> > -* When everything is off disable fdi C so that we could enable fdi B
> > -* with all lanes. Note that we don't care about enabled pipes without
> > -* an enabled pch encoder.
> > -*/
> > -   if (!pipe_has_enabled_pch(pipe_B_crtc) &&
> > -   !pipe_has_enabled_pch(pipe_C_crtc)) {
> > -   WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
> > -   WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
> > -
> > -   temp = I915_READ(SOUTH_CHICKEN1);
> > -   temp &= ~FDI_BC_BIFURCATION_SELECT;
> > -   DRM_DEBUG_KMS("disabling fdi C rx\n");
> > -   I915_WRITE(SOUTH_CHICKEN1, temp);
> > -   }
> > -}
> > -
> >  /* The FDI link training functions for ILK/Ibexpeak. */
> >  static void ironlake_fdi_link_train(struct drm_crtc *crtc)
> >  {
> > @@ -3834,41 +3815,44 @@ static void 
> > ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc,
> >I915_READ(VSYNCSHIFT(cpu_transcoder)));
> >  }
> >  
> > -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
> > +static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev, bool 
> > enable)
> >  {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > uint32_t temp;
> >  
> > temp = I915_READ(SOUTH_CHICKEN1);
> > -   if (temp & FDI_BC_BIFURCATION_SELECT)
> > +   if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable)
> > return;
> >  
> > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
> > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
> >  
> > -   temp |= FDI_BC_BIFURC

Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept

[shuang . he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: 
shuang...@intel.com)
Task id: 5925
-Summary-
Platform  Delta  drm-intel-nightly  Series Applied
PNV -2  282/282  280/282
ILK  308/308  308/308
SNB  307/307  307/307
IVB  375/375  375/375
BYT  294/294  294/294
HSW  385/385  385/385
BDW  315/315  315/315
-Detailed-
Platform  Testdrm-intel-nightly  Series 
Applied
 PNV  igt_gem_userptr_blits_minor-normal-sync  DMESG_WARN(2)PASS(3)  
DMESG_WARN(2)
*PNV  igt_gem_userptr_blits_minor-unsync-normal  PASS(4)  
DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 0/2] Confirm full SSEU enable on Gen9+

[jeff . mcgee]

From: Jeff McGee 

New IGT testing to cover the RC6/SSEU issue recently resolved on SKL.

http://lists.freedesktop.org/archives/intel-gfx/2015-February/060058.html

Jeff McGee (2):
  lib: Add media spin
  tests/pm_sseu: Create new test pm_sseu

 lib/Makefile.sources|   2 +
 lib/intel_batchbuffer.c |  24 +++
 lib/intel_batchbuffer.h |  22 ++
 lib/media_spin.c| 540 
 lib/media_spin.h|  39 
 tests/.gitignore|   1 +
 tests/Makefile.sources  |   1 +
 tests/pm_sseu.c | 373 +
 8 files changed, 1002 insertions(+)
 create mode 100644 lib/media_spin.c
 create mode 100644 lib/media_spin.h
 create mode 100644 tests/pm_sseu.c

-- 
2.3.0

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

Re: [Intel-gfx] [PATCH 5/9] drm/i915: Pass primary plane size to .update_primary_plane()

[Matt Roper]

On Tue, Mar 10, 2015 at 07:59:15PM +0200, Ville Syrjälä wrote:
> On Tue, Mar 10, 2015 at 10:10:40AM -0700, Matt Roper wrote:
> > On Tue, Mar 10, 2015 at 01:15:25PM +0200, ville.syrj...@linux.intel.com 
> > wrote:
> > > From: Ville Syrjälä 
> > > 
> > > In preparation to movable/resizable primary planes pass the clipped
> > > plane size to .update_primary_plane().
> > 
> > Personally I feel like it would make more sense to just completely kill
> > off .update_primary_plane() now rather than trying to evolve it.  We
> > already have an intel_plane->update_plane() function pointer which is
> > never set or called for non-sprites at the moment.  We could unify the
> > handling of low-level plane programming by just using that function
> > pointer for primary planes as well.
> 
> I want to kill it off as well, but that means either killing off
> set_base_atomic() or making it use the plane commit hook. I suppose we
> could hand craft a suitable plane state for it and just commit that
> without any checks or anything?

I'm not sure I follow your concern about set_base_atomic().  After your
patch series, it'll be calling

   dev_priv->display.update_primary_plane(crtc, fb, x, y,   


  0, 0, 


  intel_crtc->config->pipe_src_w,   


  intel_crtc->config->pipe_src_h);  



which basically directly programs the hardware for the primary plane and
doesn't do anything state-related.

It seems to me that just making that low-level call be:

   intel_plane = to_intel_plane(crtc->primary);
   intel_state = to_intel_plane_state(crtc->primary->state);

   intel_plane->update_plane(crtc->primary, crtc, fb, intel_fb_obj(fb),
 0, 0,
 intel_crtc->config->pipe_src_w,
 intel_crtc->config->pipe_src_h,
 x, y,
 drm_rect_width(intel_state->src),
 drm_rect_height(intel_state->src));

shouldn't make a difference; the implementation of
intel_plane->update_plane() would still be the same low-level register
programming without any state manipulation, the only difference being
that we get an extra pair of parameters containing source rect
width/height.

This would also allow us to point both primary and sprite planes at the
same function on SKL, which has two nearly-identical functions at the
moment for the lowest-level plane hardware programming.


Matt

> 
> > 
> > I wouldn't mind also seeing the name of that low-level entrypoint
> > renamed to something like 'update_hw_plane' to avoid confusion with the
> > drm_plane entrypoint...
> > 
> > 
> > Matt
> > 
> > > 
> > > Cc: Sonika Jindal 
> > > Signed-off-by: Ville Syrjälä 
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h  |  3 +-
> > >  drivers/gpu/drm/i915/intel_display.c | 63 
> > > 
> > >  2 files changed, 38 insertions(+), 28 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index b16c0a7..e99eef0 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -578,7 +578,8 @@ struct drm_i915_display_funcs {
> > > uint32_t flags);
> > >   void (*update_primary_plane)(struct drm_crtc *crtc,
> > >struct drm_framebuffer *fb,
> > > -  int x, int y);
> > > +  int x, int y,
> > > +  int crtc_w, int crtc_h);
> > >   void (*hpd_irq_setup)(struct drm_device *dev);
> > >   /* clock updates for mode set */
> > >   /* cursor updates */
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index fdc83f1..1a789f0 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -2482,7 +2482,8 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
> > >  
> > >  static void i9xx_update_primary_plane(struct drm_crtc *crtc,
> > > struct drm_framebuffer *fb,
> > > -   int x, int y)
> > > +   int x, int y,
> > > +   int crtc_w, int crtc_h)
> > >  {
> > >   struct drm_device *dev = 

Re: [Intel-gfx] [PATCH] drm/i915: Use FW_WM() macro for older gmch platforms too

[Jesse Barnes]

On 03/10/2015 08:02 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Use the FW_WM() macro from the VLV wm code to polish up the wm
> code for older gmch platforms.
> 
> Cc: Daniel Vetter 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  4 ++--
>  drivers/gpu/drm/i915/intel_pm.c | 42 
> +
>  2 files changed, 24 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8ff039d..793ed63 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4121,8 +4121,8 @@ enum skl_disp_power_wells {
>  #define   DSPFW_SPRITEB_MASK_VLV (0xff<<16) /* vlv/chv */
>  #define   DSPFW_CURSORA_SHIFT8
>  #define   DSPFW_CURSORA_MASK (0x3f<<8)
> -#define   DSPFW_PLANEC_SHIFT_OLD 0
> -#define   DSPFW_PLANEC_MASK_OLD  (0x7f<<0) /* pre-gen4 sprite C 
> */
> +#define   DSPFW_PLANEC_OLD_SHIFT 0
> +#define   DSPFW_PLANEC_OLD_MASK  (0x7f<<0) /* pre-gen4 sprite C 
> */
>  #define   DSPFW_SPRITEA_SHIFT0
>  #define   DSPFW_SPRITEA_MASK (0x7f<<0) /* g4x */
>  #define   DSPFW_SPRITEA_MASK_VLV (0xff<<0) /* vlv/chv */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8ac358d0..9ac9a2f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -301,6 +301,9 @@ static void chv_set_memory_pm5(struct drm_i915_private 
> *dev_priv, bool enable)
>   mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
> +#define FW_WM(value, plane) \
> + (((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK)
> +
>  void intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable)
>  {
>   struct drm_device *dev = dev_priv->dev;
> @@ -661,7 +664,7 @@ static void pineview_update_wm(struct drm_crtc 
> *unused_crtc)
>   pixel_size, latency->display_sr);
>   reg = I915_READ(DSPFW1);
>   reg &= ~DSPFW_SR_MASK;
> - reg |= wm << DSPFW_SR_SHIFT;
> + reg |= FW_WM(wm, SR);
>   I915_WRITE(DSPFW1, reg);
>   DRM_DEBUG_KMS("DSPFW1 register is %x\n", reg);
>  
> @@ -671,7 +674,7 @@ static void pineview_update_wm(struct drm_crtc 
> *unused_crtc)
>   pixel_size, latency->cursor_sr);
>   reg = I915_READ(DSPFW3);
>   reg &= ~DSPFW_CURSOR_SR_MASK;
> - reg |= (wm & 0x3f) << DSPFW_CURSOR_SR_SHIFT;
> + reg |= FW_WM(wm, CURSOR_SR);
>   I915_WRITE(DSPFW3, reg);
>  
>   /* Display HPLL off SR */
> @@ -680,7 +683,7 @@ static void pineview_update_wm(struct drm_crtc 
> *unused_crtc)
>   pixel_size, 
> latency->display_hpll_disable);
>   reg = I915_READ(DSPFW3);
>   reg &= ~DSPFW_HPLL_SR_MASK;
> - reg |= wm & DSPFW_HPLL_SR_MASK;
> + reg |= FW_WM(wm, HPLL_SR);
>   I915_WRITE(DSPFW3, reg);
>  
>   /* cursor HPLL off SR */
> @@ -689,7 +692,7 @@ static void pineview_update_wm(struct drm_crtc 
> *unused_crtc)
>   pixel_size, 
> latency->cursor_hpll_disable);
>   reg = I915_READ(DSPFW3);
>   reg &= ~DSPFW_HPLL_CURSOR_MASK;
> - reg |= (wm & 0x3f) << DSPFW_HPLL_CURSOR_SHIFT;
> + reg |= FW_WM(wm, HPLL_CURSOR);
>   I915_WRITE(DSPFW3, reg);
>   DRM_DEBUG_KMS("DSPFW3 register is %x\n", reg);
>  
> @@ -835,8 +838,6 @@ static bool g4x_compute_srwm(struct drm_device *dev,
> display, cursor);
>  }
>  
> -#define FW_WM(value, plane) \
> - (((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK)
>  #define FW_WM_VLV(value, plane) \
>   (((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK_VLV)
>  
> @@ -904,7 +905,6 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
>   dev_priv->wm.vlv = *wm;
>  }
>  
> -#undef FW_WM
>  #undef FW_WM_VLV
>  
>  static uint8_t vlv_compute_drain_latency(struct drm_crtc *crtc,
> @@ -1163,17 +1163,17 @@ static void g4x_update_wm(struct drm_crtc *crtc)
> plane_sr, cursor_sr);
>  
>   I915_WRITE(DSPFW1,
> -(plane_sr << DSPFW_SR_SHIFT) |
> -(cursorb_wm << DSPFW_CURSORB_SHIFT) |
> -(planeb_wm << DSPFW_PLANEB_SHIFT) |
> -(planea_wm << DSPFW_PLANEA_SHIFT));
> +FW_WM(plane_sr, SR) |
> +FW_WM(cursorb_wm, CURSORB) |
> +FW_WM(planeb_wm, PLANEB) |
> +FW_WM(planea_wm, PLANEA));
>   I915_WRITE(DSPFW2,
>  (I915_READ(DSPFW2) & ~DSPFW_CURSORA_MASK) |
> -(cursora_wm << DSPFW_CURSORA_SHIFT));
> +FW_WM(cursora_wm, CURSORA));
>   /* HPL

Re: [Intel-gfx] [PATCH 01/53] drm/i915: Remove ironlake rc6 support

[Tomas Elf]

On 05/03/2015 14:03, john.c.harri...@intel.com wrote:

From: John Harrison 

Apparently, this has never worked reliably and is currently disabled. Also, the
gains are not particularly impressive. Thus rather than try to keep unused code
from decaying and having to update it for other driver changes, it was decided
to simply remove it.

For: VIZ-5115
Signed-off-by: John Harrison 
---
  drivers/gpu/drm/i915/i915_debugfs.c  |   12 ---
  drivers/gpu/drm/i915/i915_drv.h  |3 -
  drivers/gpu/drm/i915/intel_display.c |2 -
  drivers/gpu/drm/i915/intel_drv.h |1 -
  drivers/gpu/drm/i915/intel_pm.c  |  155 --
  5 files changed, 173 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 68a1e6e..18e4900 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1846,18 +1846,6 @@ static int i915_context_status(struct seq_file *m, void 
*unused)
if (ret)
return ret;

-   if (dev_priv->ips.pwrctx) {
-   seq_puts(m, "power context ");
-   describe_obj(m, dev_priv->ips.pwrctx);
-   seq_putc(m, '\n');
-   }
-
-   if (dev_priv->ips.renderctx) {
-   seq_puts(m, "render context ");
-   describe_obj(m, dev_priv->ips.renderctx);
-   seq_putc(m, '\n');
-   }
-
list_for_each_entry(ctx, &dev_priv->context_list, link) {
if (!i915.enable_execlists &&
ctx->legacy_hw_ctx.rcs_state == NULL)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 798fa88..05f106d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1053,9 +1053,6 @@ struct intel_ilk_power_mgmt {

int c_m;
int r_t;
-
-   struct drm_i915_gem_object *pwrctx;
-   struct drm_i915_gem_object *renderctx;
  };

  struct drm_i915_private;
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 5c35098..51f7ed4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13904,8 +13904,6 @@ void intel_modeset_cleanup(struct drm_device *dev)

intel_fbc_disable(dev);

-   ironlake_teardown_rc6(dev);
-
mutex_unlock(&dev->struct_mutex);

/* flush any delayed tasks or pending work */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f4305be..b793558 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1229,7 +1229,6 @@ void intel_enable_gt_powersave(struct drm_device *dev);
  void intel_disable_gt_powersave(struct drm_device *dev);
  void intel_suspend_gt_powersave(struct drm_device *dev);
  void intel_reset_gt_powersave(struct drm_device *dev);
-void ironlake_teardown_rc6(struct drm_device *dev);
  void gen6_update_ring_freq(struct drm_device *dev);
  void gen6_rps_idle(struct drm_i915_private *dev_priv);
  void gen6_rps_boost(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 542cf68..0762572 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3539,41 +3539,6 @@ void intel_update_sprite_watermarks(struct drm_plane 
*plane,
   pixel_size, enabled, scaled);
  }

-static struct drm_i915_gem_object *
-intel_alloc_context_page(struct drm_device *dev)
-{
-   struct drm_i915_gem_object *ctx;
-   int ret;
-
-   WARN_ON(!mutex_is_locked(&dev->struct_mutex));
-
-   ctx = i915_gem_alloc_object(dev, 4096);
-   if (!ctx) {
-   DRM_DEBUG("failed to alloc power context, RC6 disabled\n");
-   return NULL;
-   }
-
-   ret = i915_gem_obj_ggtt_pin(ctx, 4096, 0);
-   if (ret) {
-   DRM_ERROR("failed to pin power context: %d\n", ret);
-   goto err_unref;
-   }
-
-   ret = i915_gem_object_set_to_gtt_domain(ctx, 1);
-   if (ret) {
-   DRM_ERROR("failed to set-domain on power context: %d\n", ret);
-   goto err_unpin;
-   }
-
-   return ctx;
-
-err_unpin:
-   i915_gem_object_ggtt_unpin(ctx);
-err_unref:
-   drm_gem_object_unreference(&ctx->base);
-   return NULL;
-}
-
  /**
   * Lock protecting IPS related data structures
   */
@@ -4990,124 +4955,6 @@ static void valleyview_enable_rps(struct drm_device 
*dev)
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
  }

-void ironlake_teardown_rc6(struct drm_device *dev)
-{
-   struct drm_i915_private *dev_priv = dev->dev_private;
-
-   if (dev_priv->ips.renderctx) {
-   i915_gem_object_ggtt_unpin(dev_priv->ips.renderctx);
-   drm_gem_object_unreference(&dev_priv->ips.renderctx->base);
-   dev_priv->ips.renderctx = NULL;
-   }
-
-   if (dev_priv->ips.pwrctx) {
-   

Re: [Intel-gfx] [PATCH 54/56] drm/i915: Rename 'do_execbuf' to 'execbuf_submit'

[Tomas Elf]

On 05/03/2015 13:57, john.c.harri...@intel.com wrote:

From: John Harrison 

The submission portion of the execbuffer code path was abstracted into a
function pointer indirection as part of the legacy vs execlist work. The two
implementation functions are called 'i915_gem_ringbuffer_submission' and
'intel_execlists_submission' but the pointer was called 'do_execbuf'. There is
already a 'i915_gem_do_execbuffer' function (which is what calls the pointer
indirection). The name of the pointer is therefore considered to be backwards
and should be changed.

This patch renames it to 'execbuf_submit' which is hopefully a bit clearer.

Signed-off-by: John Harrison 
---
  drivers/gpu/drm/i915/i915_drv.h|6 +++---
  drivers/gpu/drm/i915/i915_gem.c|4 ++--
  drivers/gpu/drm/i915/i915_gem_execbuffer.c |2 +-
  3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7c9e569..4e9a350 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1793,9 +1793,9 @@ struct drm_i915_private {
int (*alloc_request)(struct intel_engine_cs *ring,
 struct intel_context *ctx,
 struct drm_i915_gem_request **req_out);
-   int (*do_execbuf)(struct i915_execbuffer_params *params,
- struct drm_i915_gem_execbuffer2 *args,
- struct list_head *vmas);
+   int (*execbuf_submit)(struct i915_execbuffer_params *params,
+ struct drm_i915_gem_execbuffer2 *args,
+ struct list_head *vmas);
int (*init_rings)(struct drm_device *dev);
void (*cleanup_ring)(struct intel_engine_cs *ring);
void (*stop_ring)(struct intel_engine_cs *ring);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ecff3f7..27abc9d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4928,13 +4928,13 @@ int i915_gem_init(struct drm_device *dev)

if (!i915.enable_execlists) {
dev_priv->gt.alloc_request = intel_ring_alloc_request;
-   dev_priv->gt.do_execbuf = i915_gem_ringbuffer_submission;
+   dev_priv->gt.execbuf_submit = i915_gem_ringbuffer_submission;
dev_priv->gt.init_rings = i915_gem_init_rings;
dev_priv->gt.cleanup_ring = intel_cleanup_ring_buffer;
dev_priv->gt.stop_ring = intel_stop_ring_buffer;
} else {
dev_priv->gt.alloc_request = intel_logical_ring_alloc_request;
-   dev_priv->gt.do_execbuf = intel_execlists_submission;
+   dev_priv->gt.execbuf_submit = intel_execlists_submission;
dev_priv->gt.init_rings = intel_logical_rings_init;
dev_priv->gt.cleanup_ring = intel_logical_ring_cleanup;
dev_priv->gt.stop_ring = intel_logical_ring_stop;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index dfad66a..d969eb5 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1558,7 +1558,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
params->batch_obj   = batch_obj;
params->ctx = ctx;

-   ret = dev_priv->gt.do_execbuf(params, args, &eb->vmas);
+   ret = dev_priv->gt.execbuf_submit(params, args, &eb->vmas);

  err:
/*



Reviewed-by: Tomas Elf 

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

Re: [Intel-gfx] [PATCH] drm/i915: Optimistically spin for the request completion

[shuang . he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: 
shuang...@intel.com)
Task id: 5928
-Summary-
Platform  Delta  drm-intel-nightly  Series Applied
PNV -1  282/282  281/282
ILK  308/308  308/308
SNB  307/307  307/307
IVB  375/375  375/375
BYT  294/294  294/294
HSW  385/385  385/385
BDW  315/315  315/315
-Detailed-
Platform  Testdrm-intel-nightly  Series 
Applied
*PNV  igt_gem_userptr_blits_minor-unsync-normal  PASS(4)  
DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/9] drm/i915: Pass primary plane size to .update_primary_plane()

[sonika]

On Tuesday 10 March 2015 04:45 PM, ville.syrj...@linux.intel.com wrote:

From: Ville Syrjälä 

In preparation to movable/resizable primary planes pass the clipped
plane size to .update_primary_plane().

Cc: Sonika Jindal 
Signed-off-by: Ville Syrjälä 
---
  drivers/gpu/drm/i915/i915_drv.h  |  3 +-
  drivers/gpu/drm/i915/intel_display.c | 63 
  2 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b16c0a7..e99eef0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -578,7 +578,8 @@ struct drm_i915_display_funcs {
  uint32_t flags);
void (*update_primary_plane)(struct drm_crtc *crtc,
 struct drm_framebuffer *fb,
-int x, int y);
+int x, int y,
+int crtc_w, int crtc_h);
void (*hpd_irq_setup)(struct drm_device *dev);
/* clock updates for mode set */
/* cursor updates */
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index fdc83f1..1a789f0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2482,7 +2482,8 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
  
  static void i9xx_update_primary_plane(struct drm_crtc *crtc,

  struct drm_framebuffer *fb,
- int x, int y)
+ int x, int y,
+ int crtc_w, int crtc_h)
  {
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2517,20 +2518,16 @@ static void i9xx_update_primary_plane(struct drm_crtc 
*crtc,
if (INTEL_INFO(dev)->gen < 4) {
if (intel_crtc->pipe == PIPE_B)
dspcntr |= DISPPLANE_SEL_PIPE_B;
-
-   /* pipesrc and dspsize control the size that is scaled from,
-* which should always be the user's requested size.
-*/
-   I915_WRITE(DSPSIZE(plane),
-  ((intel_crtc->config->pipe_src_h - 1) << 16) |
-  (intel_crtc->config->pipe_src_w - 1));
+   I915_WRITE(DSPSIZE(plane), ((crtc_h - 1) << 16) | (crtc_w - 1));
I915_WRITE(DSPPOS(plane), 0);
} else if (IS_CHERRYVIEW(dev) && plane == PLANE_B) {
-   I915_WRITE(PRIMSIZE(plane),
-  ((intel_crtc->config->pipe_src_h - 1) << 16) |
-  (intel_crtc->config->pipe_src_w - 1));
+   I915_WRITE(PRIMSIZE(plane), ((crtc_h - 1) << 16) | (crtc_w - 
1));
I915_WRITE(PRIMPOS(plane), 0);
I915_WRITE(PRIMCNSTALPHA(plane), 0);
+   } else {
+   WARN_ONCE(crtc_w != intel_crtc->config->pipe_src_w ||
+ crtc_h != intel_crtc->config->pipe_src_h,
+ "primary plane size doesn't match pipe size\n");
}
  
  	switch (fb->pixel_format) {

@@ -2586,14 +2583,14 @@ static void i9xx_update_primary_plane(struct drm_crtc 
*crtc,
if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180)) {
dspcntr |= DISPPLANE_ROTATE_180;
  
-		x += (intel_crtc->config->pipe_src_w - 1);

-   y += (intel_crtc->config->pipe_src_h - 1);
+   x += (crtc_w - 1);
+   y += (crtc_h - 1);
  
  		/* Finding the last pixel of the last line of the display

data and adding to linear_offset*/
linear_offset +=
-   (intel_crtc->config->pipe_src_h - 1) * fb->pitches[0] +
-   (intel_crtc->config->pipe_src_w - 1) * pixel_size;
+   (crtc_h - 1) * fb->pitches[0] +
+   (crtc_w - 1) * pixel_size;
}
  
  	I915_WRITE(reg, dspcntr);

@@ -2611,7 +2608,8 @@ static void i9xx_update_primary_plane(struct drm_crtc 
*crtc,
  
  static void ironlake_update_primary_plane(struct drm_crtc *crtc,

  struct drm_framebuffer *fb,
- int x, int y)
+ int x, int y,
+ int crtc_w, int crtc_h)
  {
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2643,6 +2641,10 @@ static void ironlake_update_primary_plane(struct 
drm_crtc *crtc,
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
  
+	WARN_ONCE(crtc_w != intel_crtc->config->pipe_src_w ||

+ crtc_h != intel_crtc->config->pipe_src_h,
+ "primary plane size doesn't match pipe size\n");
+
switch (fb->pixel_form

Re: [Intel-gfx] [PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2

[Dave Airlie]

> +
> +#define fourcc_mod_code(vendor, val) \
> +   u64)DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | (val & 
> 0x00ffL))

eh, yeah no,

/home/airlied/kernel/drm-2.6/drivers/gpu/drm/i915/intel_pm.c: In
function ‘skl_wm_method2’:
/home/airlied/kernel/drm-2.6/drivers/gpu/drm/i915/intel_pm.c:2631:
warning: integer constant is too large for ‘long’ type
/home/airlied/kernel/drm-2.6/drivers/gpu/drm/i915/intel_pm.c:2632:
warning: integer constant is too large for ‘long’ type

I think you meant ULL here.

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

Re: [Intel-gfx] [PATCH] drm/i915: Add soft-pinning API for execbuffer

[Zhenyu Wang]

On 2015.03.06 09:44:07 +, Chris Wilson wrote:
> Userspace can pass in an offset that it presumes the object is located
> at. The kernel will then do its utmost to fit the object into that
> location. The assumption is that userspace is handling its own object
> locations (for example along with full-ppgtt) and that the kernel will
> rarely have to make space for the user's requests.
> 

Chris, would you add libdrm support for this? e.g beignet doesn't
handle exec object itself but use libdrm.

-- 
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

Re: [Intel-gfx] [PATCH v4 1/8] drm/i915: Introduce a PV INFO page structure for Intel GVT-g.

[Dave Airlie]

> +/*
> + * The following structure pages are defined in GEN MMIO space
> + * for virtualization. (One page for now)
> + */
> +#define VGT_MAGIC 0x4776544776544776   /* 'vGTvGTvG' */

one of my dusty 32-bit compilers dislikes this,

 CC [M]  drivers/gpu/drm/i915/i915_vgpu.o
/home/airlied/kernel/drm-2.6/drivers/gpu/drm/i915/i915_vgpu.c: In
function ‘i915_check_vgpu’:
/home/airlied/kernel/drm-2.6/drivers/gpu/drm/i915/i915_vgpu.c:73:
warning: integer constant is too large for ‘long’ type

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

Re: [Intel-gfx] [PATCH v2] drm/i915: Export total subslice and EU counts

[shuang . he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: 
shuang...@intel.com)
Task id: 5922
-Summary-
Platform  Delta  drm-intel-nightly  Series Applied
PNV  282/282  282/282
ILK  308/308  308/308
SNB  307/307  307/307
IVB  375/375  375/375
BYT  294/294  294/294
HSW  385/385  385/385
BDW  315/315  315/315
-Detailed-
Platform  Testdrm-intel-nightly  Series 
Applied
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 2/2] configure: Bump required libdrm version to 2.4.60

[Daniel Vetter]

On Mon, Mar 09, 2015 at 04:41:02PM -0700, jeff.mc...@intel.com wrote:
> From: Jeff McGee 
> 
> tests/core_getparams needs the new libdrm interfaces for
> querying subslice and EU counts.
> 
> For: VIZ-4636
> Signed-off-by: Jeff McGee 
> ---
>  configure.ac | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 16d6a2e..88a1c3d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -82,7 +82,7 @@ if test "x$GCC" = "xyes"; then
>  fi
>  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
>  
> -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.52 libdrm])
> +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.60 libdrm])

Please don't and instead copypaste the new structs/defines with a local_
prefix like we do it for all the other new igt testcases. Forcing libdrm
to get updated for igt all the time can get annoying fast.
-Daniel

>  PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
>  PKG_CHECK_MODULES(OVERLAY_XVLIB, [xv x11 xext dri2proto >= 2.6], 
> enable_overlay_xvlib=yes, enable_overlay_xvlib=no)
>  PKG_CHECK_MODULES(OVERLAY_XLIB, [cairo-xlib dri2proto >= 2.6], 
> enable_overlay_xlib=yes, enable_overlay_xlib=no)
> -- 
> 2.3.0
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PULL] topic/drm-misc

[Daniel Vetter]

Hi Dave,

Another pile of misc drm patches all over, mostly polish for atomic. Last
minute rebase was to avoid the broken merge.

Cheers, Daniel


The following changes since commit 3a656b54c8433ada6dd7ee6227854c48682d5e4d:

  drm/i915: Fix struct_mutex deadlock due to merge fumble (2015-03-10 08:39:05 
+1000)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/topic/drm-misc-2015-03-10

for you to fetch changes up to 7eb5f302bbe78b88da8b2008c502c1975e75db05:

  drm: Check in setcrtc if the primary plane supports the fb pixel format 
(2015-03-10 09:59:36 +0100)


Chris Wilson (1):
  drm: Lighten sysfs connector 'status'

Daniel Vetter (4):
  drm: Remove redundant code in the getencoder ioctl
  drm/atomic-helper: Fix kerneldoc for prepare_planes
  drm: Fixup racy refcounting in plane_force_disable
  drm/plane-helper: unexport drm_primary_helper_create_plane

Jani Nikula (4):
  drm/dp: indentation and ordering cleanups
  drm/dp: add DPCD definitions from eDP 1.2
  drm/dp: add DPCD definitions from DP 1.1 and 1.2a
  drm/dp: add DPCD definitions from eDP 1.4

Laurent Pinchart (2):
  drm: Share plane pixel format check code between legacy and atomic
  drm: Check in setcrtc if the primary plane supports the fb pixel format

Tvrtko Ursulin (2):
  drm: Complete moving rotation property to core
  drm/i915: Rotation property is now handled in DRM core

Yannick Guerrini (1):
  drm: Fix trivial typos in comments

 drivers/gpu/drm/drm_atomic.c  |  12 +--
 drivers/gpu/drm/drm_atomic_helper.c   |   4 +-
 drivers/gpu/drm/drm_crtc.c|  61 +++
 drivers/gpu/drm/drm_modes.c   |   4 +-
 drivers/gpu/drm/drm_plane_helper.c|  31 ++
 drivers/gpu/drm/drm_sysfs.c   |  61 +--
 drivers/gpu/drm/i2c/adv7511.c |   2 +-
 drivers/gpu/drm/i915/intel_atomic_plane.c |  24 +
 include/drm/drm_crtc.h|   4 +
 include/drm/drm_dp_helper.h   | 171 --
 include/drm/drm_plane_helper.h|   4 -
 11 files changed, 284 insertions(+), 94 deletions(-)

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Disable M2 frac division for integer case

[Daniel Vetter]

On Thu, Mar 05, 2015 at 07:30:57PM +0530, Vijay Purushothaman wrote:
> v2 : Handle M2 frac division for both M2 frac and int cases
> 
> v3 : Addressed Ville's review comments. Cleared the old bits for RMW
> 
> v4 : Fix feedfwd gain (Ville)
> 
> Signed-off-by: Vijay Purushothaman 
> Signed-off-by: Ville Syrjala 

sob != r-b. First is for having handled/forwarded/updated a patch, second
is review. And since I didn't see the r-b tag I thought these patches
still need more review, hence the delay in merging.

But all pulled into dinq now, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_reg.h  |1 +
>  drivers/gpu/drm/i915/intel_display.c |   14 ++
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 55143cb..8200e98 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1029,6 +1029,7 @@ enum skl_disp_power_wells {
>  #define  DPIO_CHV_FIRST_MOD  (0 << 8)
>  #define  DPIO_CHV_SECOND_MOD (1 << 8)
>  #define  DPIO_CHV_FEEDFWD_GAIN_SHIFT 0
> +#define  DPIO_CHV_FEEDFWD_GAIN_MASK  (0xF << 0)
>  #define CHV_PLL_DW3(ch) _PIPE(ch, _CHV_PLL_DW3_CH0, _CHV_PLL_DW3_CH1)
>  
>  #define _CHV_PLL_DW6_CH0 0x8018
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 7298796..c5a8725 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6131,6 +6131,7 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
>   enum dpio_channel port = vlv_pipe_to_channel(pipe);
>   u32 loopfilter, intcoeff;
>   u32 bestn, bestm1, bestm2, bestp1, bestp2, bestm2_frac;
> + u32 dpio_val;
>   int refclk;
>  
>   bestn = pipe_config->dpll.n;
> @@ -6139,6 +6140,7 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
>   bestm2 = pipe_config->dpll.m2 >> 22;
>   bestp1 = pipe_config->dpll.p1;
>   bestp2 = pipe_config->dpll.p2;
> + dpio_val = 0;
>  
>   /*
>* Enable Refclk and SSC
> @@ -6164,12 +6166,16 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
>   1 << DPIO_CHV_N_DIV_SHIFT);
>  
>   /* M2 fraction division */
> - vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW2(port), bestm2_frac);
> + if (bestm2_frac)
> + vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW2(port), bestm2_frac);
>  
>   /* M2 fraction division enable */
> - vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW3(port),
> -DPIO_CHV_FRAC_DIV_EN |
> -(2 << DPIO_CHV_FEEDFWD_GAIN_SHIFT));
> + dpio_val = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW3(port));
> + dpio_val &= ~(DPIO_CHV_FEEDFWD_GAIN_MASK | DPIO_CHV_FRAC_DIV_EN);
> + dpio_val |= (2 << DPIO_CHV_FEEDFWD_GAIN_SHIFT);
> + if (bestm2_frac)
> + dpio_val |= DPIO_CHV_FRAC_DIV_EN;
> + vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW3(port), dpio_val);
>  
>   /* Loop filter */
>   refclk = i9xx_get_refclk(crtc, 0);
> -- 
> 1.7.9.5
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Do both mt and gen6 style forcewake reset on ivb probe

[Jani Nikula]

On Fri, 27 Feb 2015, Chris Wilson  wrote:
> On Fri, Feb 27, 2015 at 06:11:09PM +0200, Mika Kuoppala wrote:
>> commit 05a2fb157e44 ("drm/i915: Consolidate forcewake code")
>> failed to take into account that we have used to reset both
>> the gen6 style and the multithreaded style forcewake registers.
>> This is due to fact that ivb can use either, depending on how the
>> bios has set up the machine.
>> 
>> Mimic the old semantics before we have determined the correct variety
>> and reset both before the ecobus probe.
>> 
>> Cc: Chris Wilson 
>> Cc: Huang Ying 
>> Signed-off-by: Mika Kuoppala 
>
> Lgtm, do we have a reference for the testing scenario that blew up?
> Reviewed-by: Chris Wilson 

Pushed to drm-intel-fixes, thanks for the patch and review.

BR,
Jani.

> -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

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Make WAIT_IOCTL negative timeouts be indefinite again

[Jani Nikula]

On Mon, 09 Mar 2015, Chris Wilson  wrote:
> On Fri, Mar 06, 2015 at 05:48:26PM +0100, Daniel Vetter wrote:
>> On Fri, Mar 06, 2015 at 08:54:35AM +, Chris Wilson wrote:
>> > On Thu, Mar 05, 2015 at 01:27:43PM +0100, Daniel Vetter wrote:
>> > > On Wed, Mar 04, 2015 at 06:09:26PM +, Chris Wilson wrote:
>> > > > This fixes a regression from
>> > > > 
>> > > > commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
>> > > > Author: Thomas Gleixner 
>> > > > Date:   Wed Jul 16 21:05:06 2014 +
>> > > > 
>> > > > drm: i915: Use nsec based interfaces
>> > > > 
>> > > > that made a negative timeout return immediately rather than the
>> > > > previously defined behaviour of waiting indefinitely.
>> > > > 
>> > > > Signed-off-by: Chris Wilson 
>> > > > Cc: Daniel Vetter 
>> > > > Cc: Ben Widawsky 
>> > > > Cc: Kristian Høgsberg 
>> > > > Cc: sta...@vger.kernel.org
>> > > 
>> > > Do you have the igt for this too? I think an wait while the buffer should
>> > > be busy with a negative timeout is all that's needed to exercise this.
>> > 
>> > Done.
>> > Testcase: igt/gem_wait
>> 
>> Thanks a lot.
>> 
>> Reviewed-by: Daniel Vetter 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89494

Pushed to drm-intel-fixes, thanks for the patch and review.

BR,
Jani.

> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: use in_interrupt() not in_irq() to check context

[Jani Nikula]

On Fri, 06 Mar 2015, Daniel Vetter  wrote:
> On Fri, Mar 06, 2015 at 03:34:26PM +, Dave Gordon wrote:
>> The kernel in_irq() function tests for hard-IRQ context only, so if a
>> system is run with the kernel 'threadirqs' option selected, the test in
>> intel_check_page_flip() generates lots of warnings, because then it gets
>> called in soft-IRQ context.
>> 
>> We can instead use in_interrupt() which allows for either type of
>> interrupt, while still detecting and complaining about misuse of the
>> page flip code if it is ever called from non-interrupt context.
>> 
>> Signed-off-by: Dave Gordon 
>
> Makes sense.
>
> Reviewed-by: Daniel Vetter 
> Cc: sta...@vger.kernel.org

Pushed to drm-intel-fixes, thanks for the patch and review. Also added

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89321

BR,
Jani.

>
> Cheers, Daniel
>> ---
>>  drivers/gpu/drm/i915/intel_display.c |2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 43d3575..73213a7 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -9817,7 +9817,7 @@ void intel_check_page_flip(struct drm_device *dev, int 
>> pipe)
>>  struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
>>  struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>  
>> -WARN_ON(!in_irq());
>> +WARN_ON(!in_interrupt());
>>  
>>  if (crtc == NULL)
>>  return;
>> -- 
>> 1.7.9.5
>> 
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@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

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4 10/12] drm/i915: Program PFI credits for VLV

[Purushothaman, Vijay A]

On 3/6/2015 12:49 AM, ville.syrj...@linux.intel.com wrote:

From: Vidya Srinivas 

PFI credit programming is required when CD clock (related to data flow from
display pipeline to end display) is greater than CZ clock (related to data
flow from memory to display plane). This programming should be done when all
planes are OFF to avoid intermittent hangs while accessing memory even from
different Gfx units (not just display).

If cdclk/czclk >=1, PFI credits could be set as any number. To get better
performance, larger PFI credit can be assigned to PND. Otherwise if
cdclk/czclk<1, the default PFI credit of 8 should be set.

v2:
 - Change log to lower log level instead of DRM_ERROR
 - Change function name to valleyview_program_pfi_credits
 - Move program PFI credits to modeset_init instead of intel_set_mode
 - Change magic numbers to logical constants

[vsyrjala v3:
  - only program in response to cdclk update
  - program the credits also when cdclk
Signed-off-by: Gajanan Bhat 
Signed-off-by: Vandana Kannan 
Signed-off-by: Ville Syrjälä 
---


Reviewed-by: Vijay Purushothaman 

Thanks,
Vijay


  drivers/gpu/drm/i915/i915_reg.h  |  8 
  drivers/gpu/drm/i915/intel_display.c | 38 
  2 files changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9f98384..145f0d4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2061,6 +2061,14 @@ enum skl_disp_power_wells {
  #define   CDCLK_FREQ_SHIFT4
  #define   CDCLK_FREQ_MASK (0x1f << CDCLK_FREQ_SHIFT)
  #define   CZCLK_FREQ_MASK 0xf
+
+#define GCI_CONTROL(VLV_DISPLAY_BASE + 0x650C)
+#define   PFI_CREDIT_63(9 << 28) /* chv only */
+#define   PFI_CREDIT_31(8 << 28) /* chv only */
+#define   PFI_CREDIT(x)(((x) - 8) << 28) /* 8-15 */
+#define   PFI_CREDIT_RESEND(1 << 27)
+#define   VGA_FAST_MODE_DISABLE(1 << 14)
+
  #define GMBUSFREQ_VLV (VLV_DISPLAY_BASE + 0x6510)
  
  /*

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 3fe9598..29ee72d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4987,6 +4987,42 @@ static void valleyview_modeset_global_pipes(struct 
drm_device *dev,
*prepare_pipes |= (1 << intel_crtc->pipe);
  }
  
+static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)

+{
+   unsigned int credits, default_credits;
+
+   if (IS_CHERRYVIEW(dev_priv))
+   default_credits = PFI_CREDIT(12);
+   else
+   default_credits = PFI_CREDIT(8);
+
+   if (DIV_ROUND_CLOSEST(dev_priv->vlv_cdclk_freq, 1000) >= 
dev_priv->rps.cz_freq) {
+   /* CHV suggested value is 31 or 63 */
+   if (IS_CHERRYVIEW(dev_priv))
+   credits = PFI_CREDIT_31;
+   else
+   credits = PFI_CREDIT(15);
+   } else {
+   credits = default_credits;
+   }
+
+   /*
+* WA - write default credits before re-programming
+* FIXME: should we also set the resend bit here?
+*/
+   I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE |
+  default_credits);
+
+   I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE |
+  credits | PFI_CREDIT_RESEND);
+
+   /*
+* FIXME is this guaranteed to clear
+* immediately or should we poll for it?
+*/
+   WARN_ON(I915_READ(GCI_CONTROL) & PFI_CREDIT_RESEND);
+}
+
  static void valleyview_modeset_global_resources(struct drm_device *dev)
  {
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -5010,6 +5046,8 @@ static void valleyview_modeset_global_resources(struct 
drm_device *dev)
else
valleyview_set_cdclk(dev, req_cdclk);
  
+		vlv_program_pfi_credits(dev_priv);

+
intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
}
  }


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

Re: [Intel-gfx] [PATCH 1/3] drm/i915/skl: Allow universal planes to position

[Daniel Vetter]

On Tue, Mar 10, 2015 at 09:23:54AM +0530, sonika wrote:
> 
> On Monday 09 March 2015 02:16 PM, Daniel Vetter wrote:
> >On Mon, Mar 09, 2015 at 08:33:27AM +0530, sonika wrote:
> >>On Thursday 05 March 2015 06:24 PM, Daniel Vetter wrote:
> >>>On Thu, Mar 05, 2015 at 02:51:26PM +0530, Sonika Jindal wrote:
> Signed-off-by: Sonika Jindal 
> >>>Imo this needs a little more commit message, and more important it needs
> >>>igt test coverage. Best approach there is probably to take the plane test
> >>>we have already and extend it to the primary plane.
> >>>-Daniel
> >>This is just to take care of the case when the size of the fb is smaller
> >>than the crtc.
> >>I have extended the rotation test (yet to be posted), to create a smaller
> >>primary plane fb to be used for 90/270 rotation.
> >>
> >>Since we still set position to 0 for primary plane, I did not add any test
> >>case for positioning of primary plane.
> >>That can be added as a separate activity when positioning support is added.
> >>Right now this is just to allow smaller fb for primary plane which is
> >>possible with universal planes gen >=9.
> >Through universal planes it's already possible to position any plane
> >anywhere, and this code is all that makes sure this doesn't happen for the
> >primary plane. Since you've just changed that I think this needs a
> >testcase in igt.
> >
> >Or maybe I missed something and it's indeed not yet possible to do this?
> >-Daniel
> Yes, it isn't possible yet. We set the position to 0 in
> skylake_update_primary_plane.

Hm indeed. But with your patch we no longer reject plane updates for the
primary plane which aren't positioned at 0,0. So there's now a bug. I
guess either go the full way towards implementing universal plane support
on skl (and fix up the hw programming) or we need to wait with this patch
a bit more.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Make sure we invalidate frontbuffer on fbcon.

[shuang . he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: 
shuang...@intel.com)
Task id: 5923
-Summary-
Platform  Delta  drm-intel-nightly  Series Applied
PNV  282/282  282/282
ILK  308/308  308/308
SNB  307/307  307/307
IVB  375/375  375/375
BYT  294/294  294/294
HSW  385/385  385/385
BDW  315/315  315/315
-Detailed-
Platform  Testdrm-intel-nightly  Series 
Applied
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 52/53] drm/i915: Remove the now obsolete 'outstanding_lazy_request'

[Daniel Vetter]

On Mon, Mar 09, 2015 at 11:51:26PM +, Tomas Elf wrote:
> On 19/02/2015 17:18, john.c.harri...@intel.com wrote:
> >From: John Harrison 
> >
> >The outstanding_lazy_request is no longer used anywhere in the driver.
> >Everything that was looking at it now has a request explicitly passed in 
> >from on
> >high. Everything that was relying upon behind the scenes is now explicitly
> >creating/passing/submitting it's own private request. Thus the OLR can be
> >removed.
> >
> 
> "Everything that was relying upon behind the scenes is now explicitly
> creating/passing/submitting it's ..."
> --->
> "Everything that was relying on it behind the scenes is now explicitly
> creating/passing/submitting its ..." ?
> 
> 
> >For: VIZ-5115
> >Signed-off-by: John Harrison 
> >---
> >  drivers/gpu/drm/i915/i915_gem.c|   16 +---
> >  drivers/gpu/drm/i915/i915_gem_execbuffer.c |4 +---
> >  drivers/gpu/drm/i915/intel_lrc.c   |6 ++
> >  drivers/gpu/drm/i915/intel_ringbuffer.c|   17 ++---
> >  drivers/gpu/drm/i915/intel_ringbuffer.h|4 
> >  5 files changed, 6 insertions(+), 41 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> >b/drivers/gpu/drm/i915/i915_gem.c
> >index 60f6671..8e7418b 100644
> >--- a/drivers/gpu/drm/i915/i915_gem.c
> >+++ b/drivers/gpu/drm/i915/i915_gem.c
> >@@ -1156,15 +1156,9 @@ i915_gem_check_wedge(struct i915_gpu_error *error,
> >  int
> >  i915_gem_check_olr(struct drm_i915_gem_request *req)
> >  {
> >-int ret;
> >-
> > WARN_ON(!mutex_is_locked(&req->ring->dev->struct_mutex));
> >
> >-ret = 0;
> >-if (req == req->ring->outstanding_lazy_request)
> >-ret = i915_add_request(req);
> >-
> >-return ret;
> >+return 0;
> >  }
> >
> >  static void fake_irq(unsigned long data)
> >@@ -2424,8 +2418,6 @@ int __i915_add_request(struct drm_i915_gem_request 
> >*request,
> > dev_priv = ring->dev->dev_private;
> > ringbuf = request->ringbuf;
> >
> >-WARN_ON(request != ring->outstanding_lazy_request);
> >-
> > request_start = intel_ring_get_tail(ringbuf);
> > /*
> >  * Emit any outstanding flushes - execbuf can fail to emit the flush
> >@@ -2486,7 +2478,6 @@ int __i915_add_request(struct drm_i915_gem_request 
> >*request,
> > }
> >
> > trace_i915_gem_request_add(request);
> >-ring->outstanding_lazy_request = NULL;
> >
> > i915_queue_hangcheck(ring->dev);
> >
> >@@ -2672,9 +2663,6 @@ static void i915_gem_reset_ring_cleanup(struct 
> >drm_i915_private *dev_priv,
> >
> > i915_gem_free_request(request);
> > }
> >-
> >-/* This may not have been flushed before the reset, so clean it now */
> >-i915_gem_request_assign(&ring->outstanding_lazy_request, NULL);
> >  }
> >
> >  void i915_gem_restore_fences(struct drm_device *dev)
> >@@ -3124,8 +3112,6 @@ int i915_gpu_idle(struct drm_device *dev)
> > }
> > }
> >
> >-WARN_ON(ring->outstanding_lazy_request);
> >-
> > ret = intel_ring_idle(ring);
> > if (ret)
> > return ret;
> >diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
> >b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >index 6a703e6..0eae592 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >@@ -1571,10 +1571,8 @@ err:
> >  * must be freed again. If it was submitted then it is being tracked
> >  * on the active request list and no clean up is required here.
> >  */
> >-if (ret && params->request) {
> >+if (ret && params->request)
> > i915_gem_request_unreference(params->request);
> >-ring->outstanding_lazy_request = NULL;
> >-}
> >
> > mutex_unlock(&dev->struct_mutex);
> >
> >diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> >b/drivers/gpu/drm/i915/intel_lrc.c
> >index 2911cf6..db63ea0 100644
> >--- a/drivers/gpu/drm/i915/intel_lrc.c
> >+++ b/drivers/gpu/drm/i915/intel_lrc.c
> >@@ -1032,8 +1032,7 @@ int intel_logical_ring_alloc_request(struct 
> >intel_engine_cs *ring,
> > if (!req_out)
> > return -EINVAL;
> >
> >-if ((*req_out = ring->outstanding_lazy_request) != NULL)
> >-return 0;
> >+*req_out = NULL;
> >
> > request = kzalloc(sizeof(*request), GFP_KERNEL);
> > if (request == NULL)
> >@@ -1067,7 +1066,7 @@ int intel_logical_ring_alloc_request(struct 
> >intel_engine_cs *ring,
> > i915_gem_context_reference(request->ctx);
> > request->ringbuf = ctx->engine[ring->id].ringbuf;
> >
> >-*req_out = ring->outstanding_lazy_request = request;
> >+*req_out = request;
> > return 0;
> >  }
> >
> >@@ -1393,7 +1392,6 @@ void intel_logical_ring_cleanup(struct intel_engine_cs 
> >*ring)
> >
> > intel_logical_ring_stop(ring);
> > WARN_ON((I915_READ_MODE(ring) & MODE_IDLE) == 0);
> >-i915_gem_request_assign(&ring->outstanding_lazy_request, NULL);
> >
> > if (ring->cleanup)
> 

Re: [Intel-gfx] [PATCH] drm/i915: Make sure we invalidate frontbuffer on fbcon.

[Daniel Vetter]

On Mon, Mar 09, 2015 at 05:57:07PM -0700, Rodrigo Vivi wrote:
> There are some cases like suspend/resume or dpms off/on sequences
> that can flush frontbuffer bits. In these cases features that relies
> on frontbuffer tracking can start working and user can stop getting
> screen updates on fbcon having impression the system is frozen.
> 
> So, let's make sure we also invalidate frontbuffer on fbdev blank.
> 
> v2: Daniel was right, backtrace didn't show other path than this blank
> one so let's make sure frontbuffer bits gets invalidate here instead of
> on random write operations that doesn't garantee we track all frontbuffer
> writes.
> 
> Signed-off-by: Rodrigo Vivi 
> ---
>  drivers/gpu/drm/i915/intel_fbdev.c | 36 +++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c 
> b/drivers/gpu/drm/i915/intel_fbdev.c
> index 234a699..324b160 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -71,6 +71,40 @@ static int intel_fbdev_set_par(struct fb_info *info)
>   return ret;
>  }
>  
> +static int intel_fbdev_blank(int blank, struct fb_info *info)
> +{
> + struct drm_fb_helper *fb_helper = info->par;
> + struct intel_fbdev *ifbdev =
> + container_of(fb_helper, struct intel_fbdev, helper);
> + int ret;
> +
> + ret = drm_fb_helper_blank(blank, info);
> +
> + if (ret == 0) {
> + /*
> +  * FIXME: After dpms off/on sequence on fbdev frontbuffer bits
> +  * gets flushed so, let's set to gtt domain again when
> +  * restoring the panel, at least while we don't have a
> +  * propper solution.
> +   */

No FIXME required, this imo _is_ the right fix. But we need a FIXME about
the panic locking inversion instead.

> + mutex_lock(&fb_helper->dev->struct_mutex);
> +
> + /*
> +  * There are some cases that can flush frontbuffer bits
> +  * while we are still on console. Like when planes gets
> +  * enabled/disabled on blank/unblank. So, let's be sure
> +  * the fb obj gets invalidated when touching blank function.
> +  * It is already on gtt domain, so we just invalidate it
> +  * making sure components trusting frontbuffer tracking
> +  * still gets invalidate after blank/unblank sequence.
> +  */

Since this mirros fbdev_set_par and we don't have a comment there I think
this one isn't needed. The commit message explains it all well already.

Merged the patche with the code comments exchanged, thanks.
-Daniel

> + intel_fb_obj_invalidate(ifbdev->fb->obj, NULL, ORIGIN_GTT);
> + mutex_unlock(&fb_helper->dev->struct_mutex);
> + }
> +
> + return ret;
> +}
> +
>  static struct fb_ops intelfb_ops = {
>   .owner = THIS_MODULE,
>   .fb_check_var = drm_fb_helper_check_var,
> @@ -79,7 +113,7 @@ static struct fb_ops intelfb_ops = {
>   .fb_copyarea = cfb_copyarea,
>   .fb_imageblit = cfb_imageblit,
>   .fb_pan_display = drm_fb_helper_pan_display,
> - .fb_blank = drm_fb_helper_blank,
> + .fb_blank = intel_fbdev_blank,
>   .fb_setcmap = drm_fb_helper_setcmap,
>   .fb_debug_enter = drm_fb_helper_debug_enter,
>   .fb_debug_leave = drm_fb_helper_debug_leave,
> -- 
> 2.1.0
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 09/12] drm/i915: Rewrite VLV/CHV watermark code

[Daniel Vetter]

On Thu, Mar 05, 2015 at 09:19:49PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Assuming the PND deadline mechanism works reasonably we should do
> memory requests as early as possible so that PND has schedule the
> requests more intelligently. Currently we're still calculating
> the watermarks as if VLV/CHV are identical to g4x, which isn't
> the case.
> 
> The current code also seems to calculate insufficient watermarks
> and hence we're seeing some underruns, especially on high resolution
> displays.
> 
> To fix it just rip out the current code and replace is with something
> that tries to utilize PND as efficiently as possible.
> 
> We now calculate the WM watermark to trigger when the FIFO still has
> 256us worth of data. 256us is the maximum deadline value supoorted by
> PND, so issuing memory requests earlier would mean we probably couldn't
> utilize the full FIFO as PND would attempt to return the data at
> least in at least 256us. We also clamp the watermark to at least 8
> cachelines as that's the magic watermark that enabling trickle feed
> would also impose. I'm assuming it matches some burst size.
> 
> In theory we could just enable trickle feed and ignore the WM values,
> except trickle feed doesn't work with max fifo mode anyway, so we'd
> still need to calculate the SR watermarks. It seems cleaner to just
> disable trickle feed and calculate all watermarks the same way. Also
> trickle feed wouldn't account for the 256us max deadline value, thoguh
> that may be a moot point in non-max fifo mode sicne the FIFOs are fairly
> small.
> 
> On VLV max fifo mode can be used with either primary or sprite planes.
> So the code now also checks all the planes (apart from the cursor)
> when calculating the SR plane watermark.
> 
> We don't have to worry about the WM1 watermarks since we're using the
> PND deadline scheme which means the hardware ignores WM1 values.
> 
> v2: Use plane->state->fb instead of plane->fb
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  11 ++
>  drivers/gpu/drm/i915/i915_reg.h |   4 +-
>  drivers/gpu/drm/i915/intel_pm.c | 330 
> +---
>  3 files changed, 188 insertions(+), 157 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5de69a0..b191b12 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1516,6 +1516,17 @@ struct ilk_wm_values {
>  
>  struct vlv_wm_values {
>   struct {
> + uint16_t primary;
> + uint16_t sprite[2];
> + uint8_t cursor;
> + } pipe[3];
> +
> + struct {
> + uint16_t plane;
> + uint8_t cursor;
> + } sr;
> +
> + struct {
>   uint8_t cursor;
>   uint8_t sprite[2];
>   uint8_t primary;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8178610..9f98384 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4127,7 +4127,7 @@ enum skl_disp_power_wells {
>  /* vlv/chv high order bits */
>  #define DSPHOWM  (VLV_DISPLAY_BASE + 0x70064)
>  #define   DSPFW_SR_HI_SHIFT  24
> -#define   DSPFW_SR_HI_MASK   (1<<24)
> +#define   DSPFW_SR_HI_MASK   (3<<24) /* 2 bits for chv, 1 for vlv */
>  #define   DSPFW_SPRITEF_HI_SHIFT 23
>  #define   DSPFW_SPRITEF_HI_MASK  (1<<23)
>  #define   DSPFW_SPRITEE_HI_SHIFT 22
> @@ -4148,7 +4148,7 @@ enum skl_disp_power_wells {
>  #define   DSPFW_PLANEA_HI_MASK   (1<<0)
>  #define DSPHOWM1 (VLV_DISPLAY_BASE + 0x70068)
>  #define   DSPFW_SR_WM1_HI_SHIFT  24
> -#define   DSPFW_SR_WM1_HI_MASK   (1<<24)
> +#define   DSPFW_SR_WM1_HI_MASK   (3<<24) /* 2 bits for chv, 1 
> for vlv */
>  #define   DSPFW_SPRITEF_WM1_HI_SHIFT 23
>  #define   DSPFW_SPRITEF_WM1_HI_MASK  (1<<23)
>  #define   DSPFW_SPRITEE_WM1_HI_SHIFT 22
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index bdb0f5d..497847c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -778,6 +778,55 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
>  (wm->ddl[pipe].sprite[0] << DDL_SPRITE_SHIFT(0)) |
>  (wm->ddl[pipe].primary << DDL_PLANE_SHIFT));
>  
> + I915_WRITE(DSPFW1,
> +((wm->sr.plane << DSPFW_SR_SHIFT) & DSPFW_SR_MASK) |
> +((wm->pipe[PIPE_B].cursor << DSPFW_CURSORB_SHIFT) & 
> DSPFW_CURSORB_MASK) |
> +((wm->pipe[PIPE_B].primary << DSPFW_PLANEB_SHIFT) & 
> DSPFW_PLANEB_MASK_VLV) |
> +((wm->pipe[PIPE_A].primary << DSPFW_PLANEA_SHIFT) & 
> DSPFW_PLANEA_MASK_VLV));
> + I915_WRITE(DSPFW2,
> +((wm->pipe[PIPE_A].sprite[1] << DSPFW_SPRITEB_SHIFT) & 
> DSPFW_SPRITEB_MASK_VLV) |
> +((wm-

Re: [Intel-gfx] [PATCH v4 10/12] drm/i915: Program PFI credits for VLV

[Daniel Vetter]

On Tue, Mar 10, 2015 at 03:35:19PM +0530, Purushothaman, Vijay A wrote:
> On 3/6/2015 12:49 AM, ville.syrj...@linux.intel.com wrote:
> >From: Vidya Srinivas 
> >
> >PFI credit programming is required when CD clock (related to data flow from
> >display pipeline to end display) is greater than CZ clock (related to data
> >flow from memory to display plane). This programming should be done when all
> >planes are OFF to avoid intermittent hangs while accessing memory even from
> >different Gfx units (not just display).
> >
> >If cdclk/czclk >=1, PFI credits could be set as any number. To get better
> >performance, larger PFI credit can be assigned to PND. Otherwise if
> >cdclk/czclk<1, the default PFI credit of 8 should be set.
> >
> >v2:
> > - Change log to lower log level instead of DRM_ERROR
> > - Change function name to valleyview_program_pfi_credits
> > - Move program PFI credits to modeset_init instead of intel_set_mode
> > - Change magic numbers to logical constants
> >
> >[vsyrjala v3:
> >  - only program in response to cdclk update
> >  - program the credits also when cdclk >  - add CHV bits
> >  v4:
> >  - Change CHV cdclk >
> >Signed-off-by: Vidya Srinivas 
> >Signed-off-by: Gajanan Bhat 
> >Signed-off-by: Vandana Kannan 
> >Signed-off-by: Ville Syrjälä 
> >---
> 
> Reviewed-by: Vijay Purushothaman 

Pulled in the remaining 4 patches from this series, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915: Prevent TLB error on first execution on SNB

[Daniel Vetter]

On Fri, Feb 13, 2015 at 02:35:59PM +, Chris Wilson wrote:
> Long ago I found that I was getting sporadic errors when booting SNB,
> with the symptom being that the first batch died with IPEHR != *ACTHD,
> typically caused by the TLB being invalid. These magically disappeared
> if I held the forcewake during the entire ring initialisation sequence.
> (It can probably be shortened to a short critical section, but the whole
> initialisation is full of register writes and so we would be taking and
> releasing forcewake almost continually, and so holding it over the
> entire sequence will probably be a net win!)
> 
> Note some of the kernels I encounted the issue already had the deferred
> forcewake release, so it is still relevant.
> 
> I know that there have been a few other reports with similar failure
> conditions on SNB, I think such as
> References: https://bugs.freedesktop.org/show_bug.cgi?id=80913
> 
> v2: Wrap i915_gem_init_hw() with its own security blanket as we take
> that path following resume and reset.
> 
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 18 --
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8d15c8110962..08450922f373 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4783,6 +4783,9 @@ i915_gem_init_hw(struct drm_device *dev)
>   if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
>   return -EIO;
>  
> + /* Double layer security blanket, see i915_gem_init() */
> + intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> +
>   if (dev_priv->ellc_size)
>   I915_WRITE(HSW_IDICR, I915_READ(HSW_IDICR) | IDIHASHMSK(0xf));
>  
> @@ -4815,7 +4818,7 @@ i915_gem_init_hw(struct drm_device *dev)
>   for_each_ring(ring, dev_priv, i) {
>   ret = ring->init_hw(ring);
>   if (ret)
> - return ret;
> + goto out;
>   }
>  
>   for (i = 0; i < NUM_L3_SLICES(dev); i++)
> @@ -4832,9 +4835,11 @@ i915_gem_init_hw(struct drm_device *dev)
>   DRM_ERROR("Context enable failed %d\n", ret);
>   i915_gem_cleanup_ringbuffer(dev);
>  
> - return ret;
> + goto out;
>   }
>  
> +out:
> + intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>   return ret;
>  }
>  
> @@ -4868,6 +4873,14 @@ int i915_gem_init(struct drm_device *dev)
>   dev_priv->gt.stop_ring = intel_logical_ring_stop;
>   }
>  
> + /* This is just a security blanket to placate dragons.
> +  * On some systems, we very sporadically observe that the first TLBs
> +  * used by the CS may be stale, despite us poking the TLB reset. If
> +  * we hold the forcewake during initialisation these problems
> +  * just magically go away.
> +  */
> + intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);

gem_init shouldn't ever touch the hw except through gem_init_hw. Do we
really need the double-layer here? Also the forcewake hack in the ring
init code should now be redundant, too.
-Daniel

> +
>   ret = i915_gem_init_userptr(dev);
>   if (ret)
>   goto out_unlock;
> @@ -4894,6 +4907,7 @@ int i915_gem_init(struct drm_device *dev)
>   }
>  
>  out_unlock:
> + intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>   mutex_unlock(&dev->struct_mutex);
>  
>   return ret;
> -- 
> 2.1.4
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/skl: Fix plane index in the colorkey vfuncs

[Daniel Vetter]

On Sat, Feb 14, 2015 at 12:26:46AM +, Damien Lespiau wrote:
> While the SKL versions of update_plane() and disable_plane() have been
> fixed before hitting upstream, the colorkey vfuncs have been left behind
> and so register writes for sprite 0 were landing on plane 0 (primary
> plane) instead of plane 1.
> 
> Signed-off-by: Damien Lespiau 

Sounds like we need an igt for this stuff asap, and then maybe also
converting this over to atomic props properly.
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> b/drivers/gpu/drm/i915/intel_sprite.c
> index f2d408d..9bd5e28 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -308,7 +308,7 @@ skl_update_colorkey(struct drm_plane *drm_plane,
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   struct intel_plane *intel_plane = to_intel_plane(drm_plane);
>   const int pipe = intel_plane->pipe;
> - const int plane = intel_plane->plane;
> + const int plane = intel_plane->plane + 1;
>   u32 plane_ctl;
>  
>   I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value);
> @@ -336,7 +336,7 @@ skl_get_colorkey(struct drm_plane *drm_plane,
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   struct intel_plane *intel_plane = to_intel_plane(drm_plane);
>   const int pipe = intel_plane->pipe;
> - const int plane = intel_plane->plane;
> + const int plane = intel_plane->plane + 1;
>   u32 plane_ctl;
>  
>   key->min_value = I915_READ(PLANE_KEYVAL(pipe, plane));
> -- 
> 1.8.3.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915: Prevent TLB error on first execution on SNB

[Chris Wilson]

On Tue, Mar 10, 2015 at 11:31:04AM +0100, Daniel Vetter wrote:
> On Fri, Feb 13, 2015 at 02:35:59PM +, Chris Wilson wrote:
> > Long ago I found that I was getting sporadic errors when booting SNB,
> > with the symptom being that the first batch died with IPEHR != *ACTHD,
> > typically caused by the TLB being invalid. These magically disappeared
> > if I held the forcewake during the entire ring initialisation sequence.
> > (It can probably be shortened to a short critical section, but the whole
> > initialisation is full of register writes and so we would be taking and
> > releasing forcewake almost continually, and so holding it over the
> > entire sequence will probably be a net win!)
> > 
> > Note some of the kernels I encounted the issue already had the deferred
> > forcewake release, so it is still relevant.
> > 
> > I know that there have been a few other reports with similar failure
> > conditions on SNB, I think such as
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=80913
> > 
> > v2: Wrap i915_gem_init_hw() with its own security blanket as we take
> > that path following resume and reset.
> > 
> > Signed-off-by: Chris Wilson 
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c | 18 --
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index 8d15c8110962..08450922f373 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4783,6 +4783,9 @@ i915_gem_init_hw(struct drm_device *dev)
> > if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
> > return -EIO;
> >  
> > +   /* Double layer security blanket, see i915_gem_init() */
> > +   intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> > +
> > if (dev_priv->ellc_size)
> > I915_WRITE(HSW_IDICR, I915_READ(HSW_IDICR) | IDIHASHMSK(0xf));
> >  
> > @@ -4815,7 +4818,7 @@ i915_gem_init_hw(struct drm_device *dev)
> > for_each_ring(ring, dev_priv, i) {
> > ret = ring->init_hw(ring);
> > if (ret)
> > -   return ret;
> > +   goto out;
> > }
> >  
> > for (i = 0; i < NUM_L3_SLICES(dev); i++)
> > @@ -4832,9 +4835,11 @@ i915_gem_init_hw(struct drm_device *dev)
> > DRM_ERROR("Context enable failed %d\n", ret);
> > i915_gem_cleanup_ringbuffer(dev);
> >  
> > -   return ret;
> > +   goto out;
> > }
> >  
> > +out:
> > +   intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> > return ret;
> >  }
> >  
> > @@ -4868,6 +4873,14 @@ int i915_gem_init(struct drm_device *dev)
> > dev_priv->gt.stop_ring = intel_logical_ring_stop;
> > }
> >  
> > +   /* This is just a security blanket to placate dragons.
> > +* On some systems, we very sporadically observe that the first TLBs
> > +* used by the CS may be stale, despite us poking the TLB reset. If
> > +* we hold the forcewake during initialisation these problems
> > +* just magically go away.
> > +*/
> > +   intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> 
> gem_init shouldn't ever touch the hw except through gem_init_hw. Do we
> really need the double-layer here?

There are register accesses before, so yes since that's how I tested
it...

> Also the forcewake hack in the ring
> init code should now be redundant, too.

I am of the opinion that they still serve documentary value. Unless you
have an assert_force_wake() handy.
-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 3/3] drm/i915: Don't assume primary & cursor are always on for wm calculation (v4)

[Tvrtko Ursulin]

On 03/09/2015 06:06 PM, Matt Roper wrote:

Current ILK-style watermark code assumes the primary plane and cursor
plane are always enabled.  This assumption, along with the combination
of two independent commits that got merged at the same time, results in
a NULL dereference.  The offending commits are:

 commit fd2d61341bf39d1054256c07d6eddd624ebc4241
 Author: Matt Roper 
 Date:   Fri Feb 27 10:12:01 2015 -0800

 drm/i915: Use plane->state->fb in watermark code (v2)

and

 commit 0fda65680e92545caea5be7805a7f0a617fb6c20
 Author: Tvrtko Ursulin 
 Date:   Fri Feb 27 15:12:35 2015 +

 drm/i915/skl: Update watermarks for Y tiling

The first commit causes us to use the FB from plane->state->fb rather
than the legacy plane->fb, which is updated a bit later in the process.

The second commit includes a change that now triggers watermark
reprogramming on primary plane enable/disable where we didn't have one
before (which wasn't really correct, but we had been getting lucky
because we always calculated as if the primary plane was on).

Together, these two commits cause the watermark calculation to
(properly) see plane->state->fb = NULL when we're in the process of
disabling the primary plane.  However the existing watermark code
assumes there's always a primary fb and tries to dereference it to find
out pixel format / bpp information.

The fix is to make ILK-style watermark calculation actually check the
true status of primary & cursor planes and adjust our watermark logic
accordingly.

v2: Update unchecked uses of state->fb for other platforms (pnv, skl,
 etc.).  Note that this is just a temporary fix.  Ultimately the
 useful information is going to be computed at check time and stored
 right in the state structures so that we don't have to figure this
 all out while we're supposed to be programming the watermarks.
 (caught by Tvrtko)

v3: Fix a couple copy/paste mistakes in SKL code. (Tvrtko)

v4: Only add FB checks for ILK/SKL codepaths.  Older platforms still use
 intel_crtc_active() and will shortcircuit out of watermark
 calculations before ever trying to dereference the primary plane's
 framebuffer.

Cc: Tvrtko Ursulin 
Reported-by: Michael Leuchtenburg 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89388
Signed-off-by: Matt Roper 
---
  drivers/gpu/drm/i915/intel_pm.c | 62 ++---
  1 file changed, 39 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a06a2c7..7566cec 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1962,13 +1962,25 @@ static void ilk_compute_wm_parameters(struct drm_crtc 
*crtc,
p->active = true;
p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
-   p->pri.bytes_per_pixel = crtc->primary->state->fb->bits_per_pixel / 8;
-   p->cur.bytes_per_pixel = 4;
+
+   if (crtc->primary->state->fb) {
+   p->pri.enabled = true;
+   p->pri.bytes_per_pixel =
+   crtc->primary->state->fb->bits_per_pixel / 8;
+   } else {
+   p->pri.enabled = false;
+   p->pri.bytes_per_pixel = 0;
+   }
+
+   if (crtc->cursor->state->fb) {
+   p->cur.enabled = true;
+   p->cur.bytes_per_pixel = 4;
+   } else {
+   p->cur.enabled = false;
+   p->cur.bytes_per_pixel = 0;
+   }
p->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;
-   /* TODO: for now, assume primary and cursor planes are always enabled. 
*/
-   p->pri.enabled = true;
-   p->cur.enabled = true;

drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
struct intel_plane *intel_plane = to_intel_plane(plane);
@@ -2734,27 +2746,31 @@ static void skl_compute_wm_pipe_parameters(struct 
drm_crtc *crtc,
p->pipe_htotal = 
intel_crtc->config->base.adjusted_mode.crtc_htotal;
p->pixel_rate = skl_pipe_pixel_rate(intel_crtc->config);

-   /*
-* For now, assume primary and cursor planes are always enabled.
-*/
-   p->plane[0].enabled = true;
-   p->plane[0].bytes_per_pixel =
-   crtc->primary->state->fb->bits_per_pixel / 8;
-   p->plane[0].horiz_pixels = intel_crtc->config->pipe_src_w;
-   p->plane[0].vert_pixels = intel_crtc->config->pipe_src_h;
-   p->plane[0].tiling = DRM_FORMAT_MOD_NONE;
fb = crtc->primary->state->fb;
-   /*
-* Framebuffer can be NULL on plane disable, but it does not
-* matter for watermarks if we assume no tiling in that case.
-