Re: [Intel-gfx] [PATCH 11/21] drm/i915/gtt: Introduce fill_page_dma()
On Tue, Jun 02, 2015 at 06:01:27PM +0300, Ville Syrjälä wrote:
On Tue, Jun 02, 2015 at 03:51:26PM +0100, Michel Thierry wrote:
On 5/22/2015 6:05 PM, Mika Kuoppala wrote:
When we setup page directories and tables, we point the entries
to a to the next level scratch structure. Make this generic
by introducing a fill_page_dma which maps and flushes. We also
need 32 bit variant for legacy gens.
v2: Fix flushes and handle valleyview (Ville)
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 71
+++--
1 file changed, 37 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index f747bd3..d020b5e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -330,6 +330,31 @@ static void cleanup_page_dma(struct drm_device *dev,
struct i915_page_dma *p)
memset(p, 0, sizeof(*p));
}
+static void fill_page_dma(struct drm_device *dev, struct i915_page_dma
*p,
+ const uint64_t val)
+{
+ int i;
+ uint64_t * const vaddr = kmap_atomic(p-page);
+
+ for (i = 0; i 512; i++)
+ vaddr[i] = val;
+
+ if (!HAS_LLC(dev) !IS_VALLEYVIEW(dev))
+ drm_clflush_virt_range(vaddr, PAGE_SIZE);
Cherryview returns true to IS_VALLEYVIEW().
You can use(!HAS_LLC IS_CHERRYVIEW) instead to flush in chv, but not
in vlv... But to make it bxt-proof, (!HAS_LLC INTEL_INFO(dev)-gen =
8) is probably better.
Has someone actually confirmed that BXT needs the clflush?
Ping on this one ... I'd like to know the answer ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 02/19] drm/i915: Clean up intel_atomic_setup_scalers slightly.
Get rid of a whole lot of ternary operators and assign the index
in scaler_id, instead of the id. They're the same thing.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_atomic.c | 21 +++--
drivers/gpu/drm/i915/intel_display.c | 2 --
drivers/gpu/drm/i915/intel_drv.h | 1 -
3 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c
b/drivers/gpu/drm/i915/intel_atomic.c
index 4df6d2d7a9c8..041bff504629 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -309,15 +309,23 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
/* walkthrough scaler_users bits and start assigning scalers */
for (i = 0; i sizeof(scaler_state-scaler_users) * 8; i++) {
int *scaler_id;
+ const char *name;
+ int idx;
/* skip if scaler not required */
if (!(scaler_state-scaler_users (1 i)))
continue;
if (i == SKL_CRTC_INDEX) {
+ name = CRTC;
+ idx = intel_crtc-base.base.id;
+
/* panel fitter case: assign as a crtc scaler */
scaler_id = scaler_state-scaler_id;
} else {
+ name = PLANE;
+ idx = plane-base.id;
+
if (!drm_state)
continue;
@@ -356,23 +364,16 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
for (j = 0; j intel_crtc-num_scalers; j++) {
if (!scaler_state-scalers[j].in_use) {
scaler_state-scalers[j].in_use = 1;
- *scaler_id =
scaler_state-scalers[j].id;
+ *scaler_id = j;
DRM_DEBUG_KMS(Attached scaler id %u.%u
to %s:%d\n,
- intel_crtc-pipe,
- i == SKL_CRTC_INDEX ?
scaler_state-scaler_id :
- plane_state-scaler_id,
- i == SKL_CRTC_INDEX ? CRTC :
PLANE,
- i == SKL_CRTC_INDEX ?
intel_crtc-base.base.id :
- plane-base.id);
+ intel_crtc-pipe, *scaler_id,
name, idx);
break;
}
}
}
if (WARN_ON(*scaler_id 0)) {
- DRM_DEBUG_KMS(Cannot find scaler for %s:%d\n,
- i == SKL_CRTC_INDEX ? CRTC : PLANE,
- i == SKL_CRTC_INDEX ?
intel_crtc-base.base.id:plane-base.id);
+ DRM_DEBUG_KMS(Cannot find scaler for %s:%d\n, name,
idx);
continue;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index d36684080987..956fb8423fff 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14059,8 +14059,6 @@ static void skl_init_scalers(struct drm_device *dev,
struct intel_crtc *intel_cr
for (i = 0; i intel_crtc-num_scalers; i++) {
intel_scaler = scaler_state-scalers[i];
intel_scaler-in_use = 0;
- intel_scaler-id = i;
-
intel_scaler-mode = PS_SCALER_MODE_DYN;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b28029a1c8f2..29d6031b19d8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -293,7 +293,6 @@ struct intel_initial_plane_config {
#define SKL_MAX_DST_H 4096
struct intel_scaler {
- int id;
int in_use;
uint32_t mode;
};
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 04/19] drm/i915: Move scaler setup to check crtc function, v2.
The scaler setup may add planes, but since they're unchanged we only
have to wait for primary flips. Also set planes_changed to indicate
at least 1 plane is modified.
Changes since v1:
- Instead of removing planes, do minimal validation needed.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_atomic.c | 17 +
drivers/gpu/drm/i915/intel_display.c | 15 +--
2 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c
b/drivers/gpu/drm/i915/intel_atomic.c
index 6ab71ea92819..d5afc2aa4ac7 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -100,14 +100,6 @@ int intel_atomic_check(struct drm_device *dev,
if (ret)
return ret;
- /*
-* FIXME: move to crtc atomic check function once this is
-* more atomic friendly.
-*/
- ret = intel_atomic_setup_scalers(dev, nuclear_crtc, crtc_state);
- if (ret)
- return ret;
-
return ret;
}
@@ -349,6 +341,15 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
plane-base.id);
return PTR_ERR(state);
}
+
+ /*
+* the plane is added after plane checks are
run,
+* but since this plane is unchanged just do the
+* minimum required validation.
+*/
+ if (plane-type == DRM_PLANE_TYPE_PRIMARY)
+ intel_crtc-atomic.wait_for_flips =
true;
+ crtc_state-base.planes_changed = true;
}
intel_plane = to_intel_plane(plane);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 29e761d9e0ec..e6837172afaf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6574,7 +6574,6 @@ static int intel_crtc_compute_config(struct intel_crtc
*crtc,
struct drm_device *dev = crtc-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_display_mode *adjusted_mode =
pipe_config-base.adjusted_mode;
- int ret;
/* FIXME should check pixel clock limits on all platforms */
if (INTEL_INFO(dev)-gen 4) {
@@ -6620,14 +6619,7 @@ static int intel_crtc_compute_config(struct intel_crtc
*crtc,
if (pipe_config-has_pch_encoder)
return ironlake_fdi_compute_config(crtc, pipe_config);
- /* FIXME: remove below call once atomic mode set is place and all crtc
-* related checks called from atomic_crtc_check function */
- ret = 0;
- DRM_DEBUG_KMS(intel_crtc = %p drm_state (pipe_config-base.state) =
%p\n,
- crtc, pipe_config-base.state);
- ret = intel_atomic_setup_scalers(dev, crtc, pipe_config);
-
- return ret;
+ return 0;
}
static int skylake_get_display_clock_speed(struct drm_device *dev)
@@ -11714,7 +11706,10 @@ static bool check_encoder_cloning(struct
drm_atomic_state *state,
static int intel_crtc_atomic_check(struct drm_crtc *crtc,
struct drm_crtc_state *crtc_state)
{
+ struct drm_device *dev = crtc-dev;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *pipe_config =
+ to_intel_crtc_state(crtc_state);
struct drm_atomic_state *state = crtc_state-state;
int idx = crtc-base.id;
bool mode_changed = needs_modeset(crtc_state);
@@ -11728,7 +11723,7 @@ static int intel_crtc_atomic_check(struct drm_crtc
*crtc,
[CRTC:%i] mismatch between state-active(%i) and
crtc-active(%i)\n,
idx, crtc-state-active, intel_crtc-active);
- return 0;
+ return intel_atomic_setup_scalers(dev, intel_crtc, pipe_config);
}
static const struct drm_crtc_helper_funcs intel_helper_funcs = {
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 07/19] drm/i915: Split plane updates of crtc-atomic into a helper, v2.
This makes it easier to verify that no changes are done when
calling this from crtc instead.
Changes since v1:
- Make intel_wm_need_update static and always check it.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_atomic_plane.c | 21 +--
drivers/gpu/drm/i915/intel_display.c | 275 ++
drivers/gpu/drm/i915/intel_drv.h | 8 +-
drivers/gpu/drm/i915/intel_sprite.c | 32 +---
4 files changed, 176 insertions(+), 160 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 86ba4b2c3a65..aa2128369a0a 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -114,6 +114,7 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
struct intel_crtc_state *crtc_state;
struct intel_plane *intel_plane = to_intel_plane(plane);
struct intel_plane_state *intel_state = to_intel_plane_state(state);
+ int ret;
crtc = crtc ? crtc : plane-crtc;
intel_crtc = to_intel_crtc(crtc);
@@ -160,20 +161,6 @@ static int intel_plane_atomic_check(struct drm_plane
*plane,
intel_state-clip.y2 =
crtc_state-base.active ? crtc_state-pipe_src_h : 0;
- /*
-* Disabling a plane is always okay; we just need to update
-* fb tracking in a special way since cleanup_fb() won't
-* get called by the plane helpers.
-*/
- if (state-fb == NULL plane-state-fb != NULL) {
- /*
-* 'prepare' is never called when plane is being disabled, so
-* we need to handle frontbuffer tracking as a special case
-*/
- intel_crtc-atomic.disabled_planes |=
- (1 drm_plane_index(plane));
- }
-
if (state-fb intel_rotation_90_or_270(state-rotation)) {
if (!(state-fb-modifier[0] == I915_FORMAT_MOD_Y_TILED ||
state-fb-modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
@@ -198,7 +185,11 @@ static int intel_plane_atomic_check(struct drm_plane
*plane,
}
}
- return intel_plane-check_plane(plane, intel_state);
+ ret = intel_plane-check_plane(plane, intel_state);
+ if (ret || !state-state)
+ return ret;
+
+ return intel_plane_atomic_calc_changes(crtc_state-base, state);
}
static void intel_plane_atomic_update(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 26d610acb61f..71f6314fb643 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4393,19 +4393,19 @@ int skl_update_scaler_crtc(struct intel_crtc_state
*state, int force_detach)
* skl_update_scaler_plane - Stages update to scaler state for a given plane.
*
* @state: crtc's scaler state
- * @intel_plane: affected plane
* @plane_state: atomic plane state to update
*
* Return
* 0 - scaler_usage updated successfully
*error - requested scaling cannot be supported or other error condition
*/
-int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
- struct intel_plane *intel_plane,
- struct intel_plane_state *plane_state)
+static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
+ struct intel_plane_state *plane_state)
{
struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state-base.crtc);
+ struct intel_plane *intel_plane =
+ to_intel_plane(plane_state-base.plane);
struct drm_framebuffer *fb = plane_state-base.fb;
int ret;
@@ -11665,6 +11665,161 @@ retry:
return ret;
}
+
+/**
+ * intel_wm_need_update - Check whether watermarks need updating
+ * @plane: drm plane
+ * @state: new plane state
+ *
+ * Check current plane state versus the new one to determine whether
+ * watermarks need to be recalculated.
+ *
+ * Returns true or false.
+ */
+static bool intel_wm_need_update(struct drm_plane *plane,
+struct drm_plane_state *state)
+{
+ /* Update watermarks on tiling changes. */
+ if (!plane-state-fb || !state-fb ||
+ plane-state-fb-modifier[0] != state-fb-modifier[0] ||
+ plane-state-rotation != state-rotation)
+ return true;
+
+ if (plane-state-crtc_w != state-crtc_w)
+ return true;
+
+ return false;
+}
+
+int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
+ struct drm_plane_state *plane_state)
+{
+ struct drm_crtc *crtc = crtc_state-crtc;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_plane *plane = plane_state-plane;
+ struct drm_device *dev = crtc-dev;
+ struct drm_i915_private
[Intel-gfx] [PATCH v3 11/19] drm/i915: move detaching scalers to begin_crtc_commit, v2.
This is probably intended to be be done during vblank evasion.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_atomic.c | 3 ---
drivers/gpu/drm/i915/intel_display.c | 8
drivers/gpu/drm/i915/intel_drv.h | 1 -
3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c
b/drivers/gpu/drm/i915/intel_atomic.c
index d5afc2aa4ac7..c1263be8c98b 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -144,9 +144,6 @@ int intel_atomic_commit(struct drm_device *dev,
for_each_crtc_in_state(state, crtc, crtc_state, i) {
to_intel_crtc(crtc)-config = to_intel_crtc_state(crtc-state);
- if (INTEL_INFO(dev)-gen = 9)
- skl_detach_scalers(to_intel_crtc(crtc));
-
drm_atomic_helper_commit_planes_on_crtc(crtc_state);
}
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index a65c1065decf..5e77c2974288 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2915,16 +2915,13 @@ unsigned long intel_plane_obj_offset(struct intel_plane
*intel_plane,
/*
* This function detaches (aka. unbinds) unused scalers in hardware
*/
-void skl_detach_scalers(struct intel_crtc *intel_crtc)
+static void skl_detach_scalers(struct intel_crtc *intel_crtc)
{
struct drm_device *dev;
struct drm_i915_private *dev_priv;
struct intel_crtc_scaler_state *scaler_state;
int i;
- if (!intel_crtc || !intel_crtc-config)
- return;
-
dev = intel_crtc-base.dev;
dev_priv = dev-dev_private;
scaler_state = intel_crtc-config-scaler_state;
@@ -13819,6 +13816,9 @@ static void intel_begin_crtc_commit(struct drm_crtc
*crtc,
intel_crtc-atomic.evade =
intel_pipe_update_start(intel_crtc,
intel_crtc-atomic.start_vbl_count);
+
+ if (!needs_modeset(crtc-state) INTEL_INFO(dev)-gen = 9)
+ skl_detach_scalers(intel_crtc);
}
static void intel_finish_crtc_commit(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index baf57b3b136f..81398e279bfb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1146,7 +1146,6 @@ void intel_mode_from_pipe_config(struct drm_display_mode
*mode,
struct intel_crtc_state *pipe_config);
void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc);
void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file);
-void skl_detach_scalers(struct intel_crtc *intel_crtc);
int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state, int
force_detach);
int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state
*crtc_state);
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 03/19] drm/i915: Add a simple atomic crtc check function, v2.
Move the check for encoder cloning here.
Changes since v1:
- Remove was/is crtc_disabled. (mattrope)
- Rename function to intel_crtc_atomic_check. (mattrope)
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_atomic.c | 5 +-
drivers/gpu/drm/i915/intel_display.c | 126 ---
2 files changed, 75 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c
b/drivers/gpu/drm/i915/intel_atomic.c
index 041bff504629..6ab71ea92819 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -100,7 +100,10 @@ int intel_atomic_check(struct drm_device *dev,
if (ret)
return ret;
- /* FIXME: move to crtc atomic check function once it is ready */
+ /*
+* FIXME: move to crtc atomic check function once this is
+* more atomic friendly.
+*/
ret = intel_atomic_setup_scalers(dev, nuclear_crtc, crtc_state);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 956fb8423fff..29e761d9e0ec 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11661,11 +11661,82 @@ retry:
return ret;
}
+static bool encoders_cloneable(const struct intel_encoder *a,
+ const struct intel_encoder *b)
+{
+ /* masks could be asymmetric, so check both ways */
+ return a == b || (a-cloneable (1 b-type)
+ b-cloneable (1 a-type));
+}
+
+static bool check_single_encoder_cloning(struct drm_atomic_state *state,
+struct intel_crtc *crtc,
+struct intel_encoder *encoder)
+{
+ struct intel_encoder *source_encoder;
+ struct drm_connector *connector;
+ struct drm_connector_state *connector_state;
+ int i;
+
+ for_each_connector_in_state(state, connector, connector_state, i) {
+ if (connector_state-crtc != crtc-base)
+ continue;
+
+ source_encoder =
+ to_intel_encoder(connector_state-best_encoder);
+ if (!encoders_cloneable(encoder, source_encoder))
+ return false;
+ }
+
+ return true;
+}
+
+static bool check_encoder_cloning(struct drm_atomic_state *state,
+ struct intel_crtc *crtc)
+{
+ struct intel_encoder *encoder;
+ struct drm_connector *connector;
+ struct drm_connector_state *connector_state;
+ int i;
+
+ for_each_connector_in_state(state, connector, connector_state, i) {
+ if (connector_state-crtc != crtc-base)
+ continue;
+
+ encoder = to_intel_encoder(connector_state-best_encoder);
+ if (!check_single_encoder_cloning(state, crtc, encoder))
+ return false;
+ }
+
+ return true;
+}
+
+static int intel_crtc_atomic_check(struct drm_crtc *crtc,
+ struct drm_crtc_state *crtc_state)
+{
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_atomic_state *state = crtc_state-state;
+ int idx = crtc-base.id;
+ bool mode_changed = needs_modeset(crtc_state);
+
+ if (mode_changed !check_encoder_cloning(state, intel_crtc)) {
+ DRM_DEBUG_KMS(rejecting invalid cloning configuration\n);
+ return -EINVAL;
+ }
+
+ I915_STATE_WARN(crtc-state-active != intel_crtc-active,
+ [CRTC:%i] mismatch between state-active(%i) and
crtc-active(%i)\n,
+ idx, crtc-state-active, intel_crtc-active);
+
+ return 0;
+}
+
static const struct drm_crtc_helper_funcs intel_helper_funcs = {
.mode_set_base_atomic = intel_pipe_set_base_atomic,
.load_lut = intel_crtc_load_lut,
.atomic_begin = intel_begin_crtc_commit,
.atomic_flush = intel_finish_crtc_commit,
+ .atomic_check = intel_crtc_atomic_check,
};
/**
@@ -11918,56 +11989,6 @@ static void intel_dump_pipe_config(struct intel_crtc
*crtc,
}
}
-static bool encoders_cloneable(const struct intel_encoder *a,
- const struct intel_encoder *b)
-{
- /* masks could be asymmetric, so check both ways */
- return a == b || (a-cloneable (1 b-type)
- b-cloneable (1 a-type));
-}
-
-static bool check_single_encoder_cloning(struct drm_atomic_state *state,
-struct intel_crtc *crtc,
-struct intel_encoder *encoder)
-{
- struct intel_encoder *source_encoder;
- struct drm_connector *connector;
- struct drm_connector_state *connector_state;
- int i;
-
- for_each_connector_in_state(state, connector,
[Intel-gfx] [PATCH v3 15/19] drm/i915: atomic plane updates in a nutshell
Now that all planes are added during a modeset we can use the
calculated changes before disabling a plane, and then either commit
or force disable a plane before disabling the crtc.
The code is shared with atomic_begin/flush, except watermark updating
and vblank evasion are not used.
This is needed for proper atomic suspend/resume support.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90868
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 103 ---
drivers/gpu/drm/i915/intel_sprite.c | 4 +-
2 files changed, 23 insertions(+), 84 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index cc4ca4970716..beb69281f45c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2217,28 +2217,6 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
intel_wait_for_pipe_off(crtc);
}
-/**
- * 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);
- to_intel_plane_state(plane-state)-visible = true;
-
- 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
@@ -4508,20 +4486,6 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
}
}
-static void intel_enable_sprite_planes(struct drm_crtc *crtc)
-{
- struct drm_device *dev = crtc-dev;
- enum pipe pipe = to_intel_crtc(crtc)-pipe;
- struct drm_plane *plane;
- struct intel_plane *intel_plane;
-
- drm_for_each_legacy_plane(plane, dev-mode_config.plane_list) {
- intel_plane = to_intel_plane(plane);
- if (intel_plane-pipe == pipe)
- intel_plane_restore(intel_plane-base);
- }
-}
-
void hsw_enable_ips(struct intel_crtc *crtc)
{
struct drm_device *dev = crtc-base.dev;
@@ -4817,27 +4781,6 @@ static void intel_pre_plane_update(struct intel_crtc
*crtc)
intel_pre_disable_primary(crtc-base);
}
-static void intel_crtc_enable_planes(struct drm_crtc *crtc)
-{
- struct drm_device *dev = crtc-dev;
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- int pipe = intel_crtc-pipe;
-
- intel_enable_primary_hw_plane(crtc-primary, crtc);
- intel_enable_sprite_planes(crtc);
- if (to_intel_plane_state(crtc-cursor-state)-visible)
- intel_crtc_update_cursor(crtc, true);
-
- intel_post_enable_primary(crtc);
-
- /*
-* FIXME: Once we grow proper nuclear flip support out of this we need
-* to compute the mask of flip planes precisely. For the time being
-* consider this a flip to a NULL plane.
-*/
- intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_ALL_MASK(pipe));
-}
-
static void intel_crtc_disable_planes(struct drm_crtc *crtc, unsigned
plane_mask)
{
struct drm_device *dev = crtc-dev;
@@ -4845,10 +4788,6 @@ static void intel_crtc_disable_planes(struct drm_crtc
*crtc, unsigned plane_mask
struct drm_plane *p;
int pipe = intel_crtc-pipe;
- intel_crtc_wait_for_pending_flips(crtc);
-
- intel_pre_disable_primary(crtc);
-
intel_crtc_dpms_overlay_disable(intel_crtc);
drm_for_each_plane_mask(p, dev, plane_mask)
@@ -6270,6 +6209,11 @@ static void intel_crtc_disable_noatomic(struct drm_crtc
*crtc)
if (!intel_crtc-active)
return;
+ if (to_intel_plane_state(crtc-primary-state)-visible) {
+ intel_crtc_wait_for_pending_flips(crtc);
+ intel_pre_disable_primary(crtc);
+ }
+
intel_crtc_disable_planes(crtc, crtc-state-plane_mask);
dev_priv-display.crtc_disable(crtc);
@@ -11783,10 +11727,6 @@ int intel_plane_atomic_calc_changes(struct
drm_crtc_state *crtc_state,
if (old_plane_state-base.fb !fb)
intel_crtc-atomic.disabled_planes |= 1 i;
- /* don't run rest during modeset yet */
- if (!intel_crtc-active || mode_changed)
- return 0;
-
was_visible = old_plane_state-visible;
visible = to_intel_plane_state(plane_state)-visible;
@@ -13255,15 +13195,18 @@ static int __intel_set_mode(struct drm_atomic_state
*state)
[Intel-gfx] [PATCH v3 06/19] drm/i915: Split skl_update_scaler, v3.
It's easier to read separate functions for crtc and plane scaler state.
Changes since v1:
- Update documentation.
Changes since v2:
- Get rid of parameters to skl_update_scaler only used for traces.
This avoids needing to document the other parameters.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 211 +++
drivers/gpu/drm/i915/intel_dp.c | 2 +-
drivers/gpu/drm/i915/intel_drv.h | 12 +-
drivers/gpu/drm/i915/intel_sprite.c | 3 +-
4 files changed, 121 insertions(+), 107 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 0f7652a31c95..26d610acb61f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4303,62 +4303,16 @@ static void cpt_verify_modeset(struct drm_device *dev,
int pipe)
}
}
-/**
- * skl_update_scaler_users - Stages update to crtc's scaler state
- * @intel_crtc: crtc
- * @crtc_state: crtc_state
- * @plane: plane (NULL indicates crtc is requesting update)
- * @plane_state: plane's state
- * @force_detach: request unconditional detachment of scaler
- *
- * This function updates scaler state for requested plane or crtc.
- * To request scaler usage update for a plane, caller shall pass plane pointer.
- * To request scaler usage update for crtc, caller shall pass plane pointer
- * as NULL.
- *
- * Return
- * 0 - scaler_usage updated successfully
- *error - requested scaling cannot be supported or other error condition
- */
-int
-skl_update_scaler_users(
- struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state,
- struct intel_plane *intel_plane, struct intel_plane_state *plane_state,
- int force_detach)
+static int
+skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
+ unsigned scaler_idx, int *scaler_id, unsigned int rotation,
+ int src_w, int src_h, int dst_w, int dst_h)
{
+ struct intel_crtc_scaler_state *scaler_state =
+ crtc_state-scaler_state;
+ struct intel_crtc *intel_crtc =
+ to_intel_crtc(crtc_state-base.crtc);
int need_scaling;
- int idx;
- int src_w, src_h, dst_w, dst_h;
- int *scaler_id;
- struct drm_framebuffer *fb;
- struct intel_crtc_scaler_state *scaler_state;
- unsigned int rotation;
-
- if (!intel_crtc || !crtc_state)
- return 0;
-
- scaler_state = crtc_state-scaler_state;
-
- idx = intel_plane ? drm_plane_index(intel_plane-base) :
SKL_CRTC_INDEX;
- fb = intel_plane ? plane_state-base.fb : NULL;
-
- if (intel_plane) {
- src_w = drm_rect_width(plane_state-src) 16;
- src_h = drm_rect_height(plane_state-src) 16;
- dst_w = drm_rect_width(plane_state-dst);
- dst_h = drm_rect_height(plane_state-dst);
- scaler_id = plane_state-scaler_id;
- rotation = plane_state-base.rotation;
- } else {
- struct drm_display_mode *adjusted_mode =
- crtc_state-base.adjusted_mode;
- src_w = crtc_state-pipe_src_w;
- src_h = crtc_state-pipe_src_h;
- dst_w = adjusted_mode-hdisplay;
- dst_h = adjusted_mode-vdisplay;
- scaler_id = scaler_state-scaler_id;
- rotation = DRM_ROTATE_0;
- }
need_scaling = intel_rotation_90_or_270(rotation) ?
(src_h != dst_w || src_w != dst_h):
@@ -4374,17 +4328,14 @@ skl_update_scaler_users(
* update to free the scaler is done in plane/panel-fit programming.
* For this purpose crtc/plane_state-scaler_id isn't reset here.
*/
- if (force_detach || !need_scaling || (intel_plane
- (!fb || !plane_state-visible))) {
+ if (force_detach || !need_scaling) {
if (*scaler_id = 0) {
- scaler_state-scaler_users = ~(1 idx);
+ scaler_state-scaler_users = ~(1 scaler_idx);
scaler_state-scalers[*scaler_id].in_use = 0;
- DRM_DEBUG_KMS(Staged freeing scaler id %d.%d from
%s:%d
- crtc_state = %p scaler_users = 0x%x\n,
- intel_crtc-pipe, *scaler_id, intel_plane ?
PLANE : CRTC,
- intel_plane ? intel_plane-base.base.id :
- intel_crtc-base.base.id, crtc_state,
+ DRM_DEBUG_KMS(scaler_user index %u.%u:
+ Staged freeing scaler id %d.%d scaler_users =
0x%x\n,
+ intel_crtc-pipe, scaler_idx, *scaler_id,
scaler_idx,
scaler_state-scaler_users);
*scaler_id = -1;
}
@@ -4397,51 +4348,112 @@
[Intel-gfx] [PATCH v3 09/19] drm/i915: clean up atomic plane check functions, v2.
By passing crtc_state to the check_plane functions a lot of duplicated
code can be removed. There are still some transitional helper calls,
they will be removed later.
Changes since v1:
- Revert state-visible changes.
- Use plane-state-crtc instead of plane-crtc.
- Use drm_atomic_get_existing_crtc_state.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_atomic_plane.c | 16 +++
drivers/gpu/drm/i915/intel_display.c | 48 ++-
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_sprite.c | 9 ++
4 files changed, 29 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index aa2128369a0a..91d53768df9d 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -116,7 +116,7 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
struct intel_plane_state *intel_state = to_intel_plane_state(state);
int ret;
- crtc = crtc ? crtc : plane-crtc;
+ crtc = crtc ? crtc : plane-state-crtc;
intel_crtc = to_intel_crtc(crtc);
/*
@@ -131,10 +131,13 @@ static int intel_plane_atomic_check(struct drm_plane
*plane,
/* FIXME: temporary hack necessary while we still use the plane update
* helper. */
if (state-state) {
- crtc_state =
- intel_atomic_get_crtc_state(state-state, intel_crtc);
- if (IS_ERR(crtc_state))
- return PTR_ERR(crtc_state);
+ struct drm_crtc_state *drm_crtc_state =
+ drm_atomic_get_existing_crtc_state(state-state, crtc);
+
+ if (WARN_ON(!drm_crtc_state))
+ return -EINVAL;
+
+ crtc_state = to_intel_crtc_state(drm_crtc_state);
} else {
crtc_state = intel_crtc-config;
}
@@ -185,7 +188,8 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
}
}
- ret = intel_plane-check_plane(plane, intel_state);
+ intel_state-visible = false;
+ ret = intel_plane-check_plane(plane, crtc_state, intel_state);
if (ret || !state-state)
return ret;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index ec4924eecd68..61697335bff2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13703,36 +13703,25 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct
intel_crtc_state *crtc_state
static int
intel_check_primary_plane(struct drm_plane *plane,
+ struct intel_crtc_state *crtc_state,
struct intel_plane_state *state)
{
- struct drm_device *dev = plane-dev;
struct drm_crtc *crtc = state-base.crtc;
- struct intel_crtc *intel_crtc;
- struct intel_crtc_state *crtc_state;
struct drm_framebuffer *fb = state-base.fb;
- struct drm_rect *dest = state-dst;
- struct drm_rect *src = state-src;
- const struct drm_rect *clip = state-clip;
- bool can_position = false;
- int max_scale = DRM_PLANE_HELPER_NO_SCALING;
int min_scale = DRM_PLANE_HELPER_NO_SCALING;
+ int max_scale = DRM_PLANE_HELPER_NO_SCALING;
+ bool can_position = false;
- crtc = crtc ? crtc : plane-crtc;
- intel_crtc = to_intel_crtc(crtc);
- crtc_state = state-base.state ?
- intel_atomic_get_crtc_state(state-base.state, intel_crtc) :
NULL;
-
- if (INTEL_INFO(dev)-gen = 9) {
- /* use scaler when colorkey is not required */
- if (to_intel_plane(plane)-ckey.flags ==
I915_SET_COLORKEY_NONE) {
- min_scale = 1;
- max_scale = skl_max_scale(intel_crtc, crtc_state);
- }
+ /* use scaler when colorkey is not required */
+ if (INTEL_INFO(plane-dev)-gen = 9
+ to_intel_plane(plane)-ckey.flags == I915_SET_COLORKEY_NONE) {
+ min_scale = 1;
+ max_scale = skl_max_scale(to_intel_crtc(crtc), crtc_state);
can_position = true;
}
- return drm_plane_helper_check_update(plane, crtc, fb,
-src, dest, clip,
+ return drm_plane_helper_check_update(plane, crtc, fb, state-src,
+state-dst, state-clip,
min_scale, max_scale,
can_position, true,
state-visible);
@@ -13973,24 +13962,17 @@ void intel_create_rotation_property(struct drm_device
*dev, struct intel_plane *
static int
intel_check_cursor_plane(struct drm_plane *plane,
+struct intel_crtc_state
[Intel-gfx] [PATCH v3 19/19] drm/i915: Make cdclk part of the atomic state.
The skylake scalers depend on the cdclk freq, but that frequency can
change during a modeset. So when a modeset happens calculate the new
cdclk in the atomic state. With the transitional helpers gone the
cached value can be used in the scaler, and committed after all
crtc's are disabled.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90874
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 3 +-
drivers/gpu/drm/i915/intel_atomic.c | 2 +
drivers/gpu/drm/i915/intel_display.c | 274 +--
drivers/gpu/drm/i915/intel_drv.h | 1 +
4 files changed, 135 insertions(+), 145 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 611fbd86c1cc..0f03d995151b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -619,7 +619,8 @@ struct drm_i915_display_funcs {
struct drm_crtc *crtc,
uint32_t sprite_width, uint32_t sprite_height,
int pixel_size, bool enable, bool scaled);
- void (*modeset_global_resources)(struct drm_atomic_state *state);
+ int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
+ void (*modeset_commit_cdclk)(struct drm_atomic_state *state);
/* Returns the active state of the crtc, and if the crtc is active,
* fills out the pipe-config with the hw state. */
bool (*get_pipe_config)(struct intel_crtc *,
diff --git a/drivers/gpu/drm/i915/intel_atomic.c
b/drivers/gpu/drm/i915/intel_atomic.c
index 060d98b10f83..0aeced82201e 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -54,6 +54,8 @@ int intel_atomic_check(struct drm_device *dev,
int i;
bool not_nuclear = false;
+ to_intel_atomic_state(state)-cdclk = to_i915(dev)-cdclk_freq;
+
/*
* FIXME: At the moment, we only support nuclear pageflip on a
* single CRTC. Cross-crtc updates will be added later.
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 746c73d2ab84..39d27d87a0b2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5194,8 +5194,13 @@ static void modeset_update_crtc_power_domains(struct
drm_atomic_state *state)
intel_display_power_get(dev_priv, domain);
}
- if (dev_priv-display.modeset_global_resources)
- dev_priv-display.modeset_global_resources(state);
+ if (dev_priv-display.modeset_commit_cdclk) {
+ unsigned int cdclk = to_intel_atomic_state(state)-cdclk;
+
+ if (cdclk != dev_priv-cdclk_freq
+ !WARN_ON(!state-allow_modeset))
+ dev_priv-display.modeset_commit_cdclk(state);
+ }
for_each_intel_crtc(dev, crtc) {
enum intel_display_power_domain domain;
@@ -5847,11 +5852,7 @@ static int intel_mode_max_pixclk(struct drm_device *dev,
int max_pixclk = 0;
for_each_intel_crtc(dev, intel_crtc) {
- if (state)
- crtc_state =
- intel_atomic_get_crtc_state(state, intel_crtc);
- else
- crtc_state = intel_crtc-config;
+ crtc_state = intel_atomic_get_crtc_state(state, intel_crtc);
if (IS_ERR(crtc_state))
return PTR_ERR(crtc_state);
@@ -5865,46 +5866,34 @@ static int intel_mode_max_pixclk(struct drm_device *dev,
return max_pixclk;
}
-static int valleyview_modeset_global_pipes(struct drm_atomic_state *state)
+static int valleyview_modeset_calc_cdclk(struct drm_atomic_state *state)
{
- struct drm_i915_private *dev_priv = to_i915(state-dev);
- struct drm_crtc *crtc;
- struct drm_crtc_state *crtc_state;
- int max_pixclk = intel_mode_max_pixclk(state-dev, state);
- int cdclk, ret = 0;
+ struct drm_device *dev = state-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ int max_pixclk = intel_mode_max_pixclk(dev, state);
if (max_pixclk 0)
return max_pixclk;
- if (IS_VALLEYVIEW(dev_priv))
- cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
- else
- cdclk = broxton_calc_cdclk(dev_priv, max_pixclk);
-
- if (cdclk == dev_priv-cdclk_freq)
- return 0;
-
- /* add all active pipes to the state */
- for_each_crtc(state-dev, crtc) {
- crtc_state = drm_atomic_get_crtc_state(state, crtc);
- if (IS_ERR(crtc_state))
- return PTR_ERR(crtc_state);
+ to_intel_atomic_state(state)-cdclk =
+ valleyview_calc_cdclk(dev_priv, max_pixclk);
- if (!crtc_state-active || needs_modeset(crtc_state))
-
[Intel-gfx] [PATCH v3 05/19] drm/i915: Assign a new pll from the crtc check function, v2.
It saves another loop over all crtc's in the state, and computing
clock is more of a per crtc thing.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 60
1 file changed, 26 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index e6837172afaf..0f7652a31c95 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11707,11 +11707,12 @@ static int intel_crtc_atomic_check(struct drm_crtc
*crtc,
struct drm_crtc_state *crtc_state)
{
struct drm_device *dev = crtc-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_crtc_state *pipe_config =
to_intel_crtc_state(crtc_state);
struct drm_atomic_state *state = crtc_state-state;
- int idx = crtc-base.id;
+ int ret, idx = crtc-base.id;
bool mode_changed = needs_modeset(crtc_state);
if (mode_changed !check_encoder_cloning(state, intel_crtc)) {
@@ -11723,6 +11724,15 @@ static int intel_crtc_atomic_check(struct drm_crtc
*crtc,
[CRTC:%i] mismatch between state-active(%i) and
crtc-active(%i)\n,
idx, crtc-state-active, intel_crtc-active);
+ if (mode_changed crtc_state-enable
+ dev_priv-display.crtc_compute_clock
+ !WARN_ON(pipe_config-shared_dpll != DPLL_ID_PRIVATE)) {
+ ret = dev_priv-display.crtc_compute_clock(intel_crtc,
+ pipe_config);
+ if (ret)
+ return ret;
+ }
+
return intel_atomic_setup_scalers(dev, intel_crtc, pipe_config);
}
@@ -12777,53 +12787,37 @@ static void update_scanline_offset(struct intel_crtc
*crtc)
crtc-scanline_offset = 1;
}
-static int intel_modeset_setup_plls(struct drm_atomic_state *state)
+static void intel_modeset_clear_plls(struct drm_atomic_state *state)
{
struct drm_device *dev = state-dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- unsigned clear_pipes = 0;
+ struct intel_shared_dpll_config *shared_dpll = NULL;
struct intel_crtc *intel_crtc;
struct intel_crtc_state *intel_crtc_state;
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
- int ret = 0;
int i;
if (!dev_priv-display.crtc_compute_clock)
- return 0;
+ return;
for_each_crtc_in_state(state, crtc, crtc_state, i) {
+ int dpll;
+
intel_crtc = to_intel_crtc(crtc);
intel_crtc_state = to_intel_crtc_state(crtc_state);
+ dpll = intel_crtc_state-shared_dpll;
- if (needs_modeset(crtc_state)) {
- clear_pipes |= 1 intel_crtc-pipe;
- intel_crtc_state-shared_dpll = DPLL_ID_PRIVATE;
- }
- }
-
- if (clear_pipes) {
- struct intel_shared_dpll_config *shared_dpll =
- intel_atomic_get_shared_dpll_state(state);
-
- for (i = 0; i dev_priv-num_shared_dpll; i++)
- shared_dpll[i].crtc_mask = ~clear_pipes;
- }
-
- for_each_crtc_in_state(state, crtc, crtc_state, i) {
- if (!needs_modeset(crtc_state) || !crtc_state-enable)
+ if (!needs_modeset(crtc_state) || dpll == DPLL_ID_PRIVATE)
continue;
- intel_crtc = to_intel_crtc(crtc);
- intel_crtc_state = to_intel_crtc_state(crtc_state);
+ intel_crtc_state-shared_dpll = DPLL_ID_PRIVATE;
- ret = dev_priv-display.crtc_compute_clock(intel_crtc,
- intel_crtc_state);
- if (ret)
- return ret;
- }
+ if (!shared_dpll)
+ shared_dpll = intel_atomic_get_shared_dpll_state(state);
- return ret;
+ shared_dpll[dpll].crtc_mask = ~(1 intel_crtc-pipe);
+ }
}
/*
@@ -12919,14 +12913,12 @@ static int intel_modeset_checks(struct
drm_atomic_state *state)
return ret;
}
- ret = intel_modeset_setup_plls(state);
- if (ret)
- return ret;
+ intel_modeset_clear_plls(state);
if (IS_HASWELL(dev))
- ret = haswell_mode_set_planes_workaround(state);
+ return haswell_mode_set_planes_workaround(state);
- return ret;
+ return 0;
}
static int
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 17/19] drm/i915: Make setting color key atomic.
By making color key atomic there are no more transitional helpers.
The plane check function will reject the color key when a scaler is
active.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_atomic_plane.c | 1 +
drivers/gpu/drm/i915/intel_display.c | 7 ++-
drivers/gpu/drm/i915/intel_drv.h | 6 +--
drivers/gpu/drm/i915/intel_sprite.c | 85 +++
4 files changed, 46 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 91d53768df9d..10a8ecedc942 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -56,6 +56,7 @@ intel_create_plane_state(struct drm_plane *plane)
state-base.plane = plane;
state-base.rotation = BIT(DRM_ROTATE_0);
+ state-ckey.flags = I915_SET_COLORKEY_NONE;
return state;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5facd0501a34..746c73d2ab84 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4401,9 +4401,9 @@ static int skl_update_scaler_plane(struct
intel_crtc_state *crtc_state,
return ret;
/* check colorkey */
- if (WARN_ON(intel_plane-ckey.flags != I915_SET_COLORKEY_NONE)) {
+ if (plane_state-ckey.flags != I915_SET_COLORKEY_NONE) {
DRM_DEBUG_KMS([PLANE:%d] scaling with color key not allowed,
- intel_plane-base.base.id);
+ intel_plane-base.base.id);
return -EINVAL;
}
@@ -13733,7 +13733,7 @@ intel_check_primary_plane(struct drm_plane *plane,
/* use scaler when colorkey is not required */
if (INTEL_INFO(plane-dev)-gen = 9
- to_intel_plane(plane)-ckey.flags == I915_SET_COLORKEY_NONE) {
+ state-ckey.flags == I915_SET_COLORKEY_NONE) {
min_scale = 1;
max_scale = skl_max_scale(to_intel_crtc(crtc), crtc_state);
can_position = true;
@@ -13881,7 +13881,6 @@ static struct drm_plane
*intel_primary_plane_create(struct drm_device *dev,
primary-check_plane = intel_check_primary_plane;
primary-commit_plane = intel_commit_primary_plane;
primary-disable_plane = intel_disable_primary_plane;
- primary-ckey.flags = I915_SET_COLORKEY_NONE;
if (HAS_FBC(dev) INTEL_INFO(dev)-gen 4)
primary-plane = !pipe;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 93b9542ab8dc..3a2ac82b0970 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -274,6 +274,8 @@ struct intel_plane_state {
* update_scaler_plane.
*/
int scaler_id;
+
+ struct drm_intel_sprite_colorkey ckey;
};
struct intel_initial_plane_config {
@@ -588,9 +590,6 @@ struct intel_plane {
bool can_scale;
int max_downscale;
- /* FIXME convert to properties */
- struct drm_intel_sprite_colorkey ckey;
-
/* Since we need to change the watermarks before/after
* enabling/disabling the planes, we need to store the parameters here
* as the other pieces of the struct may not reflect the values we want
@@ -1390,7 +1389,6 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t
sdvo_reg, bool is_sdvob);
/* intel_sprite.c */
int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane);
-int intel_plane_restore(struct drm_plane *plane);
int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
struct drm_file *file_priv);
bool intel_pipe_update_start(struct intel_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 168f90f346c2..21d3f7882c4d 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -182,7 +182,8 @@ skl_update_plane(struct drm_plane *drm_plane, struct
drm_crtc *crtc,
const int plane = intel_plane-plane + 1;
u32 plane_ctl, stride_div, stride;
int pixel_size = drm_format_plane_cpp(fb-pixel_format, 0);
- const struct drm_intel_sprite_colorkey *key = intel_plane-ckey;
+ const struct drm_intel_sprite_colorkey *key =
+ to_intel_plane_state(drm_plane-state)-ckey;
unsigned long surf_addr;
u32 tile_height, plane_offset, plane_size;
unsigned int rotation;
@@ -344,7 +345,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc
*crtc,
u32 sprctl;
unsigned long sprsurf_offset, linear_offset;
int pixel_size = drm_format_plane_cpp(fb-pixel_format, 0);
- const struct drm_intel_sprite_colorkey *key = intel_plane-ckey;
+ const struct drm_intel_sprite_colorkey *key =
+ to_intel_plane_state(dplane-state)-ckey;
[Intel-gfx] [PATCH v3 00/19] Convert to atomic, part 3.
Requisites: - [PATCH] drm/atomic: pass old crtc state to atomic_begin/flush. This patch series converts plane updates and cdclk updates to atomic, but still doesn't touch the hw readout code, which was regressing a lot. The fixes in this series are needed to support proper hw readout, and can be applied on top of topic/atomic-conversion. In particular, this fixes the following bugs: - https://bugs.freedesktop.org/show_bug.cgi?id=90874 Needs atomic CDCLK as part of state, before any plane checks, or scalers will not work correctly. - https://bugs.freedesktop.org/show_bug.cgi?id=90868 It shows a problem with plane visibility on resume. This is fixed by calcing plane states correctly across modeset. There's also a problem with DPLL 0 failing to lock, I hope that's fixed by cdclk changes, but it might have been a bug in the reverted atomic hw readout patch too. Maarten Lankhorst (19): drm/i915: Use crtc state in intel_modeset_pipe_config drm/i915: Clean up intel_atomic_setup_scalers slightly. drm/i915: Add a simple atomic crtc check function, v2. drm/i915: Move scaler setup to check crtc function, v2. drm/i915: Assign a new pll from the crtc check function, v2. drm/i915: Split skl_update_scaler, v3. drm/i915: Split plane updates of crtc-atomic into a helper, v2. drm/i915: clean up plane commit functions drm/i915: clean up atomic plane check functions, v2. drm/i915: remove force argument from disable_plane drm/i915: move detaching scalers to begin_crtc_commit, v2. drm/i915: Move crtc commit updates to separate functions. drm/i915: Do not run most checks when there's no modeset. drm/i915: Handle disabling planes better, v2. drm/i915: atomic plane updates in a nutshell drm/i915: Update less state during modeset. drm/i915: Make setting color key atomic. drm/i915: Remove transitional references from intel_plane_atomic_check. drm/i915: Make cdclk part of the atomic state. drivers/gpu/drm/i915/i915_drv.h |3 +- drivers/gpu/drm/i915/intel_atomic.c | 47 +- drivers/gpu/drm/i915/intel_atomic_plane.c | 41 +- drivers/gpu/drm/i915/intel_display.c | 1480 +++-- drivers/gpu/drm/i915/intel_dp.c |2 +- drivers/gpu/drm/i915/intel_drv.h | 27 +- drivers/gpu/drm/i915/intel_sprite.c | 170 ++-- 7 files changed, 874 insertions(+), 896 deletions(-) -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 08/19] drm/i915: clean up plane commit functions
No point in hiding behind big ifs. This will be true most of the time.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 16
drivers/gpu/drm/i915/intel_sprite.c | 33 ++---
2 files changed, 22 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 71f6314fb643..ec4924eecd68 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13756,14 +13756,14 @@ intel_commit_primary_plane(struct drm_plane *plane,
crtc-x = src-x1 16;
crtc-y = src-y1 16;
- if (intel_crtc-active) {
- if (state-visible)
- /* FIXME: kill this fastboot hack */
- intel_update_pipe_size(intel_crtc);
+ if (!intel_crtc-active)
+ return;
- dev_priv-display.update_primary_plane(crtc, plane-fb,
- crtc-x, crtc-y);
- }
+ if (state-visible)
+ /* FIXME: kill this fastboot hack */
+ intel_update_pipe_size(intel_crtc);
+
+ dev_priv-display.update_primary_plane(crtc, fb, crtc-x, crtc-y);
}
static void
@@ -14067,8 +14067,8 @@ intel_commit_cursor_plane(struct drm_plane *plane,
intel_crtc-cursor_addr = addr;
intel_crtc-cursor_bo = obj;
-update:
+update:
if (intel_crtc-active)
intel_crtc_update_cursor(crtc, state-visible);
}
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index f5921b652b90..c909b8b8ce85 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -927,31 +927,26 @@ intel_commit_sprite_plane(struct drm_plane *plane,
struct intel_crtc *intel_crtc;
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_framebuffer *fb = state-base.fb;
- int crtc_x, crtc_y;
- unsigned int crtc_w, crtc_h;
- uint32_t src_x, src_y, src_w, src_h;
crtc = crtc ? crtc : plane-crtc;
intel_crtc = to_intel_crtc(crtc);
plane-fb = fb;
- if (intel_crtc-active) {
- if (state-visible) {
- crtc_x = state-dst.x1;
- crtc_y = state-dst.y1;
- crtc_w = drm_rect_width(state-dst);
- crtc_h = drm_rect_height(state-dst);
- src_x = state-src.x1 16;
- src_y = state-src.y1 16;
- src_w = drm_rect_width(state-src) 16;
- src_h = drm_rect_height(state-src) 16;
- intel_plane-update_plane(plane, crtc, fb,
- crtc_x, crtc_y, crtc_w,
crtc_h,
- src_x, src_y, src_w, src_h);
- } else {
- intel_plane-disable_plane(plane, crtc, false);
- }
+ if (!intel_crtc-active)
+ return;
+
+ if (state-visible) {
+ intel_plane-update_plane(plane, crtc, fb,
+ state-dst.x1, state-dst.y1,
+ drm_rect_width(state-dst),
+ drm_rect_height(state-dst),
+ state-src.x1 16,
+ state-src.y1 16,
+ drm_rect_width(state-src) 16,
+ drm_rect_height(state-src) 16);
+ } else {
+ intel_plane-disable_plane(plane, crtc, false);
}
}
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 14/19] drm/i915: Handle disabling planes better, v2.
Read out the initial state, and add a quirk to force add all planes
to crtc_state-plane_mask during initial commit. This will disable
all planes during the initial modeset.
The initial plane quirk is temporary, and will go away when hardware
readout is fully atomic, and the watermark updates in intel_sprite.c
are removed.
Changes since v1:
- Unset state-visible on !primary planes.
- Do not rely on the plane-crtc pointer in intel_atomic_plane,
instead assume planes are invisible until modeset.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_atomic.c | 7 ++
drivers/gpu/drm/i915/intel_display.c | 120 ---
drivers/gpu/drm/i915/intel_drv.h | 1 +
3 files changed, 106 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c
b/drivers/gpu/drm/i915/intel_atomic.c
index c1263be8c98b..060d98b10f83 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -96,6 +96,13 @@ int intel_atomic_check(struct drm_device *dev,
return -EINVAL;
}
+ if (crtc_state
+ crtc_state-quirks PIPE_CONFIG_QUIRK_INITIAL_PLANES) {
+ ret = drm_atomic_add_affected_planes(state,
nuclear_crtc-base);
+ if (ret)
+ return ret;
+ }
+
ret = drm_atomic_helper_check_planes(dev, state);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index ae046648c29d..cc4ca4970716 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -109,8 +109,6 @@ static void skl_init_scalers(struct drm_device *dev, struct
intel_crtc *intel_cr
struct intel_crtc_state *crtc_state);
static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
int num_connectors);
-static void intel_crtc_enable_planes(struct drm_crtc *crtc);
-static void intel_crtc_disable_planes(struct drm_crtc *crtc);
static struct intel_encoder *intel_find_encoder(struct intel_connector
*connector, int pipe)
{
@@ -4840,11 +4838,11 @@ static void intel_crtc_enable_planes(struct drm_crtc
*crtc)
intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_ALL_MASK(pipe));
}
-static void intel_crtc_disable_planes(struct drm_crtc *crtc)
+static void intel_crtc_disable_planes(struct drm_crtc *crtc, unsigned
plane_mask)
{
struct drm_device *dev = crtc-dev;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_plane *intel_plane;
+ struct drm_plane *p;
int pipe = intel_crtc-pipe;
intel_crtc_wait_for_pending_flips(crtc);
@@ -4852,14 +4850,9 @@ static void intel_crtc_disable_planes(struct drm_crtc
*crtc)
intel_pre_disable_primary(crtc);
intel_crtc_dpms_overlay_disable(intel_crtc);
- for_each_intel_plane(dev, intel_plane) {
- if (intel_plane-pipe == pipe) {
- struct drm_crtc *from = intel_plane-base.crtc;
- intel_plane-disable_plane(intel_plane-base,
- from ?: crtc);
- }
- }
+ drm_for_each_plane_mask(p, dev, plane_mask)
+ to_intel_plane(p)-disable_plane(p, crtc);
/*
* FIXME: Once we grow proper nuclear flip support out of this we need
@@ -6277,7 +6270,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc
*crtc)
if (!intel_crtc-active)
return;
- intel_crtc_disable_planes(crtc);
+ intel_crtc_disable_planes(crtc, crtc-state-plane_mask);
dev_priv-display.crtc_disable(crtc);
domains = intel_crtc-enabled_power_domains;
@@ -11873,7 +11866,7 @@ int intel_plane_atomic_calc_changes(struct
drm_crtc_state *crtc_state,
intel_crtc-atomic.fb_bits |=
INTEL_FRONTBUFFER_SPRITE(intel_crtc-pipe);
- if (turn_off is_crtc_enabled) {
+ if (turn_off !mode_changed) {
intel_crtc-atomic.wait_vblank = true;
intel_crtc-atomic.update_sprite_watermarks |=
1 i;
@@ -11933,6 +11926,34 @@ static bool check_encoder_cloning(struct
drm_atomic_state *state,
return true;
}
+static void intel_crtc_check_initial_planes(struct drm_crtc *crtc,
+ struct drm_crtc_state *crtc_state)
+{
+ struct intel_crtc_state *pipe_config =
+ to_intel_crtc_state(crtc_state);
+ struct drm_plane *p;
+ unsigned visible_mask = 0;
+
+ drm_for_each_plane_mask(p, crtc-dev, crtc_state-plane_mask) {
+ struct drm_plane_state *plane_state =
+ drm_atomic_get_existing_plane_state(crtc_state-state,
p);
+
+ if (WARN_ON(!plane_state))
+
Re: [Intel-gfx] [RFC] drm/i915: prevent out of range pt in the PDE macros (take 2)
On 13/06/15 09:28, Chris Wilson wrote:
On Fri, Jun 12, 2015 at 06:30:56PM -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
We tried to fix this in the following commit:
commit fdc454c1484a20e1345cf4e4d7a9feaee814147f
Author: Michel Thierry michel.thie...@intel.com
Date: Tue Mar 24 15:46:19 2015 +
drm/i915: Prevent out of range pt in gen6_for_each_pde
but the static analyzer still complains that, just before we break due
to iter I915_PDES, we do pt = (pd)-page_table[iter] with an
iter value that is bigger than I915_PDES. Of course, this isn't really
a problem since no one uses pt outside the macro. Still, every single
new usage of the macro will create a new issue for us to mark as a
false possitive.
After the commit mentioned above we also created some new versions of
the macros, so they carry the same problem.
In order to solve this problem, let's leave the macro with a NULL
value for pt. So if somebody uses it, we're more likely to get a big
error message instead of some silent failure. I hope the static
analyzer won't complain about the new solution (I don't have a way to
check this!).
I know, the solution looks really ugly. I am hoping the reviewers will
help us decide if we prefer this patch or if we prefer to keep marking
things as false positives.
Cc: Michel Thierry michel.thie...@intel.com
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_gem_gtt.h | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
I sent this as an RFC because I really don't know if complicating the
macro even more will help us in any way. I won't really be surprised
if I see NACKs on this patch, so don't hesitate if you want to.
Also, all I did was boot a Kernel with this patch and make sure it
shows the desktop. So consider this as untested, possibly broken.
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 0d46dd2..b202ca0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -352,7 +352,8 @@ struct i915_hw_ppgtt {
*/
Overallocate page_table etc by one and put a NULL sentinel in it.
for ((iter) = gen6_pde_index(start); \
(length) 0 (pt = (pd)-page_table[iter]); \
(iter)++, \
temp = ALIGN(start+1, 1 GEN6_PDE_SHIFT) - start, \
temp = min_t(unsigned, temp, length), \
-Chris
This might trigger different warnings from some static analysers, as
'pt' doesn't get assigned at all if length == 0.
.Dave.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 18/19] drm/i915: Remove transitional references from intel_plane_atomic_check.
All transitional plane helpers are gone, party!
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_atomic_plane.c | 19 ++-
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 10a8ecedc942..f1ab8e4b9c11 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -115,6 +115,7 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
struct intel_crtc_state *crtc_state;
struct intel_plane *intel_plane = to_intel_plane(plane);
struct intel_plane_state *intel_state = to_intel_plane_state(state);
+ struct drm_crtc_state *drm_crtc_state;
int ret;
crtc = crtc ? crtc : plane-state-crtc;
@@ -129,19 +130,11 @@ static int intel_plane_atomic_check(struct drm_plane
*plane,
if (!crtc)
return 0;
- /* FIXME: temporary hack necessary while we still use the plane update
-* helper. */
- if (state-state) {
- struct drm_crtc_state *drm_crtc_state =
- drm_atomic_get_existing_crtc_state(state-state, crtc);
+ drm_crtc_state = drm_atomic_get_existing_crtc_state(state-state, crtc);
+ if (WARN_ON(!drm_crtc_state))
+ return -EINVAL;
- if (WARN_ON(!drm_crtc_state))
- return -EINVAL;
-
- crtc_state = to_intel_crtc_state(drm_crtc_state);
- } else {
- crtc_state = intel_crtc-config;
- }
+ crtc_state = to_intel_crtc_state(drm_crtc_state);
/*
* The original src/dest coordinates are stored in state-base, but
@@ -191,7 +184,7 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
intel_state-visible = false;
ret = intel_plane-check_plane(plane, crtc_state, intel_state);
- if (ret || !state-state)
+ if (ret)
return ret;
return intel_plane_atomic_calc_changes(crtc_state-base, state);
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Warning: Broadwell/i915: Unclaimed register detected before reading register 0x130040
On Mon, Jun 15, 2015 at 12:25:10PM +0200, Daniel Vetter wrote: On Mon, May 25, 2015 at 08:04:45PM +0200, Dominik Brodowski wrote: Hey, this just popped up in the dmesg of my Dell XPS 13 -- the system seems to run well, but still, it asks about being cut and sent, so here it is. It is on 4.1.0-rc4+ (Linus' tree as of May 24th, around 3pm UTC -- don't have the git commit ID anymore). Can you please boot with i915.mmio_debug=10 and try to reproduce the issue? It won't help. To see this WARN implies that hsw_unclaimed_reg_detect() fired and mmio debugging is enabled, and the WARN then fires before a subsequent register access. So it implies the unclaimed register is not being accessed by the display driver -- presuming that we have wrapped all register access appropriately. invalid register access hsw_unclaimed_reg_detect - fires, clears the invalid flag ... ( hsw_unclaimed_reg_debug before mmio hsw_unclaimed_reg_debug after ) - nothing x N ... invalid register access hsw_unclaimed_reg_debug - fires before, ergo no more useful information Or else I nerfed the automatic debugging too much. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix failure paths around initial fbdev allocation
From: Tvrtko Ursulin tvrtko.ursu...@intel.com
We had two failure modes here:
1.
Deadlock in intelfb_alloc failure path where it calls drm_framebuffer_remove,
which grabs the struct mutex and intelfb_create (caller of intelfb_alloc) was
already holding it.
2.
Double unreference on the object if __intel_framebuffer_create fails since
both it and the caller (intelfb_alloc) do the unreference.
Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_fbdev.c | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index 6372cfc..b998f69 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -141,7 +141,7 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
{
struct intel_fbdev *ifbdev =
container_of(helper, struct intel_fbdev, helper);
- struct drm_framebuffer *fb;
+ struct drm_framebuffer *fb = NULL;
struct drm_device *dev = helper-dev;
struct drm_mode_fb_cmd2 mode_cmd = {};
struct drm_i915_gem_object *obj;
@@ -159,6 +159,8 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes-surface_bpp,
sizes-surface_depth);
+ mutex_lock(dev-struct_mutex);
+
size = mode_cmd.pitches[0] * mode_cmd.height;
size = PAGE_ALIGN(size);
obj = i915_gem_object_create_stolen(dev, size);
@@ -172,8 +174,9 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
fb = __intel_framebuffer_create(dev, mode_cmd, obj);
if (IS_ERR(fb)) {
+ /* Drops object reference on failure. */
ret = PTR_ERR(fb);
- goto out_unref;
+ goto out;
}
/* Flush everything out, we'll be doing GTT only from now on */
@@ -183,15 +186,18 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
goto out_fb;
}
+ mutex_unlock(dev-struct_mutex);
+
ifbdev-fb = to_intel_framebuffer(fb);
return 0;
out_fb:
- drm_framebuffer_remove(fb);
-out_unref:
drm_gem_object_unreference(obj-base);
out:
+ mutex_unlock(dev-struct_mutex);
+ if (fb)
+ drm_framebuffer_remove(fb);
return ret;
}
@@ -209,8 +215,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
int size, ret;
bool prealloc = false;
- mutex_lock(dev-struct_mutex);
-
if (intel_fb
(sizes-fb_width intel_fb-base.width ||
sizes-fb_height intel_fb-base.height)) {
@@ -225,7 +229,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
DRM_DEBUG_KMS(no BIOS fb, allocating a new one\n);
ret = intelfb_alloc(helper, sizes);
if (ret)
- goto out_unlock;
+ return ret;
intel_fb = ifbdev-fb;
} else {
DRM_DEBUG_KMS(re-using BIOS fb\n);
@@ -237,6 +241,8 @@ static int intelfb_create(struct drm_fb_helper *helper,
obj = intel_fb-obj;
size = obj-base.size;
+ mutex_lock(dev-struct_mutex);
+
info = framebuffer_alloc(0, dev-pdev-dev);
if (!info) {
ret = -ENOMEM;
@@ -307,7 +313,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
out_unpin:
i915_gem_object_ggtt_unpin(obj);
drm_gem_object_unreference(obj-base);
-out_unlock:
mutex_unlock(dev-struct_mutex);
return ret;
}
--
2.4.2
___
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: Fix command parser to validate multiple register access with the same command.
Daniel Vetter dan...@ffwll.ch writes: On Tue, Jun 02, 2015 at 05:36:26PM +0800, Zhigang Gong wrote: The patchset LGTM and works well with beignet. The 80%+ performance regression issue in darktable also has been fixed after this patchset applied and enable the atomic in L3 at beignet side. So, Reviewed-by: Zhigang Gong zhigang.g...@linux.intel.com All three merged. Thanks Daniel. Aside: Dont we need an increment for the cmd parser version for userspace to be able to detect this? Yeah, that would be a good idea, patch attached. And please follow up with a link to the beignet patches used to validate these kernel patches for references. Zhigang, do you have a link to your Beignet patch? Thanks, Daniel Thanks, Zhigang Gong. From 9f26beaf96473800252db35c4513933ae43e3c84 Mon Sep 17 00:00:00 2001 From: Francisco Jerez curroje...@riseup.net Date: Mon, 15 Jun 2015 14:03:29 +0300 Subject: [PATCH] drm/i915: Bump command parser version number. Signed-off-by: Francisco Jerez curroje...@riseup.net --- drivers/gpu/drm/i915/i915_cmd_parser.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c index 0146fe6..6722098 100644 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -1219,6 +1219,7 @@ int i915_cmd_parser_get_version(void) * 2. Allow access to the MI_PREDICATE_SRC0 and *MI_PREDICATE_SRC1 registers. * 3. Allow access to the GPGPU_THREADS_DISPATCHED register. + * 4. L3 atomic chicken bits of HSW_SCRATCH1 and HSW_ROW_CHICKEN3. */ - return 3; + return 4; } -- 2.4.3 signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/24] drm/i915: Update power domains only on affected crtc's.
Op 15-06-15 om 13:43 schreef Daniel Vetter: On Wed, Jun 03, 2015 at 08:52:52AM +0200, Maarten Lankhorst wrote: Op 03-06-15 om 03:27 schreef Matt Roper: On Mon, Jun 01, 2015 at 03:27:07PM +0200, Maarten Lankhorst wrote: Use for_each_crtc_state to only touch affected crtc's. In order to make sure that the initial power is still set correctly we make sure modeset_update_crtc_power_domains is called during the initial modeset. Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com --- drivers/gpu/drm/i915/i915_drv.c | 3 --- drivers/gpu/drm/i915/intel_display.c | 41 +++- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index d3632c56fdf7..78ef0bb53c36 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -634,9 +634,6 @@ static int i915_drm_suspend(struct drm_device *dev) intel_display_suspend(dev); drm_modeset_unlock_all(dev); - /* suspending displays will unsets init power */ - intel_display_set_init_power(dev_priv, true); - intel_dp_mst_suspend(dev); intel_runtime_pm_disable_interrupts(dev_priv); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8e9afc55c284..4dc07602248b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5188,42 +5188,49 @@ static unsigned long get_crtc_power_domains(struct drm_crtc *crtc) return mask; } -static void modeset_update_crtc_power_domains(struct drm_atomic_state *state) +static void modeset_update_crtc_power_domains(struct drm_atomic_state *state, bool gr) What does 'gr' stand for and what does the parameter signify? It seems to just gate whether we call display.modeset_global_resources, but it's unclear to me from the commit message above in which situations we would/wouldn't want to do this and why. Well there's no point if no modeset is done to call display.modeset_global_resources. But I guess calling it power_only might be better. I wish I knew why modeset_global_resources was done in the middle, I think there's no point to do so. When doing global changes like updating the cdclk we better do that only when the hardware is guaranteed to be on. Since we could update global things both when enabling and when disabling, the only place where the hw is on in either case is in the middle. Ok. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] DC6 already programmed to be disabled
On Thu, Jun 04, 2015 at 10:41:42AM +0100, Damien Lespiau wrote: On Thu, Jun 04, 2015 at 01:39:50PM +0530, Sagar Arun Kamble wrote: The warning faced by Chandra has to corrected. This warning is not related to firmware load. Issue is - During boot DC5/6 will be already disabled and hence While enabling PW2 for the first time the check to see if DC5/6 is already disabled does not make sense. So condition should be WARN(!(I915_READ(DC_STATE_EN) DC_STATE_EN_UPTO_DC6) pg2_enabled), DC6 already programmed to be disabled.\n); Hi, we may talk about different problems here. The warn in question here is when the DMC firmware isn't loaded (because, for instance, it's not on the filesystem) Also the initial power domains tricks should result in us not trying to double-disable stuff on driver load. It works under the assumption that the bios hasn't enabled any power conservation tricks. If that assumption ever proves wrong then we need to replace the hard-coded list of initial power domains with a dynamically generated one. Unfortuantely that also means fixing up our init code to properly take power domain references, so quite a bit of work. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 01/14] drm/i915: allocate gem memory for mipi dbi cmd buffer
On Mon, Jun 01, 2015 at 02:03:15PM +0300, Ville Syrjälä wrote: On Fri, May 29, 2015 at 07:10:53PM +0200, Daniel Vetter wrote: On Fri, May 29, 2015 at 01:59:01PM +0300, Ville Syrjälä wrote: On Fri, May 29, 2015 at 04:06:53PM +0530, Gaurav K Singh wrote: Allocate gem memory for MIPI DBI command buffer. This memory will be used when sending command via DBI interface. Why would you allocate this via gem? AFAICS you only feed the bus address to the hardware. Using the dma-api would seem like the right choice here, but I'm not sure how to deal with the dma mask. Yeah dma_alloc_coherent is what you want here. The mask can be ignored, it should be suitable already. Umm, this thing seems to limited to 32bit addresses. And we set the mask to 39 or 40 bits depending on the gen. Well that's what we have dma_set_coherent_mask for, hooray. Not the first one, see the other hacks with comments in i915_driver_load. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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 1/7] drm/i915/gen8: Add infrastructure to initialize WA batch buffers
On Fri, May 29, 2015 at 07:16:38PM +0100, Chris Wilson wrote:
On Fri, May 29, 2015 at 07:03:19PM +0100, Arun Siluvery wrote:
This patch adds functions to setup WA batch buffers but they are not yet
enabled in this patch. Some of the WA are to be applied during context save
but before restore and some at the end of context save/restore but before
executing the instructions in the ring, WA batch buffers are created for
this purpose and these WA cannot be applied using normal means.
Signed-off-by: Namrta namrta.salo...@intel.com
Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 3 ++
drivers/gpu/drm/i915/intel_lrc.c | 101
+++
2 files changed, 104 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 731b5ce..dd4b31d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -814,6 +814,9 @@ struct intel_context {
/* Execlists */
bool rcs_initialized;
+ struct intel_ringbuffer *indirect_ctx_wa_bb;
+ struct intel_ringbuffer *per_ctx_wa_bb;
Eh? They are only command sequences whose starting addresses you encode
into the execlists context. Why have you allocated a ringbuf not an
object? Why have you allocated 2 pages when you only need one, and could
even find space elsewhere in the context
And these should be pinned alongside the context *not permanently*.
I want a debug mode that limits us to say 16M of GGTT space so that
these address space leaks are easier to demonstrate in practice.
Yeah the fix up execlist context to use active list tracking series
is still pending too ... And artificially limiting ggtt space would indeed
be a neat debug trick. Maybe we'd need to have different limits for
display and other buffers just to avoid making 4k completely unuseable
with this debug knob enabled.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
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: Fix command parser to validate multiple register access with the same command.
On Mon, Jun 15, 2015 at 02:18:01PM +0300, Francisco Jerez wrote: Daniel Vetter dan...@ffwll.ch writes: On Tue, Jun 02, 2015 at 05:36:26PM +0800, Zhigang Gong wrote: The patchset LGTM and works well with beignet. The 80%+ performance regression issue in darktable also has been fixed after this patchset applied and enable the atomic in L3 at beignet side. So, Reviewed-by: Zhigang Gong zhigang.g...@linux.intel.com All three merged. Thanks Daniel. Aside: Dont we need an increment for the cmd parser version for userspace to be able to detect this? Yeah, that would be a good idea, patch attached. The old version alloweed userspace to write basically any register, the new version allows only the whitelisted registers. I don't see how a version number bump would help anyone. And please follow up with a link to the beignet patches used to validate these kernel patches for references. Zhigang, do you have a link to your Beignet patch? Thanks, Daniel Thanks, Zhigang Gong. From 9f26beaf96473800252db35c4513933ae43e3c84 Mon Sep 17 00:00:00 2001 From: Francisco Jerez curroje...@riseup.net Date: Mon, 15 Jun 2015 14:03:29 +0300 Subject: [PATCH] drm/i915: Bump command parser version number. Signed-off-by: Francisco Jerez curroje...@riseup.net --- drivers/gpu/drm/i915/i915_cmd_parser.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c index 0146fe6..6722098 100644 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -1219,6 +1219,7 @@ int i915_cmd_parser_get_version(void) * 2. Allow access to the MI_PREDICATE_SRC0 and *MI_PREDICATE_SRC1 registers. * 3. Allow access to the GPGPU_THREADS_DISPATCHED register. + * 4. L3 atomic chicken bits of HSW_SCRATCH1 and HSW_ROW_CHICKEN3. */ - return 3; + return 4; } -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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] Revert drm/i915: Don't skip request retirement if the active list is empty
On Mon, 15 Jun 2015, Ville Syrjälä ville.syrj...@linux.intel.com wrote:
On Mon, Jun 15, 2015 at 12:59:37PM +0300, Jani Nikula wrote:
This reverts commit 0aedb1626566efd72b369c01992ee7413c82a0c5.
I messed things up while applying [1] to drm-intel-fixes. Rectify.
[1]
http://mid.gmane.org/1432827156-9605-1-git-send-email-ville.syrj...@linux.intel.com
Fixes: 0aedb1626566 (drm/i915: Don't skip request retirement if the active
list is empty)
Cc: sta...@vger.kernel.org
Signed-off-by: Jani Nikula jani.nik...@intel.com
Acked-by: Ville Syrjälä ville.syrj...@linux.intel.com
Pushed to drm-intel-fixes.
BR,
Jani.
However this leaves us with an early bailout if request_list is empty.
Hopefully that won't cause other issues. I suppose we shouldn't have
stuff on the active_list w/o any pending requests.
---
Note to stable team: please do *not* backport
commit 0aedb1626566efd72b369c01992ee7413c82a0c5
Author: Ville Syrjälä ville.syrj...@linux.intel.com
Date: Thu May 28 18:32:36 2015 +0300
drm/i915: Don't skip request retirement if the active list is empty
but if you did, please backport this revert as well.
Thanks, and sorry.
BR,
Jani.
---
drivers/gpu/drm/i915/i915_gem.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index c3806c66650a..2d0995e7afc3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2656,6 +2656,9 @@ void i915_gem_reset(struct drm_device *dev)
void
i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
{
+if (list_empty(ring-request_list))
+return;
+
WARN_ON(i915_verify_lists(ring-dev));
/* Retire requests first as we use it above for the early return.
--
2.1.4
--
Ville Syrjälä
Intel OTC
--
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 igt 1/4] tests/template: add IGT_TEST_DESCRIPTION
On Thu, Jun 04, 2015 at 11:31:03AM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com So people that write tests based on the template don't forget to use the macro. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com With the nit to not try to enable fbc on pre-gen6 address the series is Acked-by: Daniel Vetter daniel.vet...@ffwll.ch --- tests/template.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/template.c b/tests/template.c index 24fd850..4b5794b 100644 --- a/tests/template.c +++ b/tests/template.c @@ -26,6 +26,8 @@ #include drmtest.h +IGT_TEST_DESCRIPTION(Template test.); + /* * Note that test function (and code called by them) should generally not return * a variable indicating success/failure. Instead use the igt_require/igt_assert -- 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 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: drrs_invalidate at flip schedule
On Thu, Jun 11, 2015 at 02:57:52PM +0530, Ramalingam C wrote:
Sorry for late response. I was away for longer.
Daniel,
As we have the intel_frontbuffer_flush, I have created the
intel_frontbuffer_invalidate.
This can be called from flip preparation notification to handle the
frontbuffer invalidation.
I will share the patches now.
You need to fix up the broken DRRS code - the frontbuffer tracking code is
perfectly fine. See below diff with inline comments.
Cheers, Daniel
And my Signed-off-by: Daniel Vetter daniel.vet...@intel.com in case the
patch works as-is.
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f73da99e66b8..b96a4abb7a98 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5487,6 +5487,7 @@ void intel_edp_drrs_invalidate(struct drm_device *dev,
crtc = dp_to_dig_port(dev_priv-drrs.dp)-base.base.crtc;
pipe = to_intel_crtc(crtc)-pipe;
+ /* invalidate means busy screen hence upclock */
if (dev_priv-drrs.refresh_rate_type == DRRS_LOW_RR) {
intel_dp_set_drrs_state(dev_priv-dev,
dev_priv-drrs.dp-attached_connector-panel.
@@ -5532,8 +5533,16 @@ void intel_edp_drrs_flush(struct drm_device *dev,
pipe = to_intel_crtc(crtc)-pipe;
dev_priv-drrs.busy_frontbuffer_bits = ~frontbuffer_bits;
- if (dev_priv-drrs.refresh_rate_type != DRRS_LOW_RR
- !dev_priv-drrs.busy_frontbuffer_bits)
+ /* flush means busy screen hence upclock */
+ if (dev_priv-drrs.refresh_rate_type == DRRS_LOW_RR) {
+ intel_dp_set_drrs_state(dev_priv-dev,
+ dev_priv-drrs.dp-attached_connector-panel.
+ fixed_mode-vrefresh);
+ }
+
+ /* flush also means no more activity hence schedule downclock if all
+* other fbs are quiescent too */
+ if (!dev_priv-drrs.busy_frontbuffer_bits)
schedule_delayed_work(dev_priv-drrs.work,
msecs_to_jiffies(1000));
mutex_unlock(dev_priv-drrs.mutex);
--
Daniel Vetter
Software Engineer, Intel Corporation
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] Fix resume from suspend to RAM on IBM X30 (take two)
On Mon, Jun 01, 2015 at 10:25:10AM +0200, Thomas Richter wrote: Hi Daniel, hi Ville, as suggested, I again checked against the latest drm-intel-nightly branch and indeed, the patch as originally submitted does not apply there. So I recompiled the patch again (see attachment) which works now correctly with the latest branch. As suggested, I made a couple of additional changes: * No longer forces bit-banging on. As Daniel observed, this is not really required, and I checked that, indeed, it is not. * Ville made a comment a while ago that we should not enable the extended panel-scaling of the IVCH, so I removed it. In fact, as the X30 bios does not restore registers during resume, it also fails to restore the (undocumented) values of the advanced interpolation polynomials and as such degrades the image quality when resuming from suspend. Other than that, this patch has been successfully tested both on an IBM X30 and on an IBM R31 ThinkPad, both making use of the IVCH DVO chip. Yup this one applies cleanly now. Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Warning: Broadwell/i915: Unclaimed register detected before reading register 0x130040
On Mon, May 25, 2015 at 08:04:45PM +0200, Dominik Brodowski wrote: Hey, this just popped up in the dmesg of my Dell XPS 13 -- the system seems to run well, but still, it asks about being cut and sent, so here it is. It is on 4.1.0-rc4+ (Linus' tree as of May 24th, around 3pm UTC -- don't have the git commit ID anymore). Can you please boot with i915.mmio_debug=10 and try to reproduce the issue? That should light more onto this issue. And yup it's indeed a bug since this means a register write has been dropped by the hardware, which often tends to be somewhat harmless except when it doesn't ;-) -Daniel [ cut here ] WARNING: CPU: 0 PID: 1167 at /home/brodo/local/kernel/git/linux/drivers/gpu/drm/i915/intel_uncore.c:566 hsw_unclaimed_reg_debug+0x7d/0xa0() Unclaimed register detected before reading register 0x130040 Modules linked in: CPU: 0 PID: 1167 Comm: Xorg Not tainted 4.1.0-rc4+ #1 Hardware name: Dell Inc. XPS 13 9343/0TM99H, BIOS A03 03/25/2015 b39694d8 88020e14b998 b322906f 8001 88020e14b9e8 88020e14b9d8 b272ca8a 88020e14b9a8 88021519 00130040 880215bfd800 00130040 Call Trace: [b322906f] dump_stack+0x4f/0x7b [b272ca8a] warn_slowpath_common+0x8a/0xc0 [b272cb06] warn_slowpath_fmt+0x46/0x50 [b2be63cd] hsw_unclaimed_reg_debug+0x7d/0xa0 [b2be84b6] gen6_read32+0x56/0x1c0 [b2c096e6] hsw_disable_pc8+0x36/0x2c0 [b2b98e60] intel_runtime_resume+0x160/0x1b0 [b2a94bdf] pci_pm_runtime_resume+0x7f/0xc0 [b2a94b60] ? pci_restore_standard_config+0x50/0x50 [b2c5c8f6] __rpm_callback+0x36/0x90 [b2c5c976] rpm_callback+0x26/0xa0 [b2c5dcc6] rpm_resume+0x496/0x670 [b2756209] ? preempt_count_add+0x79/0xc0 [b2c5dee0] __pm_runtime_resume+0x40/0x60 [b2ba7fd5] intel_runtime_pm_get+0x55/0x80 [b2bc6989] i915_gem_free_object+0x29/0x360 [b2b6b417] drm_gem_object_free+0x27/0x40 [b2b6b548] drm_gem_object_handle_unreference_unlocked+0x118/0x130 [b2b6b6d4] drm_gem_handle_delete+0xb4/0x100 [b2fde8ab] ? sock_recvmsg+0x3b/0x50 [b2b6ba95] drm_gem_close_ioctl+0x25/0x30 [b2b6c4a5] drm_ioctl+0x1a5/0x6a0 [b2869672] ? do_readv_writev+0x1b2/0x260 [b323187a] ? _raw_spin_unlock_irqrestore+0x2a/0x60 [b2a5b804] ? timerqueue_del+0x24/0x70 [b278b2fe] ? __remove_hrtimer+0x4e/0xe0 [b287c110] do_vfs_ioctl+0x2e0/0x4e0 [b2886482] ? __fget+0x72/0xb0 [b287c391] SyS_ioctl+0x81/0xa0 [b3232017] system_call_fastpath+0x12/0x6a ---[ end trace 8ff5864d3e587195 ]--- Best, Dominik ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation 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 3/3] drm/i915: Bump CHV PFI credits to 63 when cdclk=czclk
On Fri, Jun 12, 2015 at 09:24:24AM -0700, Clint Taylor wrote:
On 05/26/2015 10:22 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Switch from using 31 PFI credits to 63 PFI credits when cdclk=czclk on
CHV. The spec lists both 31 and 63 as suggested values, but based on
feedback from hardware folks we should actually be using 63. Originally
I picked the 31 basically by flipping a coin.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
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 067b1de..44b9c54 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5986,7 +5986,7 @@ static void vlv_program_pfi_credits(struct
drm_i915_private *dev_priv)
if (DIV_ROUND_CLOSEST(dev_priv-cdclk_freq, 1000) =
dev_priv-rps.cz_freq) {
/* CHV suggested value is 31 or 63 */
if (IS_CHERRYVIEW(dev_priv))
-credits = PFI_CREDIT_31;
+credits = PFI_CREDIT_63;
else
credits = PFI_CREDIT(15);
} else {
Although not part of this review the else clause is setting PFI_CREDIT to 15
when the BPSEC states that the default of 8 should be used when cdclk/czclk
1. According to the original patch, 15 is the optimal value as stated by
another driver team.
Can you please file a bspec correction notice to fix the issue with the 15
vs. 8?
Reviewed-by: Clint Taylor clinton.a.tay...@intel.com
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 11/14] drm/i915: Enable MIPI display self refresh mode
On Sat, Jun 13, 2015 at 12:24:57PM +0530, Mohan Marimuthu, Yogesh wrote:
On 5/29/2015 10:51 PM, Daniel Vetter wrote:
On Fri, May 29, 2015 at 04:07:03PM +0530, Gaurav K Singh wrote:
During enable sequence for MIPI encoder in command mode, enable
MIPI display self-refresh mode bit in Pipe Ctrl reg.
Signed-off-by: Gaurav K Singh gaurav.k.si...@intel.com
Signed-off-by: Yogesh Mohan Marimuthu yogesh.mohan.marimu...@intel.com
Signed-off-by: Shobhit Kumar shobhit.ku...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index cab2ac8..fc84313 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -44,6 +44,7 @@
#include drm/drm_plane_helper.h
#include drm/drm_rect.h
#include linux/dma_remapping.h
+#include intel_dsi.h
/* Primary plane formats supported by all gen */
#define COMMON_PRIMARY_FORMATS \
@@ -2110,6 +2111,8 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
{
struct drm_device *dev = crtc-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_encoder *encoder;
+ struct intel_dsi *intel_dsi;
enum pipe pipe = crtc-pipe;
enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
pipe);
@@ -2154,6 +2157,18 @@ static void intel_enable_pipe(struct intel_crtc
*crtc)
return;
}
+ for_each_encoder_on_crtc(dev, crtc-base, encoder) {
+ if (encoder-type == INTEL_OUTPUT_DSI) {
+ intel_dsi = enc_to_intel_dsi(encoder-base);
+ if (intel_dsi (intel_dsi-operation_mode ==
+ INTEL_DSI_COMMAND_MODE)) {
+ val = val | PIPECONF_MIPI_DSR_ENABLE;
+ I915_WRITE(reg, val);
+ }
+ break;
+ }
+ }
When we have these kind of encoder/crtc state depencies we resolve them by
adding a bit of state to intel_crtc_state which is set as needed in the
encoder's compute_config callback. Then all you need here is
if (intel_state-dsi_self_refresh)
val |= PIPECONF_MIPI_DSR_ENABLE;
Also is that additional write really required?
-Daniel
Yes additional write is required. MIPI_DSR_ENABLE has to be written first
then followed
by pipe enable. if MIPI_DSR_ENABLE and pipe enable is done in same write,
observed
that the image from pipe is not sent to panel when issued mem write command.
Having a state variable instead of looping through the encoders definitely
looks good.
Need to find a place to update the state variable. I will get back on this.
Like I said such state is precomputed in the encoder callbacks, in this
case intel_dsi_compute_config.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
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 v6 2/8] drm/i915: Use cached cdclk value
On Wed, Jun 03, 2015 at 03:45:08PM +0300, Mika Kahola wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Rather than reading out the current cdclk value use the cached value we
have tucked away in dev_priv.
v2: Rebased to the latest
v3: Rebased to the latest
v4: Fix for patch style problems
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Mika Kahola mika.kah...@intel.com
This patch needs to be extended to also cover the recently added
skl_max_scale. Tvrtko has recently written a patch to add some checks to
that code too, would be good to resurrect that too. Chandra can help with
any questions wrt the skl scaler code.
Cheers, Daniel
Author:Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 3 +--
drivers/gpu/drm/i915/intel_dp.c | 5 +++--
drivers/gpu/drm/i915/intel_pm.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 9cf1553..d1dd8ab 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6610,8 +6610,7 @@ static int intel_crtc_compute_config(struct intel_crtc
*crtc,
/* FIXME should check pixel clock limits on all platforms */
if (INTEL_INFO(dev)-gen 4) {
- int clock_limit =
- dev_priv-display.get_display_clock_speed(dev);
+ int clock_limit = dev_priv-cdclk_freq;
/*
* Enable pixel doubling when the dot clock
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6f525093..9a6517d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -708,7 +708,8 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp
*intel_dp, int index)
return 0;
if (intel_dig_port-port == PORT_A) {
- return
DIV_ROUND_UP(dev_priv-display.get_display_clock_speed(dev), 2000);
+ return DIV_ROUND_UP(dev_priv-cdclk_freq, 2000);
+
} else {
return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
}
@@ -723,7 +724,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp
*intel_dp, int index)
if (intel_dig_port-port == PORT_A) {
if (index)
return 0;
- return
DIV_ROUND_CLOSEST(dev_priv-display.get_display_clock_speed(dev), 2000);
+ return DIV_ROUND_CLOSEST(dev_priv-cdclk_freq, 2000);
} else if (dev_priv-pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
/* Workaround for non-ULT HSW */
switch (index) {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index eadc15c..5db429e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1815,7 +1815,7 @@ hsw_compute_linetime_wm(struct drm_device *dev, struct
drm_crtc *crtc)
linetime = DIV_ROUND_CLOSEST(mode-crtc_htotal * 1000 * 8,
mode-crtc_clock);
ips_linetime = DIV_ROUND_CLOSEST(mode-crtc_htotal * 1000 * 8,
-
dev_priv-display.get_display_clock_speed(dev_priv-dev));
+ dev_priv-cdclk_freq);
return PIPE_WM_LINETIME_IPS_LINETIME(ips_linetime) |
PIPE_WM_LINETIME_TIME(linetime);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
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 igt 3/4] tests/kms_fbc_crc: run even if FBC is disabled by default
On Thu, Jun 04, 2015 at 11:31:05AM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com We may not be perfect, but if we don't even test, we will probably only get worse over time. The function called makes sure we restore whatever was the original FBC parameter when we exit the test, so this should not affect the other tests. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- tests/kms_fbc_crc.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c index 37221ac..d964224 100644 --- a/tests/kms_fbc_crc.c +++ b/tests/kms_fbc_crc.c @@ -559,10 +559,10 @@ igt_main igt_assert_lt(0, fread(buf, 1, sizeof(buf), status)); fclose(status); buf[sizeof(buf) - 1] = '\0'; - igt_require_f(!strstr(buf, unsupported on this chipset) - !strstr(buf, disabled per module param) - !strstr(buf, disabled per chip default), - FBC not supported/enabled\n); + igt_require_f(!strstr(buf, unsupported on this chipset), + FBC not supported\n); + + igt_set_module_param_int(enable_fbc, 1); This is risky since on older chips fbc is known to pretty much insta-kill the box. Imo we should have a gen6+ here at least (that's roughly the point where the hw workarounds start to sound less scary). Trying to enable this on older platforms just isn't worth it imo. -Daniel data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096); igt_assert(data.bufmgr); -- 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 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/vlv: fix RC6 residency time calculation
On Tue, Jun 02, 2015 at 12:37:13PM +0300, Imre Deak wrote:
On ma, 2015-06-01 at 12:01 -0700, Rodrigo Vivi wrote:
On Mon, Jun 1, 2015 at 12:32 AM, Imre Deak imre.d...@intel.com wrote:
The divider value to convert from CZ clock rate to ms needs a +1
adjustment on VLV just like on CHV. This matches both the spec and
the accuracy test by pm_rc6_residency.
v2:
- simplify logic checking for the CHV 320MHz special case (Rodrigo)
Testcase: igt/pm_rc6_residency
Signed-off-by: Imre Deak imre.d...@intel.com
---
drivers/gpu/drm/i915/i915_sysfs.c | 22 +++---
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c
b/drivers/gpu/drm/i915/i915_sysfs.c
index 2476268..55bd04c 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -64,24 +64,16 @@ static u32 calc_residency(struct drm_device *dev,
const u32 reg)
goto out;
}
- units = 0;
- div = 100ULL;
-
- if (IS_CHERRYVIEW(dev)) {
+ if (IS_CHERRYVIEW(dev) czcount_30ns == 1) {
/* Special case for 320Mhz */
- if (czcount_30ns == 1) {
- div = 1000ULL;
- units = 3125ULL;
- } else {
- /* chv counts are one less */
- czcount_30ns += 1;
- }
+ div = 1000ULL;
+ units = 3125ULL;
+ } else {
+ czcount_30ns += 1;
+ div = 100ULL;
+ units = DIV_ROUND_UP_ULL(30ULL * bias,
czcount_30ns);
Is (u64) cast unnecessary?
Yes. Here the only reason for it would be overflow, but that's not
possible.
But reading like this now I wonder if we couldn't just pass
czcount_30ns+1 instead of the increment...
But if we don't need the cast let's please just ignore this bikeshed
and let's move fwd! ;)
Yes, I think we could do more cleanup in this function as a follow-up.
For example we may still loose precision in the current way, would be
better to calculate the result directly from the reference clock rate
(CZ clock in case of VLV/CHV).
More organized than I had suggested, thanks.
Reviewed-by: Rodrigo Vivi rodrigo.v...@intel.com
Thanks.
Forgot to add:
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76877
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
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 1/7] drm/i915/gen8: Add infrastructure to initialize WA batch buffers
On Thu, Jun 04, 2015 at 03:30:56PM +0100, Siluvery, Arun wrote: On 02/06/2015 19:47, Dave Gordon wrote: On 02/06/15 19:36, Siluvery, Arun wrote: On 01/06/2015 11:22, Daniel, Thomas wrote: Indeed, allocating an extra scratch page in the context would simplify vma/mm management. A trick might be to allocate the scratch page at the start, then offset the lrc regs etc - that would then be consistent amongst gen and be easy enough to extend if we need more per-context scratch space in future. -Chris Yes, I think we already have another use for more per-context space at the start. The GuC is planning to do this. Arun, you probably should work with Alex Dai and Dave Gordon to avoid conflicts here. Thomas. Thanks for the heads-up Thomas. I have discussed with Dave and agreed to share this page; GuC probably doesn't need whole page so first half is reserved for it's use and second half is used for WA. I have modified my patches to use context page for applying these WA and don't see any issues. During the discussions Dave proposed another approach. Even though these WA are called per context they are only initialized once and not changed afterwards, same set of WA are applied for each context so instead of adding them in each context, does it make sense to create a separate page and share across all contexts? but of course GuC will anyway add a new page to context so I might as well share that page. Chris/Dave, do you see any problems with sharing page with GuC or you prefer to allocate a separate page for these WA and share across all contexts? Please give your comments. regards Arun I think we have to consider which is more future-proof i.e. which is least likely: (1) the area shared with the GuC grows (definitely still in flux), or (2) workarounds need to be context-specific (possible, but unlikely) So I'd prefer a single area set up just once to contain the pre- and post-context restore workaround batches. If necessary, the one area could contain multiple batches at different offsets, so we could point different contexts at different (shared) batches as required. I think they're unlikely to actually need per-context customisation[*], but there might be a need for different workarounds according to workload type or privilege level or some other criterion ... ? .Dave. [*] unless they need per-context memory addresses coded into them? Considering these WA are initialized only once and not changed afterwards and GuC area probably grows in future which may run into the space used by WA, independent single area setup makes senses. I also checked spec and it is not clear whether any customization is going to be required for different contexts. I have modified patches to setup a single page with WA when default_context is initialized and this is used by all contexts. I will send patches but please let me know if there are any other comments. Yeah if the wa batches aren't ctx specific, then there's really no need to allocate one of them per ctx. One global buffer with all the wa combined should really be all we need. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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 i-g-t] tests/gem_reset_stats : mask off ring_stop bits
On Wed, Jun 03, 2015 at 09:29:34AM +0100, Chris Wilson wrote: On Wed, Jun 03, 2015 at 09:20:21AM +0100, tim.g...@intel.com wrote: From: Tim Gore tim.g...@intel.com Function check_gpu_ok checks to make sure that any hangs have cleared by testing for (flags == 0). Some tests set the STOP_RINGS_ALLOW_BAN and STOP_RINGS_ALLOW_ERRORS flags but these do not get cleared by an individual ring reset, (a feature added recently to the driver), leading the check_gpu_ok function to think that the gpu is still hung. So I mask the flags with STOP_RING_ALL, to ignore the mode bits and look only at the bits that stop the rings. Once gpu_check_ok sees that the gpu is not hung I write 0 to stop_rings in order to clear it completely. This is because igt_set_stop_rings will only write to stop_rings if either a) they are currently 0 or b) we are writing 0. If we leave the mode bits set then subsequent calls to igt_set_stop_rings to create hangs will fail. Can we please just deprecate the stop_rings interface? We can do explicit hang injection and GPU resets on gen4+, most of gen3 but not gen2. Even if we mask of testing for gen2/3, that still provides (almost, just a couple of gen2/3 reset functions will be missed) complete coverage of GEM reset handling. Yeah I guess it's ok to nuke stop_rings. The only issue I see is that stop_rings provided a neat way for the kernel to support gpu resets on platforms where it's not working. I guess we'd need to move that knowledge to igt with an require_gpu_reset macro and skip the tests correctly. Otherwise I don't see an issue here really. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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 14/27] drm/i915: clean up atomic plane check functions
On Thu, Jun 11, 2015 at 06:23:09AM +0200, Maarten Lankhorst wrote: Op 11-06-15 om 03:35 schreef Matt Roper: On Thu, Jun 04, 2015 at 02:47:44PM +0200, Maarten Lankhorst wrote: By passing crtc_state to the check_plane functions a lot of duplicated code can be removed. And now that the transitional helpers are gone the crtc_state can be reliably obtained. Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com --- drivers/gpu/drm/i915/intel_atomic_plane.c | 4 ++- drivers/gpu/drm/i915/intel_display.c | 48 ++- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_sprite.c | 13 +++-- 4 files changed, 23 insertions(+), 43 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index aa2128369a0a..4d8cacbca777 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -119,6 +119,8 @@ static int intel_plane_atomic_check(struct drm_plane *plane, crtc = crtc ? crtc : plane-crtc; intel_crtc = to_intel_crtc(crtc); + intel_state-visible = false; + What do we need this change for? Primary and cursor check functions immediately overwrite state-visible, so setting this here has no effect. The sprite case where fb==NULL is the only case where this would matter, but moving the assignment from the sprite check function to here doesn't seem like it gains us anything. I was using it to clear intel_state-visible even if no crtc is set. In that case state-visible should already be false, but a bit of paranoia never hunts. :-) Resetting of state should be done in the duplicate functions. This makes sure we never ever forget to compute it correctly ;-) Sprinkling clearing code all over (and especially over the compute code) is imo fragile and should be avoided if possible. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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: Assume no scaling is available when things are not as expected
On Mon, Jun 01, 2015 at 01:04:37PM +0100, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com Cdclk crtc_clock is not allowed and suggests a different problem elsewhere. It is more robust and safe to assume no scaling is possible in this case. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com Cc: Ville Syrjälä ville.syrj...@linux.intel.com --- 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 93a5e51..4c99373 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13234,7 +13234,7 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state crtc_clock = crtc_state-base.adjusted_mode.crtc_clock; cdclk = dev_priv-display.get_display_clock_speed(dev); Probably fallout from the in-flight dynamic cdclk stuff - this code checks the wrong bits I guess. Chandra? Thanks, Daniel - if (!crtc_clock || !cdclk) + if (!crtc_clock || !cdclk || (cdclk crtc_clock)) return DRM_PLANE_HELPER_NO_SCALING; /* -- 2.4.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation 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: Fix failure paths around initial fbdev allocation
On Mon, Jun 15, 2015 at 11:49:52AM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin tvrtko.ursu...@intel.com
We had two failure modes here:
1.
Deadlock in intelfb_alloc failure path where it calls drm_framebuffer_remove,
which grabs the struct mutex and intelfb_create (caller of intelfb_alloc) was
already holding it.
2.
Double unreference on the object if __intel_framebuffer_create fails since
both it and the caller (intelfb_alloc) do the unreference.
I would suggest removing the unref from __intel_framebuffer_create().
Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_fbdev.c | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index 6372cfc..b998f69 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -141,7 +141,7 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
{
struct intel_fbdev *ifbdev =
container_of(helper, struct intel_fbdev, helper);
- struct drm_framebuffer *fb;
+ struct drm_framebuffer *fb = NULL;
struct drm_device *dev = helper-dev;
struct drm_mode_fb_cmd2 mode_cmd = {};
struct drm_i915_gem_object *obj;
@@ -159,6 +159,8 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes-surface_bpp,
sizes-surface_depth);
+ mutex_lock(dev-struct_mutex);
+
size = mode_cmd.pitches[0] * mode_cmd.height;
size = PAGE_ALIGN(size);
obj = i915_gem_object_create_stolen(dev, size);
@@ -172,8 +174,9 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
fb = __intel_framebuffer_create(dev, mode_cmd, obj);
if (IS_ERR(fb)) {
+ /* Drops object reference on failure. */
ret = PTR_ERR(fb);
- goto out_unref;
+ goto out;
}
/* Flush everything out, we'll be doing GTT only from now on */
@@ -183,15 +186,18 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
goto out_fb;
}
+ mutex_unlock(dev-struct_mutex);
+
ifbdev-fb = to_intel_framebuffer(fb);
return 0;
out_fb:
- drm_framebuffer_remove(fb);
-out_unref:
drm_gem_object_unreference(obj-base);
out:
+ mutex_unlock(dev-struct_mutex);
+ if (fb)
+ drm_framebuffer_remove(fb);
return ret;
}
@@ -209,8 +215,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
int size, ret;
bool prealloc = false;
- mutex_lock(dev-struct_mutex);
-
if (intel_fb
(sizes-fb_width intel_fb-base.width ||
sizes-fb_height intel_fb-base.height)) {
@@ -225,7 +229,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
DRM_DEBUG_KMS(no BIOS fb, allocating a new one\n);
ret = intelfb_alloc(helper, sizes);
if (ret)
- goto out_unlock;
+ return ret;
intel_fb = ifbdev-fb;
} else {
DRM_DEBUG_KMS(re-using BIOS fb\n);
@@ -237,6 +241,8 @@ static int intelfb_create(struct drm_fb_helper *helper,
obj = intel_fb-obj;
size = obj-base.size;
+ mutex_lock(dev-struct_mutex);
+
info = framebuffer_alloc(0, dev-pdev-dev);
if (!info) {
ret = -ENOMEM;
@@ -307,7 +313,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
out_unpin:
i915_gem_object_ggtt_unpin(obj);
drm_gem_object_unreference(obj-base);
-out_unlock:
mutex_unlock(dev-struct_mutex);
return ret;
}
--
2.4.2
--
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] Deadlock in intel_user_framebuffer_destroy()
On Mon, Jun 15, 2015 at 02:02:23PM +0200, Daniel Vetter wrote: On Mon, Jun 15, 2015 at 08:53:02AM +0100, Chris Wilson wrote: On Mon, Jun 15, 2015 at 09:44:15AM +0300, Jani Nikula wrote: On Wed, 03 Jun 2015, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Jun 03, 2015 at 03:43:32PM +0200, Lukas Wunner wrote: Hi, a deadlock was introduced by commit 60a5ca015ffd2aacfe5674b5a401cd2a37159e07 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Fri Jun 13 11:10:53 2014 +0300 drm/i915: Add locking around framebuffer_references-- The commit amended intel_display.c:intel_user_framebuffer_destroy() with mutex_lock(dev-struct_mutex). A few weeks prior Chris Wilson had amended intel_fbdev.c:intelfb_create() with a call to drm_framebuffer_unreference() while dev-struct_mutex is locked (commit edd586fe705e819bc711b5ed7194a0b6f9f1a7e1, drm/i915: Discard BIOS framebuffers too small to accommodate chosen mode). Just move the mutex_lock down a step. Lucas, did you try this? There's a goto unlock that also needed to be disabled, such as Your previous patch placed the mutex_lock before the goto out_unlock - I fail to see what has been broken with that version? Can you resubmit that as a proper patch with sob and Lucas' t-b? There wasn't, I just rewrote it incorrectly. There's also a drm_framebuffer_remove() called by intelfb_alloc which needs to be moved out of the mutex. A much larger disentangling of the functions involved here is required. Tvrstko volunteered himself. Brave, very brave :) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix build without CONFIG_PM
On Mon, Jun 15, 2015 at 12:52:28PM +0100, Chris Wilson wrote: drivers/gpu/drm/i915/i915_debugfs.c: In function ‘i915_runtime_pm_status’: drivers/gpu/drm/i915/i915_debugfs.c:2528:34: error: ‘struct dev_pm_info’ has no member named ‘usage_count’ atomic_read(dev-dev-power.usage_count)); Regression from commit a6aaec8be22652a808d6e316d4a92e58cb75e986 Author: Damien Lespiau damien.lesp...@intel.com Date: Thu Jun 4 18:23:58 2015 +0100 drm/i915: Add runtime PM's usage_count in i915_runtime_pm_status Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Damien Lespiau damien.lesp...@intel.com Cc: Paulo Zanoni paulo.r.zan...@intel.com Cc: Jani Nikula jani.nik...@intel.com Eeek. Reviewed-by: Damien Lespiau damien.lesp...@intel.com -- Damien --- drivers/gpu/drm/i915/i915_debugfs.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 8f9554a61019..bd7a5fbf3959 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2524,8 +2524,12 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused) seq_printf(m, GPU idle: %s\n, yesno(!dev_priv-mm.busy)); seq_printf(m, IRQs disabled: %s\n, yesno(!intel_irqs_enabled(dev_priv))); +#ifdef CONFIG_PM seq_printf(m, Usage count: %d\n, atomic_read(dev-dev-power.usage_count)); +#else + seq_printf(m, Device Power Management (CONFIG_PM) disabled\n); +#endif return 0; } -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/21] drm/i915/gtt: Remove _single from page table allocator
On Tue, Jun 02, 2015 at 10:56:55AM +0100, Michel Thierry wrote: On 5/22/2015 6:04 PM, Mika Kuoppala wrote: We are always allocating a single page. No need to be verbose so remove the suffix. Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com I saw another of your patches will take care of i915_dma_map_single/i915_dma_unmap_single... Reviewed-by: Michel Thierry michel.thie...@intel.com Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 12/19] drm/i915: Move crtc commit updates to separate functions.
To allow them to be used in intel_set_mode.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 127 +++
1 file changed, 69 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5e77c2974288..6ae9bd0e0283 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4753,6 +4753,72 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
hsw_disable_ips(intel_crtc);
}
+static void intel_post_plane_update(struct intel_crtc *crtc)
+{
+ struct intel_crtc_atomic_commit *atomic = crtc-atomic;
+ struct drm_device *dev = crtc-base.dev;
+ struct drm_plane *plane;
+
+ if (atomic-wait_vblank)
+ intel_wait_for_vblank(dev, crtc-pipe);
+
+ intel_frontbuffer_flip(dev, atomic-fb_bits);
+
+ if (atomic-update_fbc) {
+ mutex_lock(dev-struct_mutex);
+ intel_fbc_update(dev);
+ mutex_unlock(dev-struct_mutex);
+ }
+
+ if (atomic-post_enable_primary)
+ intel_post_enable_primary(crtc-base);
+
+ drm_for_each_plane_mask(plane, dev, atomic-update_sprite_watermarks)
+ intel_update_sprite_watermarks(plane, crtc-base,
+ 0, 0, 0, false, false);
+
+ memset(atomic, 0, sizeof(*atomic));
+}
+
+static void intel_pre_plane_update(struct intel_crtc *crtc)
+{
+ struct drm_device *dev = crtc-base.dev;
+ struct intel_crtc_atomic_commit *atomic = crtc-atomic;
+ struct drm_plane *p;
+
+ /* Track fb's for any planes being disabled */
+
+ drm_for_each_plane_mask(p, dev, atomic-disabled_planes) {
+ struct intel_plane *plane = to_intel_plane(p);
+ unsigned fb_bits = 0;
+
+ switch (p-type) {
+ case DRM_PLANE_TYPE_PRIMARY:
+ fb_bits = INTEL_FRONTBUFFER_PRIMARY(plane-pipe);
+ break;
+ case DRM_PLANE_TYPE_CURSOR:
+ fb_bits = INTEL_FRONTBUFFER_CURSOR(plane-pipe);
+ break;
+ case DRM_PLANE_TYPE_OVERLAY:
+ fb_bits = INTEL_FRONTBUFFER_SPRITE(plane-pipe);
+ break;
+ }
+
+ mutex_lock(dev-struct_mutex);
+ i915_gem_track_fb(intel_fb_obj(plane-base.fb), NULL, fb_bits);
+ mutex_unlock(dev-struct_mutex);
+ }
+
+ if (atomic-wait_for_flips)
+ intel_crtc_wait_for_pending_flips(crtc-base);
+
+ if (atomic-disable_fbc)
+ intel_fbc_disable(dev);
+
+ if (atomic-pre_disable_primary)
+ intel_pre_disable_primary(crtc-base);
+}
+
static void intel_crtc_enable_planes(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc-dev;
@@ -13768,43 +13834,8 @@ static void intel_begin_crtc_commit(struct drm_crtc
*crtc,
struct drm_device *dev = crtc-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct drm_crtc_state *crtc_state = intel_crtc-base.state;
- struct intel_plane *intel_plane;
- struct drm_plane *p;
- unsigned fb_bits = 0;
-
- /* Track fb's for any planes being disabled */
- list_for_each_entry(p, dev-mode_config.plane_list, head) {
- intel_plane = to_intel_plane(p);
-
- if (intel_crtc-atomic.disabled_planes
- (1 drm_plane_index(p))) {
- switch (p-type) {
- case DRM_PLANE_TYPE_PRIMARY:
- fb_bits =
INTEL_FRONTBUFFER_PRIMARY(intel_plane-pipe);
- break;
- case DRM_PLANE_TYPE_CURSOR:
- fb_bits =
INTEL_FRONTBUFFER_CURSOR(intel_plane-pipe);
- break;
- case DRM_PLANE_TYPE_OVERLAY:
- fb_bits =
INTEL_FRONTBUFFER_SPRITE(intel_plane-pipe);
- break;
- }
-
- mutex_lock(dev-struct_mutex);
- i915_gem_track_fb(intel_fb_obj(p-fb), NULL, fb_bits);
- mutex_unlock(dev-struct_mutex);
- }
- }
-
- if (intel_crtc-atomic.wait_for_flips)
- intel_crtc_wait_for_pending_flips(crtc);
-
- if (intel_crtc-atomic.disable_fbc)
- intel_fbc_disable(dev);
- if (intel_crtc-atomic.pre_disable_primary)
- intel_pre_disable_primary(crtc);
+ intel_pre_plane_update(intel_crtc);
if (intel_crtc-atomic.update_wm)
intel_update_watermarks(crtc);
@@ -13812,7 +13843,7 @@ static void intel_begin_crtc_commit(struct drm_crtc
*crtc,
[Intel-gfx] [PATCH v3 13/19] drm/i915: Do not run most checks when there's no modeset.
All the checks in intel_modeset_checks are only useful when a modeset
occurs, because there is nothing to update otherwise.
Same for power/cdclk changes, if there is no modeset they are noops.
Unfortunately intel_modeset_pipe_config still gets called without
modeset, because atomic hw readout isn't done yet.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 33 ++---
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 6ae9bd0e0283..ae046648c29d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13157,18 +13157,18 @@ intel_modeset_compute_config(struct drm_atomic_state
*state)
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
int ret, i;
+ bool any_ms = false;
ret = drm_atomic_helper_check_modeset(state-dev, state);
if (ret)
return ret;
for_each_crtc_in_state(state, crtc, crtc_state, i) {
- if (!crtc_state-enable
- WARN_ON(crtc_state-active))
- crtc_state-active = false;
-
- if (!crtc_state-enable)
+ if (!crtc_state-enable) {
+ if (needs_modeset(crtc_state))
+ any_ms = true;
continue;
+ }
if (!needs_modeset(crtc_state)) {
ret = drm_atomic_add_affected_connectors(state, crtc);
@@ -13181,14 +13181,20 @@ intel_modeset_compute_config(struct drm_atomic_state
*state)
if (ret)
return ret;
+ if (needs_modeset(crtc_state))
+ any_ms = true;
+
intel_dump_pipe_config(to_intel_crtc(crtc),
to_intel_crtc_state(crtc_state),
[modeset]);
}
- ret = intel_modeset_checks(state);
- if (ret)
- return ret;
+ if (any_ms) {
+ ret = intel_modeset_checks(state);
+
+ if (ret)
+ return ret;
+ }
return drm_atomic_helper_check_planes(state-dev, state);
}
@@ -13201,6 +13207,7 @@ static int __intel_set_mode(struct drm_atomic_state
*state)
struct drm_crtc_state *crtc_state;
int ret = 0;
int i;
+ bool any_ms = false;
ret = drm_atomic_helper_prepare_planes(dev, state);
if (ret)
@@ -13209,7 +13216,11 @@ static int __intel_set_mode(struct drm_atomic_state
*state)
drm_atomic_helper_swap_state(dev, state);
for_each_crtc_in_state(state, crtc, crtc_state, i) {
- if (!needs_modeset(crtc-state) || !crtc_state-active)
+ if (!needs_modeset(crtc-state))
+ continue;
+
+ any_ms = true;
+ if (!crtc_state-active)
continue;
intel_crtc_disable_planes(crtc);
@@ -13222,8 +13233,8 @@ static int __intel_set_mode(struct drm_atomic_state
*state)
/* The state has been swaped above, so state actually contains the
* old state now. */
-
- modeset_update_crtc_power_domains(state);
+ if (any_ms)
+ modeset_update_crtc_power_domains(state);
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
for_each_crtc_in_state(state, crtc, crtc_state, i) {
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 10/19] drm/i915: remove force argument from disable_plane
The idea was good, but planes can have a fb even though
they're disabled. This makes the force argument useless
and always true, because only the commit function updates
state.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 16 +++-
drivers/gpu/drm/i915/intel_drv.h | 2 +-
drivers/gpu/drm/i915/intel_sprite.c | 10 +-
3 files changed, 9 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 61697335bff2..a65c1065decf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4794,7 +4794,7 @@ static void intel_crtc_disable_planes(struct drm_crtc
*crtc)
struct drm_crtc *from = intel_plane-base.crtc;
intel_plane-disable_plane(intel_plane-base,
- from ?: crtc, true);
+ from ?: crtc);
}
}
@@ -13757,8 +13757,7 @@ intel_commit_primary_plane(struct drm_plane *plane,
static void
intel_disable_primary_plane(struct drm_plane *plane,
- struct drm_crtc *crtc,
- bool force)
+ struct drm_crtc *crtc)
{
struct drm_device *dev = plane-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -14006,17 +14005,8 @@ intel_check_cursor_plane(struct drm_plane *plane,
static void
intel_disable_cursor_plane(struct drm_plane *plane,
- struct drm_crtc *crtc,
- bool force)
+ struct drm_crtc *crtc)
{
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
- if (!force) {
- plane-fb = NULL;
- intel_crtc-cursor_bo = NULL;
- intel_crtc-cursor_addr = 0;
- }
-
intel_crtc_update_cursor(crtc, false);
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8c0f17e84eee..baf57b3b136f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -611,7 +611,7 @@ struct intel_plane {
uint32_t x, uint32_t y,
uint32_t src_w, uint32_t src_h);
void (*disable_plane)(struct drm_plane *plane,
- struct drm_crtc *crtc, bool force);
+ struct drm_crtc *crtc);
int (*check_plane)(struct drm_plane *plane,
struct intel_crtc_state *crtc_state,
struct intel_plane_state *state);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 999a5753dde3..f7eb3bd0a7c7 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -272,7 +272,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct
drm_crtc *crtc,
}
static void
-skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc, bool force)
+skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
{
struct drm_device *dev = dplane-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -455,7 +455,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc
*crtc,
}
static void
-vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc, bool force)
+vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
{
struct drm_device *dev = dplane-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -595,7 +595,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
}
static void
-ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc, bool force)
+ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
{
struct drm_device *dev = plane-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -722,7 +722,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
}
static void
-ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc, bool force)
+ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
{
struct drm_device *dev = plane-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -943,7 +943,7 @@ intel_commit_sprite_plane(struct drm_plane *plane,
drm_rect_width(state-src) 16,
drm_rect_height(state-src) 16);
} else {
- intel_plane-disable_plane(plane, crtc, false);
+ intel_plane-disable_plane(plane, crtc);
}
}
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 01/19] drm/i915: Use crtc state in intel_modeset_pipe_config
Grabbing crtc state from atomic state is a lot more involved,
and make sure connectors are added before calling this function.
Move check_digital_port_conflicts to intel_modeset_checks,
it's only useful to check it on a modeset.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 39 ++--
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5974e995676d..d36684080987 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12044,10 +12044,9 @@ clear_intel_crtc_state(struct intel_crtc_state
*crtc_state)
static int
intel_modeset_pipe_config(struct drm_crtc *crtc,
- struct drm_atomic_state *state)
+ struct intel_crtc_state *pipe_config)
{
- struct drm_crtc_state *crtc_state;
- struct intel_crtc_state *pipe_config;
+ struct drm_atomic_state *state = pipe_config-base.state;
struct intel_encoder *encoder;
struct drm_connector *connector;
struct drm_connector_state *connector_state;
@@ -12060,26 +12059,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
return -EINVAL;
}
- if (!check_digital_port_conflicts(state)) {
- DRM_DEBUG_KMS(rejecting conflicting digital port
configuration\n);
- return -EINVAL;
- }
-
- crtc_state = drm_atomic_get_existing_crtc_state(state, crtc);
- if (WARN_ON(!crtc_state))
- return -EINVAL;
-
- pipe_config = to_intel_crtc_state(crtc_state);
-
- /*
-* XXX: Add all connectors to make the crtc state match the encoders.
-*/
- if (!needs_modeset(pipe_config-base)) {
- ret = drm_atomic_add_affected_connectors(state, crtc);
- if (ret)
- return ret;
- }
-
clear_intel_crtc_state(pipe_config);
pipe_config-cpu_transcoder =
@@ -12907,6 +12886,11 @@ static int intel_modeset_checks(struct
drm_atomic_state *state)
struct drm_device *dev = state-dev;
int ret;
+ if (!check_digital_port_conflicts(state)) {
+ DRM_DEBUG_KMS(rejecting conflicting digital port
configuration\n);
+ return -EINVAL;
+ }
+
/*
* See if the config requires any additional preparation, e.g.
* to adjust global state with pipes off. We need to do this
@@ -12953,7 +12937,14 @@ intel_modeset_compute_config(struct drm_atomic_state
*state)
if (!crtc_state-enable)
continue;
- ret = intel_modeset_pipe_config(crtc, state);
+ if (!needs_modeset(crtc_state)) {
+ ret = drm_atomic_add_affected_connectors(state, crtc);
+ if (ret)
+ return ret;
+ }
+
+ ret = intel_modeset_pipe_config(crtc,
+ to_intel_crtc_state(crtc_state));
if (ret)
return ret;
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 16/19] drm/i915: Update less state during modeset.
No need to repeatedly call update_watermarks, or update_fbc.
Down to a single call to update_watermarks in .crtc_enable
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 66 +---
1 file changed, 16 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index beb69281f45c..5facd0501a34 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1945,10 +1945,10 @@ static void intel_disable_shared_dpll(struct intel_crtc
*crtc)
/* PCH only available on ILK+ */
BUG_ON(INTEL_INFO(dev)-gen 5);
- if (WARN_ON(pll == NULL))
- return;
+ if (pll == NULL)
+ return;
- if (WARN_ON(pll-config.crtc_mask == 0))
+ if (WARN_ON(!(pll-config.crtc_mask (1
drm_crtc_index(crtc-base)
return;
DRM_DEBUG_KMS(disable %s (active %d, on? %d) for crtc %d\n,
@@ -4643,10 +4643,6 @@ intel_post_enable_primary(struct drm_crtc *crtc)
*/
hsw_enable_ips(intel_crtc);
- mutex_lock(dev-struct_mutex);
- intel_fbc_update(dev);
- mutex_unlock(dev-struct_mutex);
-
/*
* Gen2 reports pipe underruns whenever all planes are disabled.
* So don't enable underrun reporting before at least some planes
@@ -4701,11 +4697,6 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
if (HAS_GMCH_DISPLAY(dev))
intel_set_memory_cxsr(dev_priv, false);
- mutex_lock(dev-struct_mutex);
- if (dev_priv-fbc.crtc == intel_crtc)
- intel_fbc_disable(dev);
- mutex_unlock(dev-struct_mutex);
-
/*
* FIXME IPS should be fine as long as one plane is
* enabled, but in practice it seems to have problems
@@ -4745,6 +4736,7 @@ static void intel_post_plane_update(struct intel_crtc
*crtc)
static void intel_pre_plane_update(struct intel_crtc *crtc)
{
struct drm_device *dev = crtc-base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc_atomic_commit *atomic = crtc-atomic;
struct drm_plane *p;
@@ -4774,8 +4766,13 @@ static void intel_pre_plane_update(struct intel_crtc
*crtc)
if (atomic-wait_for_flips)
intel_crtc_wait_for_pending_flips(crtc-base);
- if (atomic-disable_fbc)
- intel_fbc_disable(dev);
+ if (atomic-disable_fbc
+ dev_priv-fbc.crtc == crtc) {
+ mutex_lock(dev-struct_mutex);
+ if (dev_priv-fbc.crtc == crtc)
+ intel_fbc_disable(dev);
+ mutex_unlock(dev-struct_mutex);
+ }
if (atomic-pre_disable_primary)
intel_pre_disable_primary(crtc-base);
@@ -4992,9 +4989,6 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
int pipe = intel_crtc-pipe;
u32 reg, temp;
- if (WARN_ON(!intel_crtc-active))
- return;
-
for_each_encoder_on_crtc(dev, crtc, encoder)
encoder-disable(encoder);
@@ -5033,18 +5027,8 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
I915_WRITE(PCH_DPLL_SEL, temp);
}
- /* disable PCH DPLL */
- intel_disable_shared_dpll(intel_crtc);
-
ironlake_fdi_pll_disable(intel_crtc);
}
-
- intel_crtc-active = false;
- intel_update_watermarks(crtc);
-
- mutex_lock(dev-struct_mutex);
- intel_fbc_update(dev);
- mutex_unlock(dev-struct_mutex);
}
static void haswell_crtc_disable(struct drm_crtc *crtc)
@@ -5055,9 +5039,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
struct intel_encoder *encoder;
enum transcoder cpu_transcoder = intel_crtc-config-cpu_transcoder;
- if (WARN_ON(!intel_crtc-active))
- return;
-
for_each_encoder_on_crtc(dev, crtc, encoder) {
intel_opregion_notify_encoder(encoder, false);
encoder-disable(encoder);
@@ -5093,16 +5074,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
for_each_encoder_on_crtc(dev, crtc, encoder)
if (encoder-post_disable)
encoder-post_disable(encoder);
-
- intel_crtc-active = false;
- intel_update_watermarks(crtc);
-
- mutex_lock(dev-struct_mutex);
- intel_fbc_update(dev);
- mutex_unlock(dev-struct_mutex);
-
- if (intel_crtc_to_shared_dpll(intel_crtc))
- intel_disable_shared_dpll(intel_crtc);
}
static void i9xx_pfit_enable(struct intel_crtc *crtc)
@@ -6154,9 +6125,6 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
struct intel_encoder *encoder;
int pipe = intel_crtc-pipe;
- if (WARN_ON(!intel_crtc-active))
- return;
-
/*
* On gen2 planes
Re: [Intel-gfx] Warning: Broadwell/i915: Unclaimed register detected before reading register 0x130040
On Mon, Jun 15, 2015 at 11:47:46AM +0100, Chris Wilson wrote: On Mon, Jun 15, 2015 at 12:25:10PM +0200, Daniel Vetter wrote: On Mon, May 25, 2015 at 08:04:45PM +0200, Dominik Brodowski wrote: Hey, this just popped up in the dmesg of my Dell XPS 13 -- the system seems to run well, but still, it asks about being cut and sent, so here it is. It is on 4.1.0-rc4+ (Linus' tree as of May 24th, around 3pm UTC -- don't have the git commit ID anymore). Can you please boot with i915.mmio_debug=10 and try to reproduce the issue? It won't help. To see this WARN implies that hsw_unclaimed_reg_detect() fired and mmio debugging is enabled, and the WARN then fires before a subsequent register access. So it implies the unclaimed register is not being accessed by the display driver -- presuming that we have wrapped all register access appropriately. invalid register access hsw_unclaimed_reg_detect - fires, clears the invalid flag ... ( hsw_unclaimed_reg_debug before mmio hsw_unclaimed_reg_debug after ) - nothing x N ... invalid register access hsw_unclaimed_reg_debug - fires before, ergo no more useful information Or else I nerfed the automatic debugging too much. Hm I thought it's also gating some of the interrupt frobbery, but that doesn't exist on bdw anyway. It would be still good to figure out who's stomping all over the gpu though. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] drm/i915: prevent out of range pt in the PDE macros (take 2)
On Mon, Jun 15, 2015 at 11:33:37AM +0100, Dave Gordon wrote:
On 13/06/15 09:28, Chris Wilson wrote:
On Fri, Jun 12, 2015 at 06:30:56PM -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
We tried to fix this in the following commit:
commit fdc454c1484a20e1345cf4e4d7a9feaee814147f
Author: Michel Thierry michel.thie...@intel.com
Date: Tue Mar 24 15:46:19 2015 +
drm/i915: Prevent out of range pt in gen6_for_each_pde
but the static analyzer still complains that, just before we break due
to iter I915_PDES, we do pt = (pd)-page_table[iter] with an
iter value that is bigger than I915_PDES. Of course, this isn't really
a problem since no one uses pt outside the macro. Still, every single
new usage of the macro will create a new issue for us to mark as a
false possitive.
After the commit mentioned above we also created some new versions of
the macros, so they carry the same problem.
In order to solve this problem, let's leave the macro with a NULL
value for pt. So if somebody uses it, we're more likely to get a big
error message instead of some silent failure. I hope the static
analyzer won't complain about the new solution (I don't have a way to
check this!).
I know, the solution looks really ugly. I am hoping the reviewers will
help us decide if we prefer this patch or if we prefer to keep marking
things as false positives.
Cc: Michel Thierry michel.thie...@intel.com
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_gem_gtt.h | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
I sent this as an RFC because I really don't know if complicating the
macro even more will help us in any way. I won't really be surprised
if I see NACKs on this patch, so don't hesitate if you want to.
Also, all I did was boot a Kernel with this patch and make sure it
shows the desktop. So consider this as untested, possibly broken.
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 0d46dd2..b202ca0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -352,7 +352,8 @@ struct i915_hw_ppgtt {
*/
Overallocate page_table etc by one and put a NULL sentinel in it.
for ((iter) = gen6_pde_index(start); \
(length) 0 (pt = (pd)-page_table[iter]); \
(iter)++, \
temp = ALIGN(start+1, 1 GEN6_PDE_SHIFT) - start, \
temp = min_t(unsigned, temp, length), \
-Chris
This might trigger different warnings from some static analysers, as
'pt' doesn't get assigned at all if length == 0.
And? If pt is used when length==0 then I would agree with the analyzer
that pt should be invalid. If the analyzer can't tell that length is
non-zero in the use case and gives false positives, then the analyzer is
likely missing genuinine bugs in other cases.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add the ddi get cdclk code for BXT.
On Wed, Jun 10, 2015 at 01:18:28PM -0700, Bob Paauwe wrote:
The registers and process differ from other platforms.
Signed-off-by: Bob Paauwe bob.j.paa...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index c38c297..41464a5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6687,6 +6687,30 @@ static int skylake_get_display_clock_speed(struct
drm_device *dev)
return 24000;
}
+static int broxton_get_display_clock_speed(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ uint32_t cdctl = I915_READ(CDCLK_CTL);
+ uint32_t pll_freq = I915_READ(BXT_DE_PLL_CTL) BXT_DE_PLL_RATIO_MASK;
+
You should return 19.2MHz if the DE PLL isn't enabled.
+ switch (cdctl BXT_CDCLK_CD2X_DIV_SEL_MASK) {
+ case BXT_CDCLK_CD2X_DIV_SEL_1:
+ if (pll_freq == BXT_DE_PLL_RATIO(60)) /* PLL freq = 1152MHz */
+ return 576000;
+ else /* PLL freq = 1248MHz */
+ return 624000;
+ case BXT_CDCLK_CD2X_DIV_SEL_1_5:
+ return 384000;
+ case BXT_CDCLK_CD2X_DIV_SEL_2:
+ return 288000;
+ case BXT_CDCLK_CD2X_DIV_SEL_4:
+ return 144000;
+ }
+
+ /* error case, assume higer PLL freq. */
+ return 624000;
+}
+
static int broadwell_get_display_clock_speed(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -14649,6 +14673,9 @@ static void intel_init_display(struct drm_device *dev)
if (IS_SKYLAKE(dev))
dev_priv-display.get_display_clock_speed =
skylake_get_display_clock_speed;
+ else if (IS_BROXTON(dev))
+ dev_priv-display.get_display_clock_speed =
+ broxton_get_display_clock_speed;
else if (IS_BROADWELL(dev))
dev_priv-display.get_display_clock_speed =
broadwell_get_display_clock_speed;
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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] drm/atomic: pass old crtc state to atomic_begin/flush.
On Mon, Jun 15, 2015 at 09:30:19AM +0200, Maarten Lankhorst wrote: Op 15-06-15 om 09:10 schreef Daniel Vetter: On Fri, Jun 12, 2015 at 11:18:22AM +0200, Maarten Lankhorst wrote: In intel it's useful to keep track of some state changes with old crtc state vs new state, for example to disable initial planes or when a modeset's prevented during fastboot. Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com Hm, thus far the approach has been that the various -check callbacks diff the state and set appropriate stuff like needs_modeset or planes_changed. And with intel_crtc-atomic we've kinda started to build up similar things for i915. What do you plan to use this for? -Daniel On a modeset I want to disable all old planes by calling plane-disable_plane, which is old_crtc_state-plane_mask. This is for initial hw readout, where a plane might be active without a fb set. I want to run it during vblank evasion if possible, which means in atomic_begin or flush. commit_plane is not called if the old and new state both have a NULL fb, so the initial plane would stay active in this case. Hm, so this is for the i915 state readout code. Imo we shouldn't ever leak this out of the state readout code but instead sanitize the plane state to make sense. Roughly this would be: - read out crtc state - try to reconstruct initial fb for primary plane, if this succeeds then fully link up the plane with the crtc in the plane_state. - then walk all planes for the crtc, and if any plane is enabled in the hw state but doesn't have fb/crtc set in the plane_state force-disable it. We already do something similar with the vga plane, which we don't represent at all in kms. Then none of that initial modeset stuff would ever leak into an atomic modeset since it would be all contained in sanitize_crtc. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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: Reinstate order of operations in {intel, logical}_ring_begin()
On Mon, Jun 08, 2015 at 07:51:36PM +0100, Dave Gordon wrote: The original idea of preallocating the OLR was implemented in 9d773091 drm/i915: Preallocate next seqno before touching the ring and the sequence of operations was to allocate the OLR, then wrap past the end of the ring if necessary, then wait for space if necessary. But subsequently intel_ring_begin() was refactored, in 304d695 drm/i915: Flush outstanding requests before allocating new seqno to ensure that pending work that might need to be flushed used the old and not the newly-allocated request. This changed the sequence to wrap and/or wait, then allocate, although the comment still said /* Preallocate the olr before touching the ring */ which was no longer true as intel_wrap_ring_buffer() touches the ring. The reversal didn't introduce any problems until the introduction of dynamic pinning, in 7ba717c drm/i915/bdw: Pin the ringbuffer backing object to GGTT on-demand With that came the possibility that the ringbuffer might not be pinned to the GTT or mapped into CPU address space when intel_ring_begin() is called. It gets pinned when the request is allocated, so it's now important that this comes before *anything* that can write into the ringbuffer, specifically intel_wrap_ring_buffer(), as this will fault if (a) the ringbuffer happens not to be mapped, and (b) tail happens to be sufficiently close to the end of the ring to trigger wrapping. The original rationale for this reversal seems to no longer apply, as we shouldn't ever have anything in the ringbuffer which is not associated with a specific request, and therefore shouldn't have anything to flush. So it should now be safe to reinstate the original sequence of allocate-wrap-wait :) It still applies. If you submit say 1024 interrupted execbuffers they all share the same request. Then so does the 1025. Except the 1025th (for the sake of argument) requires extra space on the ring. To make that space it finishes the only request (since all 1024 are one and the same) the continues onwardsly blithely unaware it just lost the olr/seqno. To fix this requires request create/commit semantics, where the request create manages the pinning of the context for itself, and also imposes the limitation that a single request cannot occupy the full ringbuffer. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't skip request retirement if the active list is empty
On Fri, 29 May 2015, Jani Nikula jani.nik...@linux.intel.com wrote: On Thu, 28 May 2015, Chris Wilson ch...@chris-wilson.co.uk wrote: On Thu, May 28, 2015 at 06:32:36PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Apparently we can have requests even if though the active list is empty, so do the request retirement regardless of whether there's anything on the active list. The way it happened here is that during suspend intel_ring_idle() notices the olr hanging around and then proceeds to get rid of it by adding a request. However since there was nothing on the active lists i915_gem_retire_requests() didn't clean those up, and so the idle work never runs, and we leave the GPU busy during suspend resulting in a WARN later. Whlist I agree (I use list_empty(ring-request_list);) I strongly suspect something (i.e. execlists) isn't managing the active_list correctly. Pretty much the only thing that can generate a request without an object (and so avoid touching the active_list) is a CS flip, and I doubt you are using those... Anyway, Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk Cc: sta...@vger.linux.org Pushed to drm-intel-fixes, thanks for the patch and review. Reverted from drm-intel-fixes because I misapplied it, and applied to drm-intel-next-fixes instead. Sorry for the noise. BR, Jani. 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 -- 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] [PATCH] drm/i915: Fix build without CONFIG_PM
drivers/gpu/drm/i915/i915_debugfs.c: In function ‘i915_runtime_pm_status’: drivers/gpu/drm/i915/i915_debugfs.c:2528:34: error: ‘struct dev_pm_info’ has no member named ‘usage_count’ atomic_read(dev-dev-power.usage_count)); Regression from commit a6aaec8be22652a808d6e316d4a92e58cb75e986 Author: Damien Lespiau damien.lesp...@intel.com Date: Thu Jun 4 18:23:58 2015 +0100 drm/i915: Add runtime PM's usage_count in i915_runtime_pm_status Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Damien Lespiau damien.lesp...@intel.com Cc: Paulo Zanoni paulo.r.zan...@intel.com Cc: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 8f9554a61019..bd7a5fbf3959 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2524,8 +2524,12 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused) seq_printf(m, GPU idle: %s\n, yesno(!dev_priv-mm.busy)); seq_printf(m, IRQs disabled: %s\n, yesno(!intel_irqs_enabled(dev_priv))); +#ifdef CONFIG_PM seq_printf(m, Usage count: %d\n, atomic_read(dev-dev-power.usage_count)); +#else + seq_printf(m, Device Power Management (CONFIG_PM) disabled\n); +#endif return 0; } -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 14/27] drm/i915: clean up atomic plane check functions
Op 15-06-15 om 14:05 schreef Daniel Vetter: On Thu, Jun 11, 2015 at 06:23:09AM +0200, Maarten Lankhorst wrote: Op 11-06-15 om 03:35 schreef Matt Roper: On Thu, Jun 04, 2015 at 02:47:44PM +0200, Maarten Lankhorst wrote: By passing crtc_state to the check_plane functions a lot of duplicated code can be removed. And now that the transitional helpers are gone the crtc_state can be reliably obtained. Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com --- drivers/gpu/drm/i915/intel_atomic_plane.c | 4 ++- drivers/gpu/drm/i915/intel_display.c | 48 ++- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_sprite.c | 13 +++-- 4 files changed, 23 insertions(+), 43 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index aa2128369a0a..4d8cacbca777 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -119,6 +119,8 @@ static int intel_plane_atomic_check(struct drm_plane *plane, crtc = crtc ? crtc : plane-crtc; intel_crtc = to_intel_crtc(crtc); + intel_state-visible = false; + What do we need this change for? Primary and cursor check functions immediately overwrite state-visible, so setting this here has no effect. The sprite case where fb==NULL is the only case where this would matter, but moving the assignment from the sprite check function to here doesn't seem like it gains us anything. I was using it to clear intel_state-visible even if no crtc is set. In that case state-visible should already be false, but a bit of paranoia never hunts. :-) Resetting of state should be done in the duplicate functions. This makes sure we never ever forget to compute it correctly ;-) Sprinkling clearing code all over (and especially over the compute code) is imo fragile and should be avoided if possible. -Daniel Good point! ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [BUG, bisect] Re: drm/i915: WARN_ON(dev_priv-mm.busy)
On Mon, Jun 15, 2015 at 01:25:38AM -0700, Jeremiah Mahler wrote: Daniel, On Mon, Jun 15, 2015 at 08:57:47AM +0200, Daniel Vetter wrote: Can you please retest with commit 0aedb1626566efd72b369c01992ee7413c82a0c5 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Thu May 28 18:32:36 2015 +0300 drm/i915: Don't skip request retirement if the active list is empty Thanks, Daniel The bug is still present with that patch applied. And it is still present up to linux-next 20150611. The patch was misapplied, so what's in the tree at the moment isn't what I sent to the list. -- 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] Deadlock in intel_user_framebuffer_destroy()
Hi,
On 06/15/2015 08:53 AM, Chris Wilson wrote:
On Mon, Jun 15, 2015 at 09:44:15AM +0300, Jani Nikula wrote:
On Wed, 03 Jun 2015, Chris Wilson ch...@chris-wilson.co.uk wrote:
On Wed, Jun 03, 2015 at 03:43:32PM +0200, Lukas Wunner wrote:
Hi,
a deadlock was introduced by commit 60a5ca015ffd2aacfe5674b5a401cd2a37159e07
Author: Ville Syrjälä ville.syrj...@linux.intel.com
Date: Fri Jun 13 11:10:53 2014 +0300
drm/i915: Add locking around framebuffer_references--
The commit amended intel_display.c:intel_user_framebuffer_destroy() with
mutex_lock(dev-struct_mutex).
A few weeks prior Chris Wilson had amended intel_fbdev.c:intelfb_create()
with a call to drm_framebuffer_unreference() while dev-struct_mutex is
locked (commit edd586fe705e819bc711b5ed7194a0b6f9f1a7e1, drm/i915: Discard
BIOS framebuffers too small to accommodate chosen mode).
Just move the mutex_lock down a step.
Lucas, did you try this?
There's a goto unlock that also needed to be disabled, such as
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index dda99c0d6be1..fc7ec5138fb7 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -213,8 +213,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
bool prealloc = false;
int ret;
- mutex_lock(dev-struct_mutex);
-
if (intel_fb
(sizes-fb_width intel_fb-base.width ||
sizes-fb_height intel_fb-base.height)) {
@@ -229,7 +227,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
DRM_DEBUG_KMS(no BIOS fb, allocating a new one\n);
ret = intelfb_alloc(helper, sizes);
if (ret)
- goto out_unlock;
+ return ret;
intel_fb = ifbdev-fb;
} else {
DRM_DEBUG_KMS(re-using BIOS fb\n);
@@ -241,6 +239,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
obj = intel_fb-obj;
vma = i915_gem_obj_to_ggtt(obj, NULL);
+ mutex_lock(dev-struct_mutex);
info = framebuffer_alloc(0, dev-pdev-dev);
if (!info) {
ret = -ENOMEM;
@@ -311,7 +310,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
out_unpin:
drm_gem_object_unreference(obj-base);
-out_unlock:
mutex_unlock(dev-struct_mutex);
return ret;
}
intelfb_alloc wants struct_mutex, both for __intel_framebuffer_create
and pin_and_fence. And also there is that double obj unreference in the
failure path from the former Jani spotted.
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/9] drm/i915: HDMI 12bpc fixes
On Mon, Jun 01, 2015 at 07:04:07PM +, Konduru, Chandra wrote: -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of ville.syrj...@linux.intel.com Sent: Tuesday, May 05, 2015 7:06 AM To: intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [PATCH 0/9] drm/i915: HDMI 12bpc fixes From: Ville Syrjälä ville.syrj...@linux.intel.com Our HDMI 12bpc support has always been broken. This series aims to fix that. The problems addressed include: - missing GCP infoframes entirely - IBX w/a code was mostly nonsense - missing w/a for CPT/PPT - 12bpc vs. DBLCLK was busted Part of this was already posted [1] quite a while ago, but it's grown some new stuff since. The entire series is available in git [2]. I have additional stuff to fix the IBX transcoder B workarounds and some other random thigns on PCH platforms. I'll post that as a followup. [1] http://lists.freedesktop.org/archives/intel-gfx/2014-October/053340.html [2] hit://github.com/vsyrjala/linux.git hdmi_12bpc_fixes_9 Broken link [2]. Some typo? s/hit/git and then you have a git url ;-) Anyway thanks for patchesreview, all merged to dinq. -Daniel Ville Syrjälä (9): drm/i915: Implement WaEnableHDMI8bpcBefore12bpc:snb,ivb drm/i915: Send GCP infoframes for deep color HDMI sinks drm/i915: Enable default_phase in GCP when possible drm/i915: Fix HDMI 12bpc TRANSCONF bpc value drm/i915: Fix 12bpc HDMI enable for IBX drm/i915: Disable all infoframes when turning off the HDMI port drm/i915: Check infoframe state more diligently. drm/i915: Fix hdmi clock readout with pixel repeat drm/i915: Double the port clock when using double clocked modes with 12bpc drivers/gpu/drm/i915/i915_reg.h | 4 + drivers/gpu/drm/i915/intel_display.c | 10 +- drivers/gpu/drm/i915/intel_hdmi.c| 359 --- 3 files changed, 306 insertions(+), 67 deletions(-) -- 2.0.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] Revert drm/i915: Don't skip request retirement if the active list is empty
This reverts commit 0aedb1626566efd72b369c01992ee7413c82a0c5.
I messed things up while applying [1] to drm-intel-fixes. Rectify.
[1]
http://mid.gmane.org/1432827156-9605-1-git-send-email-ville.syrj...@linux.intel.com
Fixes: 0aedb1626566 (drm/i915: Don't skip request retirement if the active
list is empty)
Cc: sta...@vger.kernel.org
Signed-off-by: Jani Nikula jani.nik...@intel.com
---
Note to stable team: please do *not* backport
commit 0aedb1626566efd72b369c01992ee7413c82a0c5
Author: Ville Syrjälä ville.syrj...@linux.intel.com
Date: Thu May 28 18:32:36 2015 +0300
drm/i915: Don't skip request retirement if the active list is empty
but if you did, please backport this revert as well.
Thanks, and sorry.
BR,
Jani.
---
drivers/gpu/drm/i915/i915_gem.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c3806c66650a..2d0995e7afc3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2656,6 +2656,9 @@ void i915_gem_reset(struct drm_device *dev)
void
i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
{
+ if (list_empty(ring-request_list))
+ return;
+
WARN_ON(i915_verify_lists(ring-dev));
/* Retire requests first as we use it above for the early return.
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/21] drm/i915/gtt: Don't leak scratch page on mapping error
On Mon, Jun 01, 2015 at 06:02:52PM +0300, Joonas Lahtinen wrote:
On pe, 2015-05-22 at 20:04 +0300, Mika Kuoppala wrote:
Free the scratch page if dma mapping fails.
Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
Reviewed-by: Joonas Lahtinen joonas.lahti...@linux.intel.com
Queued for -next, thanks for the patch.
-Daniel
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index c61de4a..a608b1b 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2191,8 +2191,10 @@ static int setup_scratch_page(struct drm_device *dev)
#ifdef CONFIG_INTEL_IOMMU
dma_addr = pci_map_page(dev-pdev, page, 0, PAGE_SIZE,
PCI_DMA_BIDIRECTIONAL);
- if (pci_dma_mapping_error(dev-pdev, dma_addr))
+ if (pci_dma_mapping_error(dev-pdev, dma_addr)) {
+ __free_page(page);
return -EINVAL;
+ }
#else
dma_addr = page_to_phys(page);
#endif
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
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: Fix command parser to validate multiple register access with the same command.
On Tue, Jun 02, 2015 at 05:36:26PM +0800, Zhigang Gong wrote:
The patchset LGTM and works well with beignet. The 80%+ performance
regression issue in darktable also has been fixed
after this patchset applied and enable the atomic in L3 at beignet side. So,
Reviewed-by: Zhigang Gong zhigang.g...@linux.intel.com
All three merged. Aside: Dont we need an increment for the cmd parser
version for userspace to be able to detect this?
And please follow up with a link to the beignet patches used to validate
these kernel patches for references.
Thanks, Daniel
Thanks,
Zhigang Gong.
-Original Message-
From: Francisco Jerez [mailto:curroje...@riseup.net]
Sent: Friday, May 29, 2015 9:44 PM
To: intel-gfx@lists.freedesktop.org
Cc: Ville Syrjälä; Zhigang Gong; Brad Volkin
Subject: [PATCH 1/3] drm/i915: Fix command parser to validate multiple
register access with the same command.
Until now the software command checker assumed that commands could read
or write at most a single register per packet. This is not necessarily the
case,
MI_LOAD_REGISTER_IMM expects a variable-length list of offset/value pairs
and writes them in sequence. The previous code would only check whether
the first entry was valid, effectively allowing userspace to write
unrestricted
registers of the MMIO space by sending a multi-register write with a legal
first
register, with potential security implications on Gen6 and 7 hardware.
Fix it by extending the drm_i915_cmd_descriptor table to represent
multi-register access and making validate_cmd() iterate for all register
offsets
present in the command packet.
Signed-off-by: Francisco Jerez curroje...@riseup.net
---
drivers/gpu/drm/i915/i915_cmd_parser.c | 74
--
drivers/gpu/drm/i915/i915_drv.h| 5 +++
2 files changed, 48 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 61ae8ff..c4a5f73 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -123,7 +123,7 @@ static const struct drm_i915_cmd_descriptor
common_cmds[] = {
CMD( MI_SEMAPHORE_MBOX,SMI, !F, 0xFF,
R ),
CMD( MI_STORE_DWORD_INDEX, SMI, !F, 0xFF,
R ),
CMD( MI_LOAD_REGISTER_IMM(1), SMI, !F, 0xFF,
W,
- .reg = { .offset = 1, .mask = 0x007C } ),
+ .reg = { .offset = 1, .mask = 0x007C, .step = 2 }),
CMD( MI_STORE_REGISTER_MEM(1), SMI, !F, 0xFF,
W | B,
.reg = { .offset = 1, .mask = 0x007C },
.bits = {{
@@ -939,7 +939,7 @@ bool i915_needs_cmd_parser(struct intel_engine_cs
*ring)
static bool check_cmd(const struct intel_engine_cs *ring,
const struct drm_i915_cmd_descriptor *desc,
- const u32 *cmd,
+ const u32 *cmd, u32 length,
const bool is_master,
bool *oacontrol_set)
{
@@ -955,38 +955,49 @@ static bool check_cmd(const struct intel_engine_cs
*ring,
}
if (desc-flags CMD_DESC_REGISTER) {
- u32 reg_addr = cmd[desc-reg.offset] desc-reg.mask;
-
/*
-* OACONTROL requires some special handling for writes. We
-* want to make sure that any batch which enables OA also
-* disables it before the end of the batch. The goal is to
-* prevent one process from snooping on the perf data from
-* another process. To do that, we need to check the value
-* that will be written to the register. Hence, limit
-* OACONTROL writes to only MI_LOAD_REGISTER_IMM
commands.
+* Get the distance between individual register offset
+* fields if the command can perform more than one
+* access at a time.
*/
- if (reg_addr == OACONTROL) {
- if (desc-cmd.value == MI_LOAD_REGISTER_MEM) {
- DRM_DEBUG_DRIVER(CMD: Rejected LRM to
OACONTROL\n);
- return false;
+ const u32 step = desc-reg.step ? desc-reg.step : length;
+ u32 offset;
+
+ for (offset = desc-reg.offset; offset length;
+offset += step) {
+ const u32 reg_addr = cmd[offset] desc-reg.mask;
+
+ /*
+* OACONTROL requires some special handling for
+* writes. We want to make sure that any batch which
+* enables OA also disables it before the end of the
+* batch. The goal is to prevent one process from
+* snooping on the perf data from another process. To do
+* that, we need to check
Re: [Intel-gfx] [PATCH] Revert drm/i915: Don't skip request retirement if the active list is empty
On Mon, Jun 15, 2015 at 12:59:37PM +0300, Jani Nikula wrote:
This reverts commit 0aedb1626566efd72b369c01992ee7413c82a0c5.
I messed things up while applying [1] to drm-intel-fixes. Rectify.
[1]
http://mid.gmane.org/1432827156-9605-1-git-send-email-ville.syrj...@linux.intel.com
Fixes: 0aedb1626566 (drm/i915: Don't skip request retirement if the active
list is empty)
Cc: sta...@vger.kernel.org
Signed-off-by: Jani Nikula jani.nik...@intel.com
Acked-by: Ville Syrjälä ville.syrj...@linux.intel.com
However this leaves us with an early bailout if request_list is empty.
Hopefully that won't cause other issues. I suppose we shouldn't have
stuff on the active_list w/o any pending requests.
---
Note to stable team: please do *not* backport
commit 0aedb1626566efd72b369c01992ee7413c82a0c5
Author: Ville Syrjälä ville.syrj...@linux.intel.com
Date: Thu May 28 18:32:36 2015 +0300
drm/i915: Don't skip request retirement if the active list is empty
but if you did, please backport this revert as well.
Thanks, and sorry.
BR,
Jani.
---
drivers/gpu/drm/i915/i915_gem.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c3806c66650a..2d0995e7afc3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2656,6 +2656,9 @@ void i915_gem_reset(struct drm_device *dev)
void
i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
{
+ if (list_empty(ring-request_list))
+ return;
+
WARN_ON(i915_verify_lists(ring-dev));
/* Retire requests first as we use it above for the early return.
--
2.1.4
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Report to userspace if we have a (presumed) working GPU reset
In igt, we want to test handling of GPU hangs, both for recovery
purposes and for reporting. However, we don't want to inject a genuine
GPU hang onto a machine that cannot recover and so be permenantly
wedged. Rather than embed heuristics into igt, have the kernel report
exactly when it expects the GPU reset to work.
This can also be usefully extended in future to indicate different
levels of fine-grained resets.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Tim Gore tim.g...@intel.com
Cc: Tomas Elf tomas@intel.com
---
drivers/gpu/drm/i915/i915_dma.c | 5 +
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_uncore.c | 28 ++--
include/uapi/drm/i915_drm.h | 1 +
4 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 34248635c36c..88795d2f1819 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -163,6 +163,11 @@ static int i915_getparam(struct drm_device *dev, void
*data,
if (!value)
return -ENODEV;
break;
+ case I915_PARAM_HAS_GPU_RESET:
+ value = i915.enable_hangcheck
+ i915.reset
+ intel_has_gpu_reset(dev);
+ break;
default:
DRM_DEBUG(Unknown parameter %d\n, param-param);
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1110d492ec01..85da0dc3c0e6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2599,6 +2599,7 @@ extern long i915_compat_ioctl(struct file *filp, unsigned
int cmd,
unsigned long arg);
#endif
extern int intel_gpu_reset(struct drm_device *dev);
+extern bool intel_has_gpu_reset(struct drm_device *dev);
extern int i915_reset(struct drm_device *dev);
extern unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
extern unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 40382bff5ca0..a61de6e944d2 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1489,20 +1489,36 @@ static int gen6_do_reset(struct drm_device *dev)
return ret;
}
-int intel_gpu_reset(struct drm_device *dev)
+static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *)
{
if (INTEL_INFO(dev)-gen = 6)
- return gen6_do_reset(dev);
+ return gen6_do_reset;
else if (IS_GEN5(dev))
- return ironlake_do_reset(dev);
+ return ironlake_do_reset;
else if (IS_G4X(dev))
- return g4x_do_reset(dev);
+ return g4x_do_reset;
else if (IS_G33(dev))
- return g33_do_reset(dev);
+ return g33_do_reset;
else if (INTEL_INFO(dev)-gen = 3)
- return i915_do_reset(dev);
+ return i915_do_reset;
else
+ return NULL;
+}
+
+int intel_gpu_reset(struct drm_device *dev)
+{
+ int (*reset)(struct drm_device *);
+
+ reset = intel_get_gpu_reset(dev);
+ if (reset == NULL)
return -ENODEV;
+
+ return reset(dev);
+}
+
+bool intel_has_gpu_reset(struct drm_device *dev)
+{
+ return intel_get_gpu_reset(dev) != NULL;
}
void intel_uncore_check_errors(struct drm_device *dev)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 4c3420f932a5..312adbeb4eec 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -354,6 +354,7 @@ typedef struct drm_i915_irq_wait {
#define I915_PARAM_REVISION 32
#define I915_PARAM_SUBSLICE_TOTAL 33
#define I915_PARAM_EU_TOTAL 34
+#define I915_PARAM_HAS_GPU_RESET35
typedef struct drm_i915_getparam {
int param;
--
2.1.4
___
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: Fix command parser to validate multiple register access with the same command.
Daniel Vetter dan...@ffwll.ch writes: On Mon, Jun 15, 2015 at 02:26:22PM +0300, Ville Syrjälä wrote: On Mon, Jun 15, 2015 at 02:18:01PM +0300, Francisco Jerez wrote: Daniel Vetter dan...@ffwll.ch writes: On Tue, Jun 02, 2015 at 05:36:26PM +0800, Zhigang Gong wrote: The patchset LGTM and works well with beignet. The 80%+ performance regression issue in darktable also has been fixed after this patchset applied and enable the atomic in L3 at beignet side. So, Reviewed-by: Zhigang Gong zhigang.g...@linux.intel.com All three merged. Thanks Daniel. Aside: Dont we need an increment for the cmd parser version for userspace to be able to detect this? Yeah, that would be a good idea, patch attached. The old version alloweed userspace to write basically any register, the new version allows only the whitelisted registers. I don't see how a version number bump would help anyone. Oops, totally missed the context of patch 1. Jani I think that one's for you too ... IMHO the version bump is still useful for userspace to find out whether it can use plain LRIs to write the L3 atomic chicken bits. It's true that as Ville said it would have been possible for userspace to write the same bits before this series by building a batch specifically crafted to cheat the command parser, but I don't think we want userspace to rely on a command parser bug (e.g. because we may want to back-port the fix to earlier kernel versions). Thanks for pointing this out. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 18/27] drm/i915: Handle disabling planes better.
On Thu, Jun 11, 2015 at 05:51:27AM +0200, Maarten Lankhorst wrote: Op 11-06-15 om 03:37 schreef Matt Roper: If we're not trying to seamlessly inherit the cursor image, I'm not sure I understand what the benefit of figuring out whether the cursor is actually on or not is. Just assuming true, as we do for sprites, would be enough to make the cursor always turn off, right? Yeah, when reworking this patch to apply after modeset revert I fixed that, but kept cursor readout since I wrote it anyway. Imo it's better to ditch anything but primary plane reconstruction to avoid complications. We don't need anything else really. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [BUG, bisect] Re: drm/i915: WARN_ON(dev_priv-mm.busy)
On Mon, 15 Jun 2015, Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Mon, Jun 15, 2015 at 01:25:38AM -0700, Jeremiah Mahler wrote: Daniel, On Mon, Jun 15, 2015 at 08:57:47AM +0200, Daniel Vetter wrote: Can you please retest with commit 0aedb1626566efd72b369c01992ee7413c82a0c5 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Thu May 28 18:32:36 2015 +0300 drm/i915: Don't skip request retirement if the active list is empty Thanks, Daniel The bug is still present with that patch applied. And it is still present up to linux-next 20150611. The patch was misapplied, so what's in the tree at the moment isn't what I sent to the list. Auch, my bad. So we should 1) revert commit 0aedb1626566efd72b369c01992ee7413c82a0c5 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Thu May 28 18:32:36 2015 +0300 drm/i915: Don't skip request retirement if the active list is empty in v4.1 and 2) reapply http://patchwork.freedesktop.org/patch/50659 to drm-intel-next-fixes for v4.2. Is that right? BR, Jani. -- Ville Syrjälä Intel OTC -- 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/atomic: pass old crtc state to atomic_begin/flush.
Op 15-06-15 om 11:13 schreef Daniel Vetter: On Mon, Jun 15, 2015 at 09:30:19AM +0200, Maarten Lankhorst wrote: Op 15-06-15 om 09:10 schreef Daniel Vetter: On Fri, Jun 12, 2015 at 11:18:22AM +0200, Maarten Lankhorst wrote: In intel it's useful to keep track of some state changes with old crtc state vs new state, for example to disable initial planes or when a modeset's prevented during fastboot. Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com Hm, thus far the approach has been that the various -check callbacks diff the state and set appropriate stuff like needs_modeset or planes_changed. And with intel_crtc-atomic we've kinda started to build up similar things for i915. What do you plan to use this for? -Daniel On a modeset I want to disable all old planes by calling plane-disable_plane, which is old_crtc_state-plane_mask. This is for initial hw readout, where a plane might be active without a fb set. I want to run it during vblank evasion if possible, which means in atomic_begin or flush. commit_plane is not called if the old and new state both have a NULL fb, so the initial plane would stay active in this case. Hm, so this is for the i915 state readout code. Imo we shouldn't ever leak this out of the state readout code but instead sanitize the plane state to make sense. Roughly this would be: - read out crtc state - try to reconstruct initial fb for primary plane, if this succeeds then fully link up the plane with the crtc in the plane_state. Agreed. Right now get_initial_plane_config takes an initial_plane_state, could we make this atomic too? - then walk all planes for the crtc, and if any plane is enabled in the hw state but doesn't have fb/crtc set in the plane_state force-disable it. Can we disable those planes without penalty? Some of them call watermark update, this is a bug but still.. We already do something similar with the vga plane, which we don't represent at all in kms. Then none of that initial modeset stuff would ever leak into an atomic modeset since it would be all contained in sanitize_crtc. Ok. ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: replace csr_mutex by completion in csr firmware loading
On Thu, Jun 04, 2015 at 11:29:35AM +0530, Sagar Arun Kamble wrote: On Thu, 2015-05-21 at 23:29 +0200, Daniel Vetter wrote: On Thu, May 21, 2015 at 10:35:07PM +0530, Animesh Manna wrote: On 5/21/2015 5:41 PM, Daniel Vetter wrote: On Thu, May 21, 2015 at 03:49:52PM +0530, Animesh Manna wrote: Before enabling dc5/dc6, used wait for completion instead of busy waiting. v1: - Based on review comment from Daniel replaced mutex and related implementation with completion. In current patch not used power domain lock, don't want to block runtime power management if dmc firmware failed to load. Will analyzing further and possibly send as a incremental patch. - Based on review comment from Damien, warning for firmware loading failure is removed. Signed-off-by: Animesh Manna animesh.ma...@intel.com Sorry if this cross with my comments, but upon further digging into this code we don't even need a completion at all. As long as we prevent the power well from getting disabled by holding a reference for it before we launch the firmware loader it'll all be fine. And the async work can then just drop that reference when everything is set up. Yes this means runtime D3 requires the firmware to be loaded, but that's imo not a problem. Also doing it this way is more in-line with how we do all the other async setup. But there's another serious issue with this design here, see below. Ok, previously I was thinking more on how to pass the firmware loading status before enabling low power states (dc5/dc6). Now while thinking about power management framework I have a fundamental doubt - if got request to disable a power-well and firmware loading failed for some reason, should we disable the power-well(option1) or keep it enable(option2). Both the cases will not trigger for dc5/dc6. I understood from your comment to follow option2, I will change the design accordingly as the current implementation based on option1. To implement option2, I can see there is one mutex present for a power domain, do not have mutex for each power-well, which need to be added and can be used as you mentioned above. If we want to follow option1, I can think a better way of managing firmware loading. As firmware is required when display engine goes to low power state, so we can only parse the packaged firmware during driver initialization. Firmware loading (writing to registers) can be done in skl_enable_dc6/gen9_enable_dc5 function itself and then trigger for dc5/dc6. Planning to implement option2 and will send for review, please correct me if I understood wrongly. power wells are reference counted, which means the only thing you need to do is grab such a reference. Then the corresponding power well won't ever get disabled until the async firmware loader has done it's job and released the power well reference again. See e.g. how the async rps setup grabs a runtime_pm reference (works the same for power wells, they simply nest within the runtime pm stuff). So for option2 no fiddling with mutexes or power well internals needed at all (well the wait_for can be removed afterwards ofc). And we don't need any other mutex, the csr_mutex and crs.state can be removed afterwards too. -Daniel Hi Daniel, We already are grabbing RPM reference before start of DMC FW load and release post load completion. DC5/6 can happen without Runtime PM as well. So we need to wait for CSR FW load for some time once we disable PW2. If dc5/6 can happen despite you holding a runtime pm reference, then you're holding the wrong runtime pm reference. They nest, hence just walk up the chain of power domains until you have one that does prevent dc5/6. I guess there's a confusion between runtime pm as the overall concept (which I'm talking about here, implemented in intel_rpm.c) and the linux runtime pm api which is only used for the very last level of runtime pm in i915 to control entry/exit to D3. On top of that we stack the power wells i915 specific runtime pm api and the corresponding busyness tracking on the render side. Having completion instead of csr.lock+csr.state is correct thing to do. I hope the above explanation helps. I can follow up with a sketch patch too if you want. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] drm/i915: prevent out of range pt in the PDE macros (take 2)
On 12/06/15 22:30, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
We tried to fix this in the following commit:
commit fdc454c1484a20e1345cf4e4d7a9feaee814147f
Author: Michel Thierry michel.thie...@intel.com
Date: Tue Mar 24 15:46:19 2015 +
drm/i915: Prevent out of range pt in gen6_for_each_pde
but the static analyzer still complains that, just before we break due
to iter I915_PDES, we do pt = (pd)-page_table[iter] with an
iter value that is bigger than I915_PDES. Of course, this isn't really
a problem since no one uses pt outside the macro. Still, every single
new usage of the macro will create a new issue for us to mark as a
false possitive.
After the commit mentioned above we also created some new versions of
the macros, so they carry the same problem.
In order to solve this problem, let's leave the macro with a NULL
value for pt. So if somebody uses it, we're more likely to get a big
error message instead of some silent failure. I hope the static
analyzer won't complain about the new solution (I don't have a way to
check this!).
I know, the solution looks really ugly. I am hoping the reviewers will
help us decide if we prefer this patch or if we prefer to keep marking
things as false positives.
Cc: Michel Thierry michel.thie...@intel.com
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_gem_gtt.h | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
I sent this as an RFC because I really don't know if complicating the
macro even more will help us in any way. I won't really be surprised
if I see NACKs on this patch, so don't hesitate if you want to.
Also, all I did was boot a Kernel with this patch and make sure it
shows the desktop. So consider this as untested, possibly broken.
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 0d46dd2..b202ca0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -352,7 +352,8 @@ struct i915_hw_ppgtt {
*/
#define gen6_for_each_pde(pt, pd, start, length, temp, iter) \
for (iter = gen6_pde_index(start); \
- pt = (pd)-page_table[iter], length 0 iter I915_PDES; \
+ pt = iter I915_PDES ? (pd)-page_table[iter] : NULL, \
+ length 0 iter I915_PDES; \
You don't need the repeated test on 'iter'; you can write the test
clause of the loop as:
(pt = iter I915_PDES ? (pd)-page_table[iter] : NULL)
length 0;
using the fact that pt will be NULL when iter = I915_PDES to break from
the loop :)
This version will leave 'pt' NULL after the loop if the break was due to
the test on 'iter', but non-NULL if the test on 'length' triggered the
break -- is this a useful feature?
.Dave.
temp = ALIGN(start+1, 1 GEN6_PDE_SHIFT) - start, \
temp = min_t(unsigned, temp, length), \
@@ -360,7 +361,8 @@ struct i915_hw_ppgtt {
#define gen6_for_all_pdes(pt, ppgtt, iter) \
for (iter = 0; \
- pt = ppgtt-pd.page_table[iter], iter I915_PDES; \
+ pt = iter I915_PDES ? ppgtt-pd.page_table[iter] : NULL, \
+ iter I915_PDES; \
iter++)
static inline uint32_t i915_pte_index(uint64_t address, uint32_t pde_shift)
@@ -417,7 +419,8 @@ static inline uint32_t gen6_pde_index(uint32_t addr)
*/
#define gen8_for_each_pde(pt, pd, start, length, temp, iter) \
for (iter = gen8_pde_index(start); \
- pt = (pd)-page_table[iter], length 0 iter I915_PDES;
\
+ pt = iter I915_PDES ? (pd)-page_table[iter] : NULL, \
+ length 0 iter I915_PDES;\
iter++,\
temp = ALIGN(start+1, 1 GEN8_PDE_SHIFT) - start,\
temp = min(temp, length), \
@@ -425,7 +428,9 @@ static inline uint32_t gen6_pde_index(uint32_t addr)
#define gen8_for_each_pdpe(pd, pdp, start, length, temp, iter)
\
for (iter = gen8_pdpe_index(start); \
- pd = (pdp)-page_directory[iter], length 0 iter
GEN8_LEGACY_PDPES; \
+ pd = iter GEN8_LEGACY_PDPES ?\
+ (pdp)-page_directory[iter] : NULL, \
+ length 0 iter GEN8_LEGACY_PDPES;\
iter++,\
temp = ALIGN(start+1, 1 GEN8_PDPE_SHIFT) - start, \
temp = min(temp, length), \
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [BUG, bisect] Re: drm/i915: WARN_ON(dev_priv-mm.busy)
On Mon, 15 Jun 2015, Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Mon, Jun 15, 2015 at 01:25:38AM -0700, Jeremiah Mahler wrote: Daniel, On Mon, Jun 15, 2015 at 08:57:47AM +0200, Daniel Vetter wrote: Can you please retest with commit 0aedb1626566efd72b369c01992ee7413c82a0c5 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Thu May 28 18:32:36 2015 +0300 drm/i915: Don't skip request retirement if the active list is empty Thanks, Daniel The bug is still present with that patch applied. And it is still present up to linux-next 20150611. The patch was misapplied, so what's in the tree at the moment isn't what I sent to the list. This should be rectified in current drm-intel-nightly branch of [1]. Jeremiah, please give that a try. BR, Jani. [1] http://cgit.freedesktop.org/drm-intel -- Ville Syrjälä Intel OTC -- 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 04/24] drm/i915: Update power domains only on affected crtc's.
On Wed, Jun 03, 2015 at 08:52:52AM +0200, Maarten Lankhorst wrote: Op 03-06-15 om 03:27 schreef Matt Roper: On Mon, Jun 01, 2015 at 03:27:07PM +0200, Maarten Lankhorst wrote: Use for_each_crtc_state to only touch affected crtc's. In order to make sure that the initial power is still set correctly we make sure modeset_update_crtc_power_domains is called during the initial modeset. Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com --- drivers/gpu/drm/i915/i915_drv.c | 3 --- drivers/gpu/drm/i915/intel_display.c | 41 +++- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index d3632c56fdf7..78ef0bb53c36 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -634,9 +634,6 @@ static int i915_drm_suspend(struct drm_device *dev) intel_display_suspend(dev); drm_modeset_unlock_all(dev); - /* suspending displays will unsets init power */ - intel_display_set_init_power(dev_priv, true); - intel_dp_mst_suspend(dev); intel_runtime_pm_disable_interrupts(dev_priv); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8e9afc55c284..4dc07602248b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5188,42 +5188,49 @@ static unsigned long get_crtc_power_domains(struct drm_crtc *crtc) return mask; } -static void modeset_update_crtc_power_domains(struct drm_atomic_state *state) +static void modeset_update_crtc_power_domains(struct drm_atomic_state *state, bool gr) What does 'gr' stand for and what does the parameter signify? It seems to just gate whether we call display.modeset_global_resources, but it's unclear to me from the commit message above in which situations we would/wouldn't want to do this and why. Well there's no point if no modeset is done to call display.modeset_global_resources. But I guess calling it power_only might be better. I wish I knew why modeset_global_resources was done in the middle, I think there's no point to do so. When doing global changes like updating the cdclk we better do that only when the hardware is guaranteed to be on. Since we could update global things both when enabling and when disabling, the only place where the hw is on in either case is in the middle. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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 08/24] drm/i915: Do not add planes from intel_atomic_setup_scalers.
On Thu, Jun 04, 2015 at 05:39:52AM +0200, Maarten Lankhorst wrote:
Hey,
Op 04-06-15 om 01:33 schreef Matt Roper:
On Wed, Jun 03, 2015 at 12:32:43PM -0700, Konduru, Chandra wrote:
-Original Message-
From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com]
Sent: Wednesday, June 03, 2015 12:02 AM
To: Konduru, Chandra; Roper, Matthew D
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 08/24] drm/i915: Do not add planes from
intel_atomic_setup_scalers.
Op 03-06-15 om 03:52 schreef Konduru, Chandra:
-Original Message-
From: Roper, Matthew D
Sent: Tuesday, June 02, 2015 6:30 PM
To: Maarten Lankhorst
Cc: intel-gfx@lists.freedesktop.org; Konduru, Chandra
Subject: Re: [Intel-gfx] [PATCH 08/24] drm/i915: Do not add planes from
intel_atomic_setup_scalers.
On Mon, Jun 01, 2015 at 03:27:11PM +0200, Maarten Lankhorst wrote:
This may postpone going to HQ mode until the plane is in the
drm_atomic_state if it's not using scaler 0, but it does allow moving
intel_atomic_setup_scalers to the crtc check function.
Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
drivers/gpu/drm/i915/intel_atomic.c | 41
++-
---
--
drivers/gpu/drm/i915/intel_display.c | 27 +++-
drivers/gpu/drm/i915/intel_drv.h | 2 ++
3 files changed, 39 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c
b/drivers/gpu/drm/i915/intel_atomic.c
index 1edd1651c045..a8202fa0daa8 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -100,14 +100,6 @@ int intel_atomic_check(struct drm_device *dev,
if (ret)
return ret;
- /*
- * FIXME: move to crtc atomic check function once this is
- * more atomic friendly.
- */
- ret = intel_atomic_setup_scalers(dev, nuclear_crtc, crtc_state);
- if (ret)
- return ret;
-
return ret;
}
@@ -336,21 +328,10 @@ int intel_atomic_setup_scalers(struct drm_device
*dev,
/* find the plane that set the bit as
scaler_user */
plane = drm_state-planes[i];
- /*
- * to enable/disable hq mode, add planes that
are using
scaler
- * into this transaction
- */
if (!plane) {
- struct drm_plane_state *state;
- plane = drm_plane_from_index(dev, i);
- state =
drm_atomic_get_plane_state(drm_state, plane);
- if (IS_ERR(state)) {
- DRM_DEBUG_KMS(Failed to add
[PLANE:%d] to drm_state\n,
- plane-base.id);
- return PTR_ERR(state);
- }
+ DRM_DEBUG_KMS(Failed to find [PLANE:%d]
in drm_state\n, plane-base.id);
+ continue;
}
-
intel_plane = to_intel_plane(plane);
/* plane on different crtc cannot be a scaler
user of this
crtc */
@@ -396,6 +377,24 @@ int intel_atomic_setup_scalers(struct drm_device
*dev,
}
}
+ /* plane not part of mask must leave hq mode? */
+ if (num_scalers_need 1 scaler_state-scalers[0].in_use
+ scaler_state-scalers[0].mode == PS_SCALER_MODE_HQ) {
+ scaler_state-scalers[0].mode = PS_SCALER_MODE_DYN;
+
+ intel_crtc-atomic.skl_update_scaler0 =
+ PS_SCALER_EN | PS_SCALER_MODE_DYN;
+ }
+
+ /* plane not part of mask can enter hq mode? */
+ if (num_scalers_need == 1 scaler_state-scalers[0].in_use
+ intel_crtc-pipe != PIPE_C scaler_state-scalers[0].mode
!=
PS_SCALER_MODE_HQ) {
+ scaler_state-scalers[0].mode = PS_SCALER_MODE_HQ;
+
+ intel_crtc-atomic.skl_update_scaler0 =
+ PS_SCALER_EN | PS_SCALER_MODE_HQ;
+ }
+
I don't have access to the hw spec at the moment; is scaler #0 the only
one that can ever go into HQ mode?
Yes
If there isn't a hardware
requirement about this, then it seems like we're missing the case where
planes A and B get scalers 0 and 1. Then plane A (and thus scaler 0) is
disabled, which should allow scaler 1 to go into HQ mode.
In this case, scaler 0 to be allocated to plane B to operate in HQ mode.
Is it really bad to keep it on scaler 1 for a while until the next time
the plane is
added?
I guess it's not immediately clear to me why we need to not pull the
other planes into the transaction. Is this just to avoid doing
Re: [Intel-gfx] Deadlock in intel_user_framebuffer_destroy()
On Mon, Jun 15, 2015 at 08:53:02AM +0100, Chris Wilson wrote:
On Mon, Jun 15, 2015 at 09:44:15AM +0300, Jani Nikula wrote:
On Wed, 03 Jun 2015, Chris Wilson ch...@chris-wilson.co.uk wrote:
On Wed, Jun 03, 2015 at 03:43:32PM +0200, Lukas Wunner wrote:
Hi,
a deadlock was introduced by commit
60a5ca015ffd2aacfe5674b5a401cd2a37159e07
Author: Ville Syrjälä ville.syrj...@linux.intel.com
Date: Fri Jun 13 11:10:53 2014 +0300
drm/i915: Add locking around framebuffer_references--
The commit amended intel_display.c:intel_user_framebuffer_destroy() with
mutex_lock(dev-struct_mutex).
A few weeks prior Chris Wilson had amended intel_fbdev.c:intelfb_create()
with a call to drm_framebuffer_unreference() while dev-struct_mutex is
locked (commit edd586fe705e819bc711b5ed7194a0b6f9f1a7e1, drm/i915:
Discard
BIOS framebuffers too small to accommodate chosen mode).
Just move the mutex_lock down a step.
Lucas, did you try this?
There's a goto unlock that also needed to be disabled, such as
Your previous patch placed the mutex_lock before the goto out_unlock - I
fail to see what has been broken with that version? Can you resubmit that
as a proper patch with sob and Lucas' t-b?
Thanks, Daniel
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index dda99c0d6be1..fc7ec5138fb7 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -213,8 +213,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
bool prealloc = false;
int ret;
- mutex_lock(dev-struct_mutex);
-
if (intel_fb
(sizes-fb_width intel_fb-base.width ||
sizes-fb_height intel_fb-base.height)) {
@@ -229,7 +227,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
DRM_DEBUG_KMS(no BIOS fb, allocating a new one\n);
ret = intelfb_alloc(helper, sizes);
if (ret)
- goto out_unlock;
+ return ret;
intel_fb = ifbdev-fb;
} else {
DRM_DEBUG_KMS(re-using BIOS fb\n);
@@ -241,6 +239,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
obj = intel_fb-obj;
vma = i915_gem_obj_to_ggtt(obj, NULL);
+ mutex_lock(dev-struct_mutex);
info = framebuffer_alloc(0, dev-pdev-dev);
if (!info) {
ret = -ENOMEM;
@@ -311,7 +310,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
out_unpin:
drm_gem_object_unreference(obj-base);
-out_unlock:
mutex_unlock(dev-struct_mutex);
return ret;
}
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
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 4/5] drm/i915: Corrected the platform checks in i915_ring_freq_table function
On Mon, 2015-06-15 at 15:15 +0200, Daniel Vetter wrote:
On Sun, Jun 07, 2015 at 06:32:23PM +0530, akash.g...@intel.com wrote:
From: Akash Goel akash.g...@intel.com
Corrected the platform checks in i915_ring_freq_table debugfs function
so as to allow the read of ring frequency table for BDW and disallow for VLV
Issue: VIZ-5144
Signed-off-by: Akash Goel akash.g...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 47636f3..1c83596 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1779,7 +1779,8 @@ static int i915_ring_freq_table(struct seq_file *m,
void *unused)
int ret = 0;
int gpu_freq, ia_freq;
- if (!(IS_GEN6(dev) || IS_GEN7(dev))) {
+ if (!(IS_GEN6(dev) || (IS_GEN7(dev) !IS_VALLEYVIEW(dev)) ||
+ IS_BROADWELL(dev))) {
This is really hard to read with the double negation. What about
if (gen 6 || IS_VLV(dev))
/* not supported */
instaed? Presuming I decoded this correctly ...
Yes this way also it is fine. Will make this change.
Accordingly the next patch will also change, like this
if (gen 6 || IS_VLV(dev) || IS_BROXTON(dev))
/* not supported */
Best regards
Akash
-Daniel
seq_puts(m, unsupported on this chipset\n);
return 0;
}
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Report to userspace if we have a (presumed) working GPU reset
On Mon, Jun 15, 2015 at 02:53:41PM +0100, Chris Wilson wrote:
On Mon, Jun 15, 2015 at 03:45:38PM +0200, Daniel Vetter wrote:
On Mon, Jun 15, 2015 at 12:23:48PM +0100, Chris Wilson wrote:
In igt, we want to test handling of GPU hangs, both for recovery
purposes and for reporting. However, we don't want to inject a genuine
GPU hang onto a machine that cannot recover and so be permenantly
wedged. Rather than embed heuristics into igt, have the kernel report
exactly when it expects the GPU reset to work.
This can also be usefully extended in future to indicate different
levels of fine-grained resets.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Tim Gore tim.g...@intel.com
Cc: Tomas Elf tomas@intel.com
Yeah makes sense. Will merge as soon as someone smashes a t-b with a few
igt patches using this on top.
diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index deb5560..8a1ffb2 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -26,6 +26,7 @@
#include errno.h
#include sys/types.h
#include sys/stat.h
+#include sys/ioctl.h
#include fcntl.h
#include drmtest.h
@@ -47,6 +48,21 @@
* engines.
*/
+static bool has_gpu_reset(int fd)
+{
+ struct drm_i915_getparam gp;
+ int val = 0;
+
+ memset(gp, 0, sizeof(gp));
+ gp.param = 35; /* HAS_GPU_RESET */
+ gp.value = val;
+
+ if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, gp, sizeof(gp)))
+ return intel_gen(intel_get_drm_devid(fd)) = 5;
+
+ return val 0;
+}
/**
* igt_require_hang_ring:
@@ -60,7 +76,7 @@
void igt_require_hang_ring(int fd, int ring)
{
gem_context_require_ban_period(fd);
- igt_require(intel_gen(intel_get_drm_devid(fd)) = 5);
+ igt_require(has_gpu_reset(fd));
}
Speaking of which, do we want
igt_require(getenv(IGT_DISABLE_HANG) == NULL);
here?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Avoid fluctuating to UNKNOWN connector status during forced probes
On Mon, Jun 15, 2015 at 03:03:39PM +0200, Daniel Vetter wrote: On Mon, Jun 15, 2015 at 01:35:21PM +0100, Chris Wilson wrote: On Mon, Jun 15, 2015 at 02:29:28PM +0200, Daniel Vetter wrote: On Fri, Jun 05, 2015 at 08:46:19AM +0100, Chris Wilson wrote: For old-school component TV and CRT connectors, we require a heavyweight load-detection mechanism. This involves setting up a CRTC and sending a signal to the output, before reading back any response. As that is quite slow and CPU heavy, the process is only performed when the output detection is forced by user request. As it requires a driving CRTC, we often don't have the resources to complete the probe. This leaves us in a quandary where the unforced path just returns the old connector status, but the forced detection path elects to return UNKNOWN. If we have an active connection, we likely have the resources available to complete the probe - but if it is currently disconnected, then it becomes unknown and triggers a hotplug event, with often quite unfortunate userspace behaviour (e.g. one output is blanked and the spurious TV turned on). To reduce spurious hotplug events on older devices, we can prevent transitions between disconnected - unknown. v2: Convert tv_type to use proper unknown enum (Ville) References: https://bugs.freedesktop.org/show_bug.cgi?id=87049 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrj...@linux.intel.com [v1] Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com [v1] This solution is at odds with commit b7703726251191cd9f3ef3a80b2d9667901eec95 Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Wed Jan 21 08:45:22 2015 +0100 drm/probe-helper: clamp unknown connector status in the poll work We're missing this bit of logic from the hotplug handlers, but that was somewhat intentional since a hotplug should indicate that something has changed. And in the i915 hpd handler we filter by source. Given that the report is older than me having resurrect that patch can we close it as fixed or do I miss something? It's a different path. This also concerns the actual forced reprobe fluctuating between two states. In that case I still think it should be in the probe helpers. Which raises the policy decision whether we should ever hand unkown back to userspace since apparently it just can't cope sensible with it. But that call should be done consistently accross drivers. Hmm, the probe helper is not the central arbiter here which is a problem in moving it entirely into its domain. UNKNOWN makes a sense form the hw perspective, and we still need to report that status before any probes are made at all (whether that is init/resume) and the problem in this case is not so much as userspace mishandling it, but the fluctuation between UNKNOWN/DISCONNECTED is causing activity and reconfiguration not entirely of its own fault. -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 v6 2/8] drm/i915: Use cached cdclk value
On 06/15/2015 02:09 PM, Damien Lespiau wrote: On Mon, Jun 15, 2015 at 01:40:24PM +0100, Tvrtko Ursulin wrote: On 06/15/2015 01:14 PM, Damien Lespiau wrote: On Mon, Jun 15, 2015 at 01:54:40PM +0200, Daniel Vetter wrote: On Wed, Jun 03, 2015 at 03:45:08PM +0300, Mika Kahola wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Rather than reading out the current cdclk value use the cached value we have tucked away in dev_priv. v2: Rebased to the latest v3: Rebased to the latest v4: Fix for patch style problems Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Mika Kahola mika.kah...@intel.com This patch needs to be extended to also cover the recently added skl_max_scale. Tvrtko has recently written a patch to add some checks to that code too, would be good to resurrect that too. Chandra can help with any questions wrt the skl scaler code. Not quite I'm afraid. The CDCLK used in skl_max_scale() has to be part of the atomic state, even bumping CDCLK if possible/needed. If you use the cached cdclk in skl_max_scale(), it won't do the right thing when CDCLK is off (ie cached frew is the fallback 24Mhz ref clock) and we try to do the first modeset before waking up the display. I filed a bug about it already to track it: https://bugs.freedesktop.org/show_bug.cgi?id=90874 I know nothing about these specific clocks, but FWIW, my patch was only about enabling new platforms - making skl_max_scale more robust in cases where clock querying does not yet work correctly. My reasoning was based on a comment from Ville that one of those two clocks must never be lower than the other. So it sounded reasonable to ignore such cases ie. assume no scaling is possible and allow a normal (unscaled) modeset to succeed rather than fail it and display nothing. So to be more specific, I believe this is because we detect CDCLK as being disabled or on the ref clock in simulation? Probably a reference clock. It definitely wasn't zero since skl_max_scale already handles that. But I forgot the exact details. Generally speaking, it's questionable if we want to work around such limitations in the code like that, I'd rather go for defaulting to max CDCLK in simulation. In this particular case, we really shouldn't get cdclk crtc_clock at this point, I'd expect the cdclk we use (probably part of the atomic state) to be bumped to cover crtc_clock prior to plane checks (See Marteen's [PATCH v3 19/19] drm/i915: Make cdclk part of the atomic state.), I guess we could add a WARN_ON(cdclk crtc_clock) in skl_max_scale() to ensure that's indeed the case? WARN_ON sounds fine to me. For the other considerations - you're the expert. :) Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] Android.mk: Disable tools that do not build for android
-Original Message- From: Jani Nikula [mailto:jani.nik...@linux.intel.com] Sent: Monday, June 15, 2015 2:24 PM To: Morton, Derek J; Wood, Thomas Cc: Intel Graphics Development Subject: Re: [Intel-gfx] [PATCH i-g-t] Android.mk: Disable tools that do not build for android On Mon, 15 Jun 2015, Morton, Derek J derek.j.mor...@intel.com wrote: -Original Message- From: Thomas Wood [mailto:thomas.w...@intel.com] Sent: Monday, June 15, 2015 2:09 PM To: Morton, Derek J Cc: Intel Graphics Development; Gore, Tim Subject: Re: [PATCH i-g-t] Android.mk: Disable tools that do not build for android On 15 June 2015 at 14:04, Derek Morton derek.j.mor...@intel.com wrote: Disable the tools / demo code that do not currently build for android until they can be fixed. What needs to be fixed? Intel_reg - Relies on a header file not provided by the Android compiler ...which one? sys/io.h Tim Gore has looked into this issue. The complexity in fixing is more with the licencing than technical. //Derek Intel_display_crc - Cairo dependency Intel_sprite_on - An API difference in the Android i915 driver. //Derek Affected tools / demos intel_reg intel_display_crc intel_sprite_on Signed-off-by: Derek Morton derek.j.mor...@intel.com --- Android.mk | 2 +- tools/Android.mk | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Android.mk b/Android.mk index 1ab3e64..681d114 100644 --- a/Android.mk +++ b/Android.mk @@ -1,2 +1,2 @@ -include $(call all-named-subdir-makefiles, lib tests tools benchmarks demos) +include $(call all-named-subdir-makefiles, lib tests tools +benchmarks) diff --git a/tools/Android.mk b/tools/Android.mk index 39f4512..fd2800e 100644 --- a/tools/Android.mk +++ b/tools/Android.mk @@ -36,7 +36,9 @@ endef ## skip_tools_list := \ +intel_display_crc \ intel_framebuffer_dump \ +intel_reg \ intel_reg_dumper \ intel_vga_read \ intel_vga_write -- 1.9.1 ___ 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 igt 3/4] tests/kms_fbc_crc: run even if FBC is disabled by default
2015-06-15 9:11 GMT-03:00 Daniel Vetter dan...@ffwll.ch: On Thu, Jun 04, 2015 at 11:31:05AM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com We may not be perfect, but if we don't even test, we will probably only get worse over time. The function called makes sure we restore whatever was the original FBC parameter when we exit the test, so this should not affect the other tests. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- tests/kms_fbc_crc.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c index 37221ac..d964224 100644 --- a/tests/kms_fbc_crc.c +++ b/tests/kms_fbc_crc.c @@ -559,10 +559,10 @@ igt_main igt_assert_lt(0, fread(buf, 1, sizeof(buf), status)); fclose(status); buf[sizeof(buf) - 1] = '\0'; - igt_require_f(!strstr(buf, unsupported on this chipset) - !strstr(buf, disabled per module param) - !strstr(buf, disabled per chip default), - FBC not supported/enabled\n); + igt_require_f(!strstr(buf, unsupported on this chipset), + FBC not supported\n); + + igt_set_module_param_int(enable_fbc, 1); This is risky since on older chips fbc is known to pretty much insta-kill the box. Imo we should have a gen6+ here at least (that's roughly the point where the hw workarounds start to sound less scary). Well, we can try and then keep an eye on bugzilla, watching for the possible insta-kill bug reports, can't we? At least to me, this was not known as instal-kill since it's not documented anywhere and we don't have bug reports. Also, the move to the frontbuffer tracking infrastructure could maybe have solved the insta-kill problem. Trying to enable this on older platforms just isn't worth it imo. If we're not even going to try this, shouldn't we remove all the Kernel code support for FBC on these platforms? This is something I wanted to discuss at some point, but now you gave us a nice excuse :). Maybe it's better to just not have the code at all vs have a code that (maybe) instakills the machine and we're probably not going to fix anytime soon. I don't know, just an idea. Btw, those patches are already on IGT. Thomas seemed to be OK with the ideas, and after one week on the mailing list I pushed them. -Daniel data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096); igt_assert(data.bufmgr); -- 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 http://blog.ffwll.ch -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/7] drm/i915/gen8: Add infrastructure to initialize WA batch buffers
On 15/06/2015 11:41, Daniel Vetter wrote: On Thu, Jun 04, 2015 at 03:30:56PM +0100, Siluvery, Arun wrote: On 02/06/2015 19:47, Dave Gordon wrote: On 02/06/15 19:36, Siluvery, Arun wrote: On 01/06/2015 11:22, Daniel, Thomas wrote: Indeed, allocating an extra scratch page in the context would simplify vma/mm management. A trick might be to allocate the scratch page at the start, then offset the lrc regs etc - that would then be consistent amongst gen and be easy enough to extend if we need more per-context scratch space in future. -Chris Yes, I think we already have another use for more per-context space at the start. The GuC is planning to do this. Arun, you probably should work with Alex Dai and Dave Gordon to avoid conflicts here. Thomas. Thanks for the heads-up Thomas. I have discussed with Dave and agreed to share this page; GuC probably doesn't need whole page so first half is reserved for it's use and second half is used for WA. I have modified my patches to use context page for applying these WA and don't see any issues. During the discussions Dave proposed another approach. Even though these WA are called per context they are only initialized once and not changed afterwards, same set of WA are applied for each context so instead of adding them in each context, does it make sense to create a separate page and share across all contexts? but of course GuC will anyway add a new page to context so I might as well share that page. Chris/Dave, do you see any problems with sharing page with GuC or you prefer to allocate a separate page for these WA and share across all contexts? Please give your comments. regards Arun I think we have to consider which is more future-proof i.e. which is least likely: (1) the area shared with the GuC grows (definitely still in flux), or (2) workarounds need to be context-specific (possible, but unlikely) So I'd prefer a single area set up just once to contain the pre- and post-context restore workaround batches. If necessary, the one area could contain multiple batches at different offsets, so we could point different contexts at different (shared) batches as required. I think they're unlikely to actually need per-context customisation[*], but there might be a need for different workarounds according to workload type or privilege level or some other criterion ... ? .Dave. [*] unless they need per-context memory addresses coded into them? Considering these WA are initialized only once and not changed afterwards and GuC area probably grows in future which may run into the space used by WA, independent single area setup makes senses. I also checked spec and it is not clear whether any customization is going to be required for different contexts. I have modified patches to setup a single page with WA when default_context is initialized and this is used by all contexts. I will send patches but please let me know if there are any other comments. Yeah if the wa batches aren't ctx specific, then there's really no need to allocate one of them per ctx. One global buffer with all the wa combined should really be all we need. -Daniel Hi Daniel, Agree, this is already taken into account in the next revision v3 (already sent to mailing list). I can see you are still going through the list but when you get there, please let me know if you have any other comments. regards Arun ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 2/8] drm/i915: Use cached cdclk value
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
Vetter
Sent: Monday, June 15, 2015 2:55 PM
To: Kahola, Mika
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v6 2/8] drm/i915: Use cached cdclk value
On Wed, Jun 03, 2015 at 03:45:08PM +0300, Mika Kahola wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Rather than reading out the current cdclk value use the cached value
we have tucked away in dev_priv.
v2: Rebased to the latest
v3: Rebased to the latest
v4: Fix for patch style problems
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Mika Kahola mika.kah...@intel.com
This patch needs to be extended to also cover the recently added
skl_max_scale. Tvrtko has recently written a patch to add some checks to
that code too, would be good to resurrect that too. Chandra can help with
any questions wrt the skl scaler code.
Cheers, Daniel
Jani has pushed these patches already so maybe this is an item for a separate
patch?
Cheers,
Mika
Author:Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 3 +--
drivers/gpu/drm/i915/intel_dp.c | 5 +++--
drivers/gpu/drm/i915/intel_pm.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 9cf1553..d1dd8ab 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6610,8 +6610,7 @@ static int intel_crtc_compute_config(struct
intel_crtc *crtc,
/* FIXME should check pixel clock limits on all platforms */
if (INTEL_INFO(dev)-gen 4) {
- int clock_limit =
- dev_priv-display.get_display_clock_speed(dev);
+ int clock_limit = dev_priv-cdclk_freq;
/*
* Enable pixel doubling when the dot clock diff --git
a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6f525093..9a6517d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -708,7 +708,8 @@ static uint32_t ilk_get_aux_clock_divider(struct
intel_dp *intel_dp, int index)
return 0;
if (intel_dig_port-port == PORT_A) {
- return DIV_ROUND_UP(dev_priv-
display.get_display_clock_speed(dev), 2000);
+ return DIV_ROUND_UP(dev_priv-cdclk_freq, 2000);
+
} else {
return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
}
@@ -723,7 +724,7 @@ static uint32_t hsw_get_aux_clock_divider(struct
intel_dp *intel_dp, int index)
if (intel_dig_port-port == PORT_A) {
if (index)
return 0;
- return DIV_ROUND_CLOSEST(dev_priv-
display.get_display_clock_speed(dev), 2000);
+ return DIV_ROUND_CLOSEST(dev_priv-cdclk_freq, 2000);
} else if (dev_priv-pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
/* Workaround for non-ULT HSW */
switch (index) {
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index eadc15c..5db429e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1815,7 +1815,7 @@ hsw_compute_linetime_wm(struct drm_device
*dev, struct drm_crtc *crtc)
linetime = DIV_ROUND_CLOSEST(mode-crtc_htotal * 1000 * 8,
mode-crtc_clock);
ips_linetime = DIV_ROUND_CLOSEST(mode-crtc_htotal * 1000 * 8,
-dev_priv-
display.get_display_clock_speed(dev_priv-dev));
+dev_priv-cdclk_freq);
return PIPE_WM_LINETIME_IPS_LINETIME(ips_linetime) |
PIPE_WM_LINETIME_TIME(linetime);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
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 v6 2/8] drm/i915: Use cached cdclk value
On Mon, Jun 15, 2015 at 12:21:17PM +, Kahola, Mika wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Monday, June 15, 2015 2:55 PM To: Kahola, Mika Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v6 2/8] drm/i915: Use cached cdclk value On Wed, Jun 03, 2015 at 03:45:08PM +0300, Mika Kahola wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Rather than reading out the current cdclk value use the cached value we have tucked away in dev_priv. v2: Rebased to the latest v3: Rebased to the latest v4: Fix for patch style problems Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Mika Kahola mika.kah...@intel.com This patch needs to be extended to also cover the recently added skl_max_scale. Tvrtko has recently written a patch to add some checks to that code too, would be good to resurrect that too. Chandra can help with any questions wrt the skl scaler code. Cheers, Daniel Jani has pushed these patches already so maybe this is an item for a separate patch? Yeah that's what I've meant with extending. Please work together with Damien and Chandra and Maarten in figuring out what exactly is needed here. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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 i-g-t] Android.mk: Disable tools that do not build for android
On 15 June 2015 at 14:04, Derek Morton derek.j.mor...@intel.com wrote: Disable the tools / demo code that do not currently build for android until they can be fixed. What needs to be fixed? Affected tools / demos intel_reg intel_display_crc intel_sprite_on Signed-off-by: Derek Morton derek.j.mor...@intel.com --- Android.mk | 2 +- tools/Android.mk | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Android.mk b/Android.mk index 1ab3e64..681d114 100644 --- a/Android.mk +++ b/Android.mk @@ -1,2 +1,2 @@ -include $(call all-named-subdir-makefiles, lib tests tools benchmarks demos) +include $(call all-named-subdir-makefiles, lib tests tools benchmarks) diff --git a/tools/Android.mk b/tools/Android.mk index 39f4512..fd2800e 100644 --- a/tools/Android.mk +++ b/tools/Android.mk @@ -36,7 +36,9 @@ endef ## skip_tools_list := \ +intel_display_crc \ intel_framebuffer_dump \ +intel_reg \ intel_reg_dumper \ intel_vga_read \ intel_vga_write -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/irq: wait a little before queuing the hotplug work function
On Thu, 11 Jun 2015, Jani Nikula jani.nik...@intel.com wrote:
Currently it's possible this happens when a (non-DP) cable is unplugged:
- user starts unplugging cable
- hotplug irq fires
- hotplug work function runs
- connector detect function runs
- ddc pin is still connected, edid read succeeds
- we decide nothing changed, no uevent
- cable completely unplugged
- our state is inconsistent with reality, no power save
The user plugs cable back in:
- most of the same at first
- ddc pin connected, edid read succeeds
- we decide nothing changed because we thought the cable was plugged
in all along, no uevent
- cable completely plugged
- our state is somewhat consistent, however monitor might be
different, the link might not recover?
With our current implementation we rely on the hotplug pin being *both*
the last to be connected on plug *and* last to be disconnected on
unplug. The educated guess is that this is not the case.
Per the logs in the case of the referenced bug, the hdmi detect code
runs within a few *microseconds* after the hotplug irq, and the EDID has
been successfully read within 25 ms of the irq. If the DDC lines are
still connected when the hotplug irq fires, the user has to be blazingly
fast to complete the unplug before the detect code runs.
We can afford to wait a little before queuing the work function for a
bit more reliability. Obviously it's still possible for the user to
unplug the cable really slowly, but let's at least give the user a
fighting chance to unplug fast enough.
I'd love to claim the proposed delay of 400 ms is based on real life
measured data and rigorous analysis, but in truth it is just a gut
feeling based compromise between solving the issue and meeting some
vague real time human interaction deadline for having a picture on
screen. (However I expect any criticism to be more substantiated than
my gut feeling is better than yours.)
An alternative would be to check the hotplug state in the irq handler,
but there have been reliability problems with it in the past, and we've
opted not to use it. [citation needed]
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82593
Tested-by: Hugh Greenberg hugegreen...@gmail.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Jani Nikula jani.nik...@intel.com
Daniel wants a fancier solution: queing a one-shot re-check after a
while in the cases that the connector state appears to not have
changed. It'll take a while until I get to it. But this patch is not
going anywhere.
BR,
Jani.
---
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_irq.c | 12
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 78ef0bb53c36..002767ddfe06 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -552,7 +552,7 @@ void intel_hpd_cancel_work(struct drm_i915_private
*dev_priv)
spin_unlock_irq(dev_priv-irq_lock);
cancel_work_sync(dev_priv-hotplug.dig_port_work);
- cancel_work_sync(dev_priv-hotplug.hotplug_work);
+ cancel_delayed_work_sync(dev_priv-hotplug.hotplug_work);
cancel_delayed_work_sync(dev_priv-hotplug.reenable_work);
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 611fbd86c1cc..a8a99f658772 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -221,7 +221,7 @@ enum hpd_pin {
for ((__pin) = (HPD_NONE + 1); (__pin) HPD_NUM_PINS; (__pin)++)
struct i915_hotplug {
- struct work_struct hotplug_work;
+ struct delayed_work hotplug_work;
struct {
unsigned long last_jiffies;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 56db9e747464..bab67c279c22 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -828,6 +828,8 @@ static bool intel_hpd_irq_event(struct drm_device *dev,
return true;
}
+#define HOTPLUG_DELAY_MS 400
+
static void i915_digport_work_func(struct work_struct *work)
{
struct drm_i915_private *dev_priv =
@@ -872,7 +874,8 @@ static void i915_digport_work_func(struct work_struct
*work)
spin_lock_irq(dev_priv-irq_lock);
dev_priv-hotplug.event_bits |= old_bits;
spin_unlock_irq(dev_priv-irq_lock);
- schedule_work(dev_priv-hotplug.hotplug_work);
+ mod_delayed_work(system_wq, dev_priv-hotplug.hotplug_work,
+ msecs_to_jiffies(HOTPLUG_DELAY_MS));
}
}
@@ -884,7 +887,7 @@ static void i915_digport_work_func(struct work_struct
*work)
static void i915_hotplug_work_func(struct work_struct *work)
{
struct drm_i915_private *dev_priv =
- container_of(work, struct
Re: [Intel-gfx] [PATCH i-g-t] Android.mk: Disable tools that do not build for android
On Mon, Jun 15, 2015 at 01:19:16PM +, Morton, Derek J wrote: -Original Message- From: Thomas Wood [mailto:thomas.w...@intel.com] Sent: Monday, June 15, 2015 2:09 PM To: Morton, Derek J Cc: Intel Graphics Development; Gore, Tim Subject: Re: [PATCH i-g-t] Android.mk: Disable tools that do not build for android On 15 June 2015 at 14:04, Derek Morton derek.j.mor...@intel.com wrote: Disable the tools / demo code that do not currently build for android until they can be fixed. What needs to be fixed? Intel_reg - Relies on a header file not provided by the Android compiler Intel_display_crc - Cairo dependency Cairo should be available by now on android, at least on some builds. The test at least go to great pains to build if cairo is around. Imo we should adopt it. Intel_sprite_on - An API difference in the Android i915 driver. This tool is only around for the benefit of the android display folks. If you dont need it then it's time to remove it. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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/atomic: pass old crtc state to atomic_begin/flush.
On Mon, Jun 15, 2015 at 11:18:46AM +0200, Maarten Lankhorst wrote: Op 15-06-15 om 11:13 schreef Daniel Vetter: On Mon, Jun 15, 2015 at 09:30:19AM +0200, Maarten Lankhorst wrote: Op 15-06-15 om 09:10 schreef Daniel Vetter: On Fri, Jun 12, 2015 at 11:18:22AM +0200, Maarten Lankhorst wrote: In intel it's useful to keep track of some state changes with old crtc state vs new state, for example to disable initial planes or when a modeset's prevented during fastboot. Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com Hm, thus far the approach has been that the various -check callbacks diff the state and set appropriate stuff like needs_modeset or planes_changed. And with intel_crtc-atomic we've kinda started to build up similar things for i915. What do you plan to use this for? -Daniel On a modeset I want to disable all old planes by calling plane-disable_plane, which is old_crtc_state-plane_mask. This is for initial hw readout, where a plane might be active without a fb set. I want to run it during vblank evasion if possible, which means in atomic_begin or flush. commit_plane is not called if the old and new state both have a NULL fb, so the initial plane would stay active in this case. Hm, so this is for the i915 state readout code. Imo we shouldn't ever leak this out of the state readout code but instead sanitize the plane state to make sense. Roughly this would be: - read out crtc state - try to reconstruct initial fb for primary plane, if this succeeds then fully link up the plane with the crtc in the plane_state. Agreed. Right now get_initial_plane_config takes an initial_plane_state, could we make this atomic too? The initial fb takeover code is a bit tricky since we need to temporarily store a few things while not everything is set up yet fully. We could try to move that information into the plane state, but it would duplicate existing information stored in state-fb-i915_gem_object. Not sure whether it's worth it to have something fully atomic for plane state readout. The other option would be to allow enabled planes without a full-blown fb object, but experience says this leads to piles of drama in the watermark code. - then walk all planes for the crtc, and if any plane is enabled in the hw state but doesn't have fb/crtc set in the plane_state force-disable it. Can we disable those planes without penalty? Some of them call watermark update, this is a bug but still.. What kind of penalty do you think of? Performance doesn't matter since if we get a bad state and need to frob planes around we'll likely also need a full modeset anyway. Wrt watermark updates that needs to be avoided ofc, so similar to the force disable all planes in crtc_disable problem. We might be able to reuse a bit of code for that. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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: Avoid fluctuating to UNKNOWN connector status during forced probes
On Fri, Jun 05, 2015 at 08:46:19AM +0100, Chris Wilson wrote:
For old-school component TV and CRT connectors, we require a heavyweight
load-detection mechanism. This involves setting up a CRTC and sending a
signal to the output, before reading back any response. As that is quite
slow and CPU heavy, the process is only performed when the output
detection is forced by user request. As it requires a driving CRTC, we
often don't have the resources to complete the probe. This leaves us in
a quandary where the unforced path just returns the old connector
status, but the forced detection path elects to return UNKNOWN. If we
have an active connection, we likely have the resources available to
complete the probe - but if it is currently disconnected, then it
becomes unknown and triggers a hotplug event, with often quite unfortunate
userspace behaviour (e.g. one output is blanked and the spurious TV
turned on).
To reduce spurious hotplug events on older devices, we can prevent
transitions between disconnected - unknown.
v2: Convert tv_type to use proper unknown enum (Ville)
References: https://bugs.freedesktop.org/show_bug.cgi?id=87049
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ville Syrjälä ville.syrj...@linux.intel.com [v1]
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com [v1]
This solution is at odds with
commit b7703726251191cd9f3ef3a80b2d9667901eec95
Author: Daniel Vetter daniel.vet...@ffwll.ch
Date: Wed Jan 21 08:45:22 2015 +0100
drm/probe-helper: clamp unknown connector status in the poll work
We're missing this bit of logic from the hotplug handlers, but that was
somewhat intentional since a hotplug should indicate that something has
changed. And in the i915 hpd handler we filter by source.
Given that the report is older than me having resurrect that patch can we
close it as fixed or do I miss something?
Thanks, Daniel
---
drivers/gpu/drm/i915/intel_crt.c | 39 ++-
drivers/gpu/drm/i915/intel_tv.c | 37 ++---
2 files changed, 32 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_crt.c
b/drivers/gpu/drm/i915/intel_crt.c
index 521af2c..70c5288 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -666,8 +666,6 @@ intel_crt_detect(struct drm_connector *connector, bool
force)
struct intel_encoder *intel_encoder = crt-base;
enum intel_display_power_domain power_domain;
enum drm_connector_status status;
- struct intel_load_detect_pipe tmp;
- struct drm_modeset_acquire_ctx ctx;
DRM_DEBUG_KMS([CONNECTOR:%d:%s] force=%d\n,
connector-base.id, connector-name,
@@ -703,27 +701,26 @@ intel_crt_detect(struct drm_connector *connector, bool
force)
goto out;
}
- if (!force) {
- status = connector-status;
- goto out;
- }
-
- drm_modeset_acquire_init(ctx, 0);
-
- /* for pre-945g platforms use load detect */
- if (intel_get_load_detect_pipe(connector, NULL, tmp, ctx)) {
- if (intel_crt_detect_ddc(connector))
- status = connector_status_connected;
- else if (INTEL_INFO(dev)-gen 4)
- status = intel_crt_load_detect(crt);
- else
+ status = connector-status;
+ if (force) {
+ struct intel_load_detect_pipe tmp;
+ struct drm_modeset_acquire_ctx ctx;
+
+ drm_modeset_acquire_init(ctx, 0);
+
+ /* for pre-945g platforms use load detect */
+ if (intel_get_load_detect_pipe(connector, NULL, tmp, ctx)) {
+ if (intel_crt_detect_ddc(connector))
+ status = connector_status_connected;
+ else if (INTEL_INFO(dev)-gen 4)
+ status = intel_crt_load_detect(crt);
+ intel_release_load_detect_pipe(connector, tmp, ctx);
+ } else if (status == connector_status_connected)
status = connector_status_unknown;
- intel_release_load_detect_pipe(connector, tmp, ctx);
- } else
- status = connector_status_unknown;
- drm_modeset_drop_locks(ctx);
- drm_modeset_acquire_fini(ctx);
+ drm_modeset_drop_locks(ctx);
+ drm_modeset_acquire_fini(ctx);
+ }
out:
intel_display_power_put(dev_priv, power_domain);
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 8b9d325..135584f 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1233,7 +1233,6 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
intel_wait_for_vblank(intel_tv-base.base.dev,
to_intel_crtc(intel_tv-base.base.crtc)-pipe);
- type = -1;
tv_dac = I915_READ(TV_DAC);
[Intel-gfx] [PATCH i-g-t] Android.mk: Disable tools that do not build for android
Disable the tools / demo code that do not currently build for android until they can be fixed. Affected tools / demos intel_reg intel_display_crc intel_sprite_on Signed-off-by: Derek Morton derek.j.mor...@intel.com --- Android.mk | 2 +- tools/Android.mk | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Android.mk b/Android.mk index 1ab3e64..681d114 100644 --- a/Android.mk +++ b/Android.mk @@ -1,2 +1,2 @@ -include $(call all-named-subdir-makefiles, lib tests tools benchmarks demos) +include $(call all-named-subdir-makefiles, lib tests tools benchmarks) diff --git a/tools/Android.mk b/tools/Android.mk index 39f4512..fd2800e 100644 --- a/tools/Android.mk +++ b/tools/Android.mk @@ -36,7 +36,9 @@ endef ## skip_tools_list := \ +intel_display_crc \ intel_framebuffer_dump \ +intel_reg \ intel_reg_dumper \ intel_vga_read \ intel_vga_write -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] intel_reg: install and load the register files
On 10 June 2015 at 11:30, Thomas Wood thomas.w...@intel.com wrote:
Cc: Jani Nikula jani.nik...@intel.com
Signed-off-by: Thomas Wood thomas.w...@intel.com
I haven't pushed this yet because there seems to be a discrepancy
between the built in register list and that from the quick_dump files.
Jani also suggested it may be a better idea to use the files to update
the built in list so that the definitions are always included with the
binary.
---
configure.ac | 5 +++--
tools/Makefile.am| 3 ++-
tools/intel_reg.c| 4 ++--
tools/quick_dump/Makefile.am | 13 -
4 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/configure.ac b/configure.ac
index c370ec3..33caddc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -230,9 +230,10 @@ AC_DEFINE_UNQUOTED(TARGET_CPU_PLATFORM, [$host_cpu],
[Target platform])
files=broadwell cherryview haswell ivybridge sandybridge valleyview skylake
for file in $files; do
- QUICK_DUMP_EXTRA_DIST=$QUICK_DUMP_EXTRA_DIST $file `tr '\n' ' '
$srcdir/tools/quick_dump/$file`
+ REGSPECFILES=$REGSPECFILES $file `cat
$srcdir/tools/quick_dump/$file`
done
-AC_SUBST(QUICK_DUMP_EXTRA_DIST)
+REGSPECFILES=`echo $REGSPECFILES | tr ' ' '\n' | sort -u | tr '\n' ' '`
+AC_SUBST(REGSPECFILES)
AC_CONFIG_FILES([
Makefile
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 04bfd12..f673f3c 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -7,6 +7,7 @@ SUBDIRS += quick_dump
endif
AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib
-AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS)
$(LIBUNWIND_CFLAGS)
+AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) \
+ $(LIBUNWIND_CFLAGS) -DREGSPECDIR=\$(pkgdatadir)/registers\
LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS)
$(CAIRO_LIBS) $(LIBUDEV_LIBS) $(LIBUNWIND_LIBS)
diff --git a/tools/intel_reg.c b/tools/intel_reg.c
index 090cc25..66ec485 100644
--- a/tools/intel_reg.c
+++ b/tools/intel_reg.c
@@ -698,7 +698,7 @@ static int get_reg_spec_file(char *buf, size_t buflen,
const char *dir,
static int read_reg_spec(struct config *config)
{
char buf[PATH_MAX];
- char *path;
+ const char *path;
struct stat st;
int r;
@@ -707,7 +707,7 @@ static int read_reg_spec(struct config *config)
path = getenv(INTEL_REG_SPEC);
if (!path)
- goto builtin;
+ path = REGSPECDIR;
r = stat(path, st);
if (r) {
diff --git a/tools/quick_dump/Makefile.am b/tools/quick_dump/Makefile.am
index 3d0bd23..b526d19 100644
--- a/tools/quick_dump/Makefile.am
+++ b/tools/quick_dump/Makefile.am
@@ -25,9 +25,12 @@ chipset_wrap_python.c: chipset.i
all-local: I915ChipsetPython.la
$(LN_S) -f .libs/I915ChipsetPython.so _chipset.so
+regspecdir = $(pkgdatadir)/registers
+dist_regspec_DATA = $(REGSPECFILES) \
+ base_interrupt.txt base_other.txt base_power.txt \
+ base_rings.txt
+
CLEANFILES = chipset_wrap_python.c chipset.py _chipset.so
-EXTRA_DIST = $(QUICK_DUMP_EXTRA_DIST) \
- base_interrupt.txt base_other.txt base_power.txt base_rings.txt
\
- quick_dump.py \
- reg_access.py \
- chipset.i
+EXTRA_DIST = quick_dump.py \
+reg_access.py \
+chipset.i
--
2.4.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/5] drm/i915: Corrected the platform checks in i915_ring_freq_table function
On Sun, Jun 07, 2015 at 06:32:23PM +0530, akash.g...@intel.com wrote:
From: Akash Goel akash.g...@intel.com
Corrected the platform checks in i915_ring_freq_table debugfs function
so as to allow the read of ring frequency table for BDW and disallow for VLV
Issue: VIZ-5144
Signed-off-by: Akash Goel akash.g...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 47636f3..1c83596 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1779,7 +1779,8 @@ static int i915_ring_freq_table(struct seq_file *m,
void *unused)
int ret = 0;
int gpu_freq, ia_freq;
- if (!(IS_GEN6(dev) || IS_GEN7(dev))) {
+ if (!(IS_GEN6(dev) || (IS_GEN7(dev) !IS_VALLEYVIEW(dev)) ||
+ IS_BROADWELL(dev))) {
This is really hard to read with the double negation. What about
if (gen 6 || IS_VLV(dev))
/* not supported */
instaed? Presuming I decoded this correctly ...
-Daniel
seq_puts(m, unsupported on this chipset\n);
return 0;
}
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
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 i-g-t v4] lib/tests/igt_segfault Add unit test to test segfault handling
On Fri, Jun 12, 2015 at 10:08:43AM +, Morton, Derek J wrote:
This is the same as the previously submitted patch except the text
encoding should (hopefully) now be correct.
Hm, I failed to push out my igt branch with the patch I applied before
vacation, done that now. Is there any material difference? Or just the
content-enconding thing?
-Daniel
//Derek
-Original Message-
From: Morton, Derek J
Sent: Friday, June 12, 2015 11:05 AM
To: intel-gfx@lists.freedesktop.org
Cc: Wood, Thomas; dan...@ffwll.ch; Morton, Derek J
Subject: [PATCH i-g-t v4] lib/tests/igt_segfault Add unit test to test
segfault handling
Unit test to check a segfaulting subtest is handled correctly.
v2: Added script to check subtest results
v3: Removed script. Updated test to use fork to monitor return status.
v4: Added igt_segfault to .gitignore
Signed-off-by: Derek Morton derek.j.mor...@intel.com
---
lib/tests/.gitignore | 1 +
lib/tests/Makefile.sources | 1 +
lib/tests/igt_segfault.c | 139
+
3 files changed, 141 insertions(+)
create mode 100644 lib/tests/igt_segfault.c
diff --git a/lib/tests/.gitignore b/lib/tests/.gitignore index
a745a23..729568b 100644
--- a/lib/tests/.gitignore
+++ b/lib/tests/.gitignore
@@ -5,6 +5,7 @@ igt_list_only
igt_no_exit
igt_no_exit_list_only
igt_no_subtest
+igt_segfault
igt_simple_test_subtests
igt_simulation
igt_timeout
diff --git a/lib/tests/Makefile.sources b/lib/tests/Makefile.sources index
10e0617..5fa0b31 100644
--- a/lib/tests/Makefile.sources
+++ b/lib/tests/Makefile.sources
@@ -8,6 +8,7 @@ check_PROGRAMS = \
igt_simple_test_subtests \
igt_timeout \
igt_invalid_subtest_name \
+igt_segfault \
$(NULL)
check_SCRIPTS = \
diff --git a/lib/tests/igt_segfault.c b/lib/tests/igt_segfault.c new file
mode 100644 index 000..b420b1a
--- /dev/null
+++ b/lib/tests/igt_segfault.c
@@ -0,0 +1,139 @@
+/*
+ * 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:
+ *Derek Morton derek.j.mor...@intel.com
+ *
+ */
+
+/*
+ * Testcase: Test the framework catches a segfault and returns an error.
+ *
+ * 1. Test a crashing simple test is reported.
+ * 2. Test a crashing subtest is reported.
+ * 3. Test a crashing subtest following a passing subtest is reported.
+ * 4. Test a crashing subtest preceeding a passing subtest is reported.
+ */
+
+#include stdlib.h
+#include sys/wait.h
+#include sys/types.h
+#include assert.h
+#include errno.h
+
+#include drmtest.h
+#include igt_core.h
+
+/*
+ * We need to hide assert from the cocci igt test refactor spatch.
+ *
+ * IMPORTANT: Test infrastructure tests are the only valid places where
+using
+ * assert is allowed.
+ */
+#define internal_assert assert
+
+bool simple;
+bool runa;
+bool runc;
+char test[] = test;
+char *argv_run[] = { test };
+
+static int do_fork(void)
+{
+int pid, status;
+int argc;
+void (*crashme)(void) = NULL;
+
+switch (pid = fork()) {
+case -1:
+internal_assert(0);
+case 0:
+if (simple) {
+argc = 1;
+igt_simple_init(argc, argv_run);
+crashme();
+
+igt_exit();
+} else {
+
+argc = 1;
+igt_subtest_init(argc, argv_run);
+
+if(runa)
+igt_subtest(A)
+;
+
+igt_subtest(B)
+crashme();
+
+if(runc)
+igt_subtest(C)
+;
+
+igt_exit();
+}
+
Re: [Intel-gfx] [PATCH v3 6/6] drm/i915/gen8: Add WaRsRestoreWithPerCtxtBb workaround
On 12/06/2015 18:03, Dave Gordon wrote: On 12/06/15 12:58, Siluvery, Arun wrote: On 09/06/2015 19:43, Dave Gordon wrote: On 05/06/15 14:57, Arun Siluvery wrote: In Per context w/a batch buffer, WaRsRestoreWithPerCtxtBb v2: This patches modifies definitions of MI_LOAD_REGISTER_MEM and MI_LOAD_REGISTER_REG; Add GEN8 specific defines for these instructions so as to not break any future users of existing definitions (Michel) Signed-off-by: Rafael Barbalho rafael.barba...@intel.com Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 26 ++ drivers/gpu/drm/i915/intel_lrc.c | 59 2 files changed, 85 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 33b0ff1..6928162 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h [snip] #define MI_LOAD_REGISTER_MEMMI_INSTR(0x29, 0) #define MI_LOAD_REGISTER_REGMI_INSTR(0x2A, 0) +#define MI_LOAD_REGISTER_MEM_GEN8 MI_INSTR(0x29, 2) +#define MI_LRM_USE_GLOBAL_GTT (122) +#define MI_LRM_ASYNC_MODE_ENABLE (121) +#define MI_LOAD_REGISTER_REG_GEN8 MI_INSTR(0x2A, 1) Isn't the original definition of MI_LOAD_REGISTER_REG wrong anyway? It's a two-operand instruction, each of which is a one-word MMIO register address, hence always 3 words total. The length bias is 2, so the so-called 'flags' field must be 1. The original definition (where the second argument of the MI_INSTR macro is 0) shouldn't work. So just correct the original definition of MI_LOAD_REGISTER_REG; this isn't something that's actually changed on GEN8. I did notice that the original instructions are odd but thought I might be wrong hence I created new ones to not disturb the original ones. ok I will just correct original one and reuse it. While we're mentioning it, I think the above MI_LOAD_REGISTER_MEM is wrong too. The length should be 1 pre-GEN8, and 2 in GEN8+. ok. #define MI_RS_STORE_DATA_IMMMI_INSTR(0x2B, 0) #define MI_LOAD_URB_MEM MI_INSTR(0x2C, 0) #define MI_STORE_URB_MEMMI_INSTR(0x2D, 0) And these are wrong too! In fact all of these instructions have been added under a comment which says Commands used only by the command parser. Looks like they were added as placeholders without the proper length fields, and then people have started using them as though they were complete definitions :( Time update them all, perhaps ... these are not related to this patch, so it can be taken up as a different patch. As a minimum, you should move these updated #defines out of the section under the comment Commands used only by the command parser and put them in the appropriate place in the regular list of MI_ commnds, preferably in numerical order. Then the ones that are genuinely only used by the command parser could be left for another patch ... [snip] +/* + * BSpec says MI_LOAD_REGISTER_MEM, MI_LOAD_REGISTER_REG and + * MI_BATCH_BUFFER_END instructions in this sequence need to be + * in the same cacheline. + */ +while (((unsigned long) (cmd + index) % CACHELINE_BYTES) != 0) +cmd[index++] = MI_NOOP; + +cmd[index++] = MI_LOAD_REGISTER_MEM_GEN8 | +MI_LRM_USE_GLOBAL_GTT | +MI_LRM_ASYNC_MODE_ENABLE; +cmd[index++] = INSTPM; +cmd[index++] = scratch_addr; +cmd[index++] = 0; + +/* + * BSpec says there should not be any commands programmed + * between MI_LOAD_REGISTER_REG and MI_BATCH_BUFFER_END so + * do not add any new commands + */ +cmd[index++] = MI_LOAD_REGISTER_REG_GEN8; +cmd[index++] = GEN8_RS_PREEMPT_STATUS; +cmd[index++] = GEN8_RS_PREEMPT_STATUS; + /* padding */ while (index end) cmd[index++] = MI_NOOP; Where's the MI_BATCH_BUFFER_END referred to in the comment? MI_BATCH_BUFFER_END is just below while loop, it is in patch [v3 1/6]. Since the diff context is only few lines it didn't showup in the diff. The second comment above says no commands between LOAD_REG_REG and BB_END, so the point of my comment was that the BB_END is *NOT* immediately after the LOAD_REG_REG -- there are a bunch of no-ops there! true, but they are no-ops so they shouldn't really affect anything. I guess the spec means no valid commands. And therefore also, these instructions do *not* all end up in the same cacheline, thus contradicting the first comment above. I don't understand why. As per the requirement the commands from the first MI_LOAD_REGISTER_MEM_GEN8 (after while) through BB_END will be part of same cacheline (in this case second line). Padding *after* a BB_END would be redundant. yes, I just wanted to keep MI_BATCH_BUFFER_END at the end instead of abruptly terminating the batch which is why I am padding with no-ops, I can change this if that is preferred. .Dave. ___ Intel-gfx mailing list
Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Add debug messages at the start/end of DMC firmware loading
On Thu, Jun 04, 2015 at 06:10:04PM +0100, Damien Lespiau wrote: On Thu, Jun 04, 2015 at 04:47:00PM +0100, Chris Wilson wrote: On Thu, Jun 04, 2015 at 04:42:16PM +0100, Damien Lespiau wrote: It's handy to have debug message for the big events and this one qualifies IMHO. Also helpful to see what's happening while we're loading the firwmare and how much time it takes. Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/intel_csr.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index aae0652..2cf25ee 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -389,6 +389,7 @@ static void finish_csr_load(const struct firmware *fw, void *context) intel_csr_load_program(dev); fw_loaded = true; + DRM_DEBUG_KMS(Finished loading %s\n, dev_priv-csr.fw_path); KMS? DRM_DEBUG_DRIVER() as it is off wider scope. Is it? AFAIK the DMC is all about saving/restoring display registers when we toggle the power wells between frame updates for eDP + PSR panels. But then I guess the loading itself is a generic driver function. Well I merged both patches. Someone still owes me the conversion to refcount away from the hand-rolled completion though ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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 v6 2/8] drm/i915: Use cached cdclk value
On 06/15/2015 01:14 PM, Damien Lespiau wrote: On Mon, Jun 15, 2015 at 01:54:40PM +0200, Daniel Vetter wrote: On Wed, Jun 03, 2015 at 03:45:08PM +0300, Mika Kahola wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Rather than reading out the current cdclk value use the cached value we have tucked away in dev_priv. v2: Rebased to the latest v3: Rebased to the latest v4: Fix for patch style problems Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Mika Kahola mika.kah...@intel.com This patch needs to be extended to also cover the recently added skl_max_scale. Tvrtko has recently written a patch to add some checks to that code too, would be good to resurrect that too. Chandra can help with any questions wrt the skl scaler code. Not quite I'm afraid. The CDCLK used in skl_max_scale() has to be part of the atomic state, even bumping CDCLK if possible/needed. If you use the cached cdclk in skl_max_scale(), it won't do the right thing when CDCLK is off (ie cached frew is the fallback 24Mhz ref clock) and we try to do the first modeset before waking up the display. I filed a bug about it already to track it: https://bugs.freedesktop.org/show_bug.cgi?id=90874 I know nothing about these specific clocks, but FWIW, my patch was only about enabling new platforms - making skl_max_scale more robust in cases where clock querying does not yet work correctly. My reasoning was based on a comment from Ville that one of those two clocks must never be lower than the other. So it sounded reasonable to ignore such cases ie. assume no scaling is possible and allow a normal (unscaled) modeset to succeed rather than fail it and display nothing. 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: Add runtime PM's usage_count in i915_runtime_pm_status
On Thu, Jun 04, 2015 at 06:23:58PM +0100, Damien Lespiau wrote: Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index d8bd4e1..92cf273 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2493,6 +2493,8 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused) seq_printf(m, GPU idle: %s\n, yesno(!dev_priv-mm.busy)); seq_printf(m, IRQs disabled: %s\n, yesno(!intel_irqs_enabled(dev_priv))); + seq_printf(m, Usage count: %d\n, +atomic_read(dev-dev-power.usage_count)); Needs a check for CONFIG_PM. Since I pulled in what Jani merged as a merge commit, can you please do a follow-up patch? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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: Avoid fluctuating to UNKNOWN connector status during forced probes
On Mon, Jun 15, 2015 at 01:35:21PM +0100, Chris Wilson wrote: On Mon, Jun 15, 2015 at 02:29:28PM +0200, Daniel Vetter wrote: On Fri, Jun 05, 2015 at 08:46:19AM +0100, Chris Wilson wrote: For old-school component TV and CRT connectors, we require a heavyweight load-detection mechanism. This involves setting up a CRTC and sending a signal to the output, before reading back any response. As that is quite slow and CPU heavy, the process is only performed when the output detection is forced by user request. As it requires a driving CRTC, we often don't have the resources to complete the probe. This leaves us in a quandary where the unforced path just returns the old connector status, but the forced detection path elects to return UNKNOWN. If we have an active connection, we likely have the resources available to complete the probe - but if it is currently disconnected, then it becomes unknown and triggers a hotplug event, with often quite unfortunate userspace behaviour (e.g. one output is blanked and the spurious TV turned on). To reduce spurious hotplug events on older devices, we can prevent transitions between disconnected - unknown. v2: Convert tv_type to use proper unknown enum (Ville) References: https://bugs.freedesktop.org/show_bug.cgi?id=87049 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrj...@linux.intel.com [v1] Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com [v1] This solution is at odds with commit b7703726251191cd9f3ef3a80b2d9667901eec95 Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Wed Jan 21 08:45:22 2015 +0100 drm/probe-helper: clamp unknown connector status in the poll work We're missing this bit of logic from the hotplug handlers, but that was somewhat intentional since a hotplug should indicate that something has changed. And in the i915 hpd handler we filter by source. Given that the report is older than me having resurrect that patch can we close it as fixed or do I miss something? It's a different path. This also concerns the actual forced reprobe fluctuating between two states. In that case I still think it should be in the probe helpers. Which raises the policy decision whether we should ever hand unkown back to userspace since apparently it just can't cope sensible with it. But that call should be done consistently accross drivers. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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 i-g-t] Android.mk: Disable tools that do not build for android
-Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Monday, June 15, 2015 2:43 PM To: Morton, Derek J Cc: Wood, Thomas; Intel Graphics Development Subject: Re: [Intel-gfx] [PATCH i-g-t] Android.mk: Disable tools that do not build for android On Mon, Jun 15, 2015 at 01:19:16PM +, Morton, Derek J wrote: -Original Message- From: Thomas Wood [mailto:thomas.w...@intel.com] Sent: Monday, June 15, 2015 2:09 PM To: Morton, Derek J Cc: Intel Graphics Development; Gore, Tim Subject: Re: [PATCH i-g-t] Android.mk: Disable tools that do not build for android On 15 June 2015 at 14:04, Derek Morton derek.j.mor...@intel.com wrote: Disable the tools / demo code that do not currently build for android until they can be fixed. What needs to be fixed? Intel_reg - Relies on a header file not provided by the Android compiler Intel_display_crc - Cairo dependency Cairo should be available by now on android, at least on some builds. The test at least go to great pains to build if cairo is around. Imo we should adopt it. Cairo is not present by default on the Gmin build. There is a CAIRO_DEPENDANT flag for building tests. I will update the patch so the tool gets built conditionally with the same flag. Intel_sprite_on - An API difference in the Android i915 driver. This tool is only around for the benefit of the android display folks. If you dont need it then it's time to remove it. -Daniel My aim is to get the vanilla freedesktop code to build on android. Currently intel_sprit_on requires a local patch to build so needs to be disabled by default until that is resolved. //Derek -- Daniel Vetter Software Engineer, Intel Corporation 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 our labels start at column 0
On Fri, Jun 05, 2015 at 12:27:21PM +0300, Ville Syrjälä wrote: On Fri, Jun 05, 2015 at 12:24:45PM +0300, Jani Nikula wrote: On Thu, 04 Jun 2015, Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Thu, Jun 04, 2015 at 04:56:18PM +0100, Damien Lespiau wrote: I noticed one of those and it turned out we have a few lingering around. Yuck. I'd prefer we got the other way. Consider the following diffs for example: What's the, uh, diff between those to consider? Look at the @@ line. One tells you in which function the line is added, the other one doesn't. It always pisses me off when reviewing patches cause then I have to figure out the function based on the label, surroundng context, and/or line numbers. Yeah that's an annoying sucker but I guess just part of the fail. Imo consistency wins this bikeshed ;-) I'm also thinking this may have caused some of the numerous misapplied patches we've had since our labels all tend to be similar. Diff doesn't look at the heading after the @@ but only at concept. And when applying with some mismatches that can end up in really surprising places. Chaning how we place labels won't help. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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 2/2] drm/i915: Add runtime PM's usage_count in i915_runtime_pm_status
On Mon, Jun 15, 2015 at 02:44:05PM +0200, Daniel Vetter wrote: On Thu, Jun 04, 2015 at 06:23:58PM +0100, Damien Lespiau wrote: Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index d8bd4e1..92cf273 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2493,6 +2493,8 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused) seq_printf(m, GPU idle: %s\n, yesno(!dev_priv-mm.busy)); seq_printf(m, IRQs disabled: %s\n, yesno(!intel_irqs_enabled(dev_priv))); + seq_printf(m, Usage count: %d\n, + atomic_read(dev-dev-power.usage_count)); Needs a check for CONFIG_PM. Since I pulled in what Jani merged as a merge commit, can you please do a follow-up patch? Ooops indeed, Chris just sent one. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
