* [PATCH 01/11] drm/i915: Bring UP Power Wells before disabling RC6.
2014-07-14 12:18 [PATCH 00/11] drm-intel-collector - update Rodrigo Vivi
@ 2014-07-14 12:18 ` Rodrigo Vivi
2014-07-14 12:18 ` [PATCH 02/11] drm/i915: Don't save/restore RS when not used Rodrigo Vivi
` (9 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2014-07-14 12:18 UTC (permalink / raw)
To: intel-gfx; +Cc: Deepak S, Paulo Zanoni, Rodrigo Vivi
From: Deepak S <deepak.s@intel.com>
We need do forcewake before Disabling RC6, This is what the BIOS
expects while going into suspend.
v2: updated commit message. (Daniel)
Reviewer: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Deepak S <deepak.s@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 780c3ab..4fb8917 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3398,8 +3398,14 @@ static void valleyview_disable_rps(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ /* we're doing forcewake before Disabling RC6,
+ * This what the BIOS expects when going into suspend */
+ gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
+
I915_WRITE(GEN6_RC_CONTROL, 0);
+ gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
+
gen6_disable_rps_interrupts(dev);
}
--
1.9.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 02/11] drm/i915: Don't save/restore RS when not used
2014-07-14 12:18 [PATCH 00/11] drm-intel-collector - update Rodrigo Vivi
2014-07-14 12:18 ` [PATCH 01/11] drm/i915: Bring UP Power Wells before disabling RC6 Rodrigo Vivi
@ 2014-07-14 12:18 ` Rodrigo Vivi
2014-07-14 12:18 ` [PATCH 03/11] drm/i915: Upgrade execbuffer fail after resume failure to EIO Rodrigo Vivi
` (8 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2014-07-14 12:18 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi, Ben Widawsky, Ben Widawsky
From: Ben Widawsky <benjamin.widawsky@intel.com>
v2: fix conflict on rebase.
Cc: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_gem_context.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index de72a28..49ae7ab 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -565,6 +565,7 @@ mi_set_context(struct intel_engine_cs *ring,
struct intel_context *new_context,
u32 hw_flags)
{
+ u32 flags = hw_flags | MI_MM_SPACE_GTT;
int ret;
/* w/a: If Flush TLB Invalidation Mode is enabled, driver must do a TLB
@@ -578,6 +579,10 @@ mi_set_context(struct intel_engine_cs *ring,
return ret;
}
+ /* These flags are for resource streamer on HSW+ */
+ if (!IS_HASWELL(ring->dev) && INTEL_INFO(ring->dev)->gen < 8)
+ flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN);
+
ret = intel_ring_begin(ring, 6);
if (ret)
return ret;
@@ -591,10 +596,7 @@ mi_set_context(struct intel_engine_cs *ring,
intel_ring_emit(ring, MI_NOOP);
intel_ring_emit(ring, MI_SET_CONTEXT);
intel_ring_emit(ring, i915_gem_obj_ggtt_offset(new_context->legacy_hw_ctx.rcs_state) |
- MI_MM_SPACE_GTT |
- MI_SAVE_EXT_STATE_EN |
- MI_RESTORE_EXT_STATE_EN |
- hw_flags);
+ flags);
/*
* w/a: MI_SET_CONTEXT must always be followed by MI_NOOP
* WaMiSetContext_Hang:snb,ivb,vlv
--
1.9.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 03/11] drm/i915: Upgrade execbuffer fail after resume failure to EIO
2014-07-14 12:18 [PATCH 00/11] drm-intel-collector - update Rodrigo Vivi
2014-07-14 12:18 ` [PATCH 01/11] drm/i915: Bring UP Power Wells before disabling RC6 Rodrigo Vivi
2014-07-14 12:18 ` [PATCH 02/11] drm/i915: Don't save/restore RS when not used Rodrigo Vivi
@ 2014-07-14 12:18 ` Rodrigo Vivi
2014-07-14 12:18 ` [PATCH 04/11] drm/i915: Add property to set HDMI aspect ratio Rodrigo Vivi
` (7 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2014-07-14 12:18 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Chris Wilson <chris@chris-wilson.co.uk>
If we try to execute on a known ring, but it has failed to be
initialised correctly, report that the GPU is hung rather than the
command invalid. This leaves us reporting EINVAL only if the user
requests execution on a ring that is not supported by the device.
This should prevent UXA from getting stuck in a null render loop after a
failed resume.
v2 (Rodrigo): Fix conflict and add VCS2 ring and
s/intel_ring_buffer/intel_engine_cs.
Reported-by: Jiri Kosina <jikos@jikos.cz>
References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 60998fc..288ff61 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1233,6 +1233,19 @@ eb_get_batch(struct eb_vmas *eb)
return vma->obj;
}
+static bool
+intel_ring_valid(struct intel_engine_cs *ring)
+{
+ switch (ring->id) {
+ case RCS: return true;
+ case VCS: return HAS_BSD(ring->dev);
+ case BCS: return HAS_BLT(ring->dev);
+ case VECS: return HAS_VEBOX(ring->dev);
+ case VCS2: return HAS_BSD2(ring->dev);
+ default: return false;
+ }
+}
+
static int
i915_gem_do_execbuffer(struct drm_device *dev, void *data,
struct drm_file *file,
@@ -1289,7 +1302,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
if (!intel_ring_initialized(ring)) {
DRM_DEBUG("execbuf with invalid ring: %d\n",
(int)(args->flags & I915_EXEC_RING_MASK));
- return -EINVAL;
+ return intel_ring_valid(ring) ? -EIO : -EINVAL;
}
if (args->buffer_count < 1) {
--
1.9.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 04/11] drm/i915: Add property to set HDMI aspect ratio
2014-07-14 12:18 [PATCH 00/11] drm-intel-collector - update Rodrigo Vivi
` (2 preceding siblings ...)
2014-07-14 12:18 ` [PATCH 03/11] drm/i915: Upgrade execbuffer fail after resume failure to EIO Rodrigo Vivi
@ 2014-07-14 12:18 ` Rodrigo Vivi
2014-07-14 12:18 ` [PATCH 05/11] drm/i915: honour forced connector modes Rodrigo Vivi
` (6 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2014-07-14 12:18 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter, Jesse Barnes, Rodrigo Vivi
From: Vandana Kannan <vandana.kannan@intel.com>
Added a property to enable user space to set aspect ratio for HDMI displays.
If there is no user specified value, then PAR_NONE/Automatic option is set
by default. User can select aspect ratio 4:3 or 16:9. The aspect ratio
selected by user would come into effect with a mode set.
v2: Daniel's review comments incorporated.
Call for a mode set to update property.
Reviewer: Ville Syrjälä <ville.syrjala@linux.intel.com>
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Cc: Jesse Barnes <jesse.barnes@intel.com>
Cc: Vijay Purushothaman <vijay.a.purushothaman@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_hdmi.c | 12 ++++++++++++
drivers/gpu/drm/i915/intel_modes.c | 28 ++++++++++++++++++++++++++++
4 files changed, 43 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 47c8ec1..804ea1b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1562,6 +1562,7 @@ struct drm_i915_private {
struct drm_property *broadcast_rgb_property;
struct drm_property *force_audio_property;
+ struct drm_property *aspect_ratio_property;
uint32_t hw_context_size;
struct list_head context_list;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 016d894..bee9e53 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -498,6 +498,7 @@ struct intel_hdmi {
bool has_audio;
enum hdmi_force_audio force_audio;
bool rgb_quant_range_selectable;
+ enum hdmi_picture_aspect aspect_ratio;
void (*write_infoframe)(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len);
@@ -931,6 +932,7 @@ int intel_connector_update_modes(struct drm_connector *connector,
int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
void intel_attach_force_audio_property(struct drm_connector *connector);
void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
+void intel_attach_aspect_ratio_property(struct drm_connector *connector);
/* intel_overlay.c */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 2422413..1851284 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -367,6 +367,9 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
union hdmi_infoframe frame;
int ret;
+ /* Set user selected PAR to incoming mode's member */
+ adjusted_mode->picture_aspect_ratio = intel_hdmi->aspect_ratio;
+
ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
adjusted_mode);
if (ret < 0) {
@@ -1124,6 +1127,11 @@ intel_hdmi_set_property(struct drm_connector *connector,
goto done;
}
+ if (property == dev_priv->aspect_ratio_property) {
+ intel_hdmi->aspect_ratio = val;
+ goto done;
+ }
+
return -EINVAL;
done:
@@ -1484,6 +1492,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
{
intel_attach_force_audio_property(connector);
intel_attach_broadcast_rgb_property(connector);
+ intel_attach_aspect_ratio_property(connector);
intel_hdmi->color_range_auto = true;
}
@@ -1551,6 +1560,9 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
intel_connector->get_hw_state = intel_connector_get_hw_state;
intel_connector->unregister = intel_connector_unregister;
+ /* Initialize aspect ratio member of intel_hdmi */
+ intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+
intel_hdmi_add_properties(intel_hdmi, connector);
intel_connector_attach_encoder(intel_connector, intel_encoder);
diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
index 0e860f3..6f814da 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -126,3 +126,31 @@ intel_attach_broadcast_rgb_property(struct drm_connector *connector)
drm_object_attach_property(&connector->base, prop, 0);
}
+
+static const struct drm_prop_enum_list aspect_ratio_names[] = {
+ { HDMI_PICTURE_ASPECT_NONE, "Automatic" },
+ { HDMI_PICTURE_ASPECT_4_3, "4:3" },
+ { HDMI_PICTURE_ASPECT_16_9, "16:9" },
+};
+
+void
+intel_attach_aspect_ratio_property(struct drm_connector *connector)
+{
+ struct drm_device *dev = connector->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_property *prop;
+
+ prop = dev_priv->aspect_ratio_property;
+ if (prop == NULL) {
+ prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM,
+ "HDMI aspect ratio",
+ aspect_ratio_names,
+ ARRAY_SIZE(aspect_ratio_names));
+ if (prop == NULL)
+ return;
+
+ dev_priv->aspect_ratio_property = prop;
+ }
+
+ drm_object_attach_property(&connector->base, prop, 0);
+}
--
1.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 05/11] drm/i915: honour forced connector modes
2014-07-14 12:18 [PATCH 00/11] drm-intel-collector - update Rodrigo Vivi
` (3 preceding siblings ...)
2014-07-14 12:18 ` [PATCH 04/11] drm/i915: Add property to set HDMI aspect ratio Rodrigo Vivi
@ 2014-07-14 12:18 ` Rodrigo Vivi
2014-07-14 12:18 ` [PATCH 06/11] drm/i915: Don't promote UC to WT automagically Rodrigo Vivi
` (5 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2014-07-14 12:18 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Chris Wilson <chris@chris-wilson.co.uk>
In the move over to use BIOS connector configs, we lost the ability to
force a specific set of connectors on or off. Try to remedy that by
dropping back to the old behavior if we detect a hard coded connector
config that tries to enable a connector (disabling is easy!).
Based on earlier patches by Jesse Barnes.
v2: Remove Jesse's patch
Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/intel_fbdev.c | 33 ++++++++++++---------------------
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index f475414..5d879d18 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -331,24 +331,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
int num_connectors_enabled = 0;
int num_connectors_detected = 0;
- /*
- * If the user specified any force options, just bail here
- * and use that config.
- */
- for (i = 0; i < fb_helper->connector_count; i++) {
- struct drm_fb_helper_connector *fb_conn;
- struct drm_connector *connector;
-
- fb_conn = fb_helper->connector_info[i];
- connector = fb_conn->connector;
-
- if (!enabled[i])
- continue;
-
- if (connector->force != DRM_FORCE_UNSPECIFIED)
- return false;
- }
-
save_enabled = kcalloc(dev->mode_config.num_connector, sizeof(bool),
GFP_KERNEL);
if (!save_enabled)
@@ -374,8 +356,18 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
continue;
}
+ if (connector->force == DRM_FORCE_OFF) {
+ DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n",
+ connector->name);
+ enabled[i] = false;
+ continue;
+ }
+
encoder = connector->encoder;
if (!encoder || WARN_ON(!encoder->crtc)) {
+ if (connector->force > DRM_FORCE_OFF)
+ goto bail;
+
DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n",
connector->name);
enabled[i] = false;
@@ -394,8 +386,7 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
for (j = 0; j < fb_helper->connector_count; j++) {
if (crtcs[j] == new_crtc) {
DRM_DEBUG_KMS("fallback: cloned configuration\n");
- fallback = true;
- goto out;
+ goto bail;
}
}
@@ -466,8 +457,8 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
fallback = true;
}
-out:
if (fallback) {
+bail:
DRM_DEBUG_KMS("Not using firmware configuration\n");
memcpy(enabled, save_enabled, dev->mode_config.num_connector);
kfree(save_enabled);
--
1.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 06/11] drm/i915: Don't promote UC to WT automagically
2014-07-14 12:18 [PATCH 00/11] drm-intel-collector - update Rodrigo Vivi
` (4 preceding siblings ...)
2014-07-14 12:18 ` [PATCH 05/11] drm/i915: honour forced connector modes Rodrigo Vivi
@ 2014-07-14 12:18 ` Rodrigo Vivi
2014-07-14 12:18 ` [PATCH 07/11] drm/i915: Refactor the physical and virtual page hws setup Rodrigo Vivi
` (4 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2014-07-14 12:18 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
If the object is already UC leave it as UC instead of automagically
promoting it to WT in i915_gem_object_pin_to_display_plane() when
the hardware is WT capable.
Supposedly the user wanted UC for a reason, so let's respect that.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e5d4d73..c3e7e8f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3840,6 +3840,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
struct intel_engine_cs *pipelined)
{
u32 old_read_domains, old_write_domain;
+ unsigned int cache_level;
bool was_pin_display;
int ret;
@@ -3864,8 +3865,12 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
* of uncaching, which would allow us to flush all the LLC-cached data
* with that bit in the PTE to main memory with just one PIPE_CONTROL.
*/
- ret = i915_gem_object_set_cache_level(obj,
- HAS_WT(obj->base.dev) ? I915_CACHE_WT : I915_CACHE_NONE);
+ if (HAS_WT(obj->base.dev) && obj->cache_level != I915_CACHE_NONE)
+ cache_level = I915_CACHE_WT;
+ else
+ cache_level = I915_CACHE_NONE;
+
+ ret = i915_gem_object_set_cache_level(obj, cache_level);
if (ret)
goto err_unpin_display;
--
1.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 07/11] drm/i915: Refactor the physical and virtual page hws setup
2014-07-14 12:18 [PATCH 00/11] drm-intel-collector - update Rodrigo Vivi
` (5 preceding siblings ...)
2014-07-14 12:18 ` [PATCH 06/11] drm/i915: Don't promote UC to WT automagically Rodrigo Vivi
@ 2014-07-14 12:18 ` Rodrigo Vivi
2014-07-14 12:18 ` [PATCH 08/11] drm/i915: clean up PPGTT checking logic Rodrigo Vivi
` (3 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2014-07-14 12:18 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Chris Wilson <chris@chris-wilson.co.uk>
We duplicated the legacy physical HWS setup routine for no good reason.
Combine it with the more recent virtual HWS setup for simplicity.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_dma.c | 16 +------
drivers/gpu/drm/i915/intel_ringbuffer.c | 81 ++++++++++++++++-----------------
2 files changed, 39 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5ae6081..44919f8 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -104,17 +104,6 @@ void i915_update_dri1_breadcrumb(struct drm_device *dev)
}
}
-static void i915_write_hws_pga(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
- u32 addr;
-
- addr = dev_priv->status_page_dmah->busaddr;
- if (INTEL_INFO(dev)->gen >= 4)
- addr |= (dev_priv->status_page_dmah->busaddr >> 28) & 0xf0;
- I915_WRITE(HWS_PGA, addr);
-}
-
/**
* Frees the hardware status page, whether it's a physical address or a virtual
* address set up by the X Server.
@@ -255,10 +244,7 @@ static int i915_dma_resume(struct drm_device *dev)
}
DRM_DEBUG_DRIVER("hw status page @ %p\n",
ring->status_page.page_addr);
- if (ring->status_page.gfx_addr != 0)
- intel_ring_setup_status_page(ring);
- else
- i915_write_hws_pga(dev);
+ intel_ring_setup_status_page(ring);
DRM_DEBUG_DRIVER("Enabled hardware status page\n");
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 599709e..316babf 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -443,17 +443,6 @@ u64 intel_ring_get_active_head(struct intel_engine_cs *ring)
return acthd;
}
-static void ring_setup_phys_status_page(struct intel_engine_cs *ring)
-{
- struct drm_i915_private *dev_priv = ring->dev->dev_private;
- u32 addr;
-
- addr = dev_priv->status_page_dmah->busaddr;
- if (INTEL_INFO(ring->dev)->gen >= 4)
- addr |= (dev_priv->status_page_dmah->busaddr >> 28) & 0xf0;
- I915_WRITE(HWS_PGA, addr);
-}
-
static bool stop_ring(struct intel_engine_cs *ring)
{
struct drm_i915_private *dev_priv = to_i915(ring->dev);
@@ -511,10 +500,7 @@ static int init_ring_common(struct intel_engine_cs *ring)
}
}
- if (I915_NEED_GFX_HWS(dev))
- intel_ring_setup_status_page(ring);
- else
- ring_setup_phys_status_page(ring);
+ intel_ring_setup_status_page(ring);
/* Initialize the ring. This must happen _after_ we've cleared the ring
* registers with the above sequence (the readback of the HEAD registers
@@ -1101,39 +1087,48 @@ void intel_ring_setup_status_page(struct intel_engine_cs *ring)
{
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = ring->dev->dev_private;
- u32 mmio = 0;
+ u32 mmio, addr;
- /* The ring status page addresses are no longer next to the rest of
- * the ring registers as of gen7.
- */
- if (IS_GEN7(dev)) {
- switch (ring->id) {
- case RCS:
- mmio = RENDER_HWS_PGA_GEN7;
- break;
- case BCS:
- mmio = BLT_HWS_PGA_GEN7;
- break;
- /*
- * VCS2 actually doesn't exist on Gen7. Only shut up
- * gcc switch check warning
+ if (!I915_NEED_GFX_HWS(dev)) {
+ addr = dev_priv->status_page_dmah->busaddr;
+ if (INTEL_INFO(ring->dev)->gen >= 4)
+ addr |= (dev_priv->status_page_dmah->busaddr >> 28) & 0xf0;
+ mmio = HWS_PGA;
+ } else {
+ addr = ring->status_page.gfx_addr;
+ /* The ring status page addresses are no longer next to the rest of
+ * the ring registers as of gen7.
*/
- case VCS2:
- case VCS:
- mmio = BSD_HWS_PGA_GEN7;
- break;
- case VECS:
- mmio = VEBOX_HWS_PGA_GEN7;
- break;
+ if (IS_GEN7(dev)) {
+ switch (ring->id) {
+ default:
+ case RCS:
+ mmio = RENDER_HWS_PGA_GEN7;
+ break;
+ case BCS:
+ mmio = BLT_HWS_PGA_GEN7;
+ break;
+ /*
+ * VCS2 actually doesn't exist on Gen7. Only shut up
+ * gcc switch check warning
+ */
+ case VCS2:
+ case VCS:
+ mmio = BSD_HWS_PGA_GEN7;
+ break;
+ case VECS:
+ mmio = VEBOX_HWS_PGA_GEN7;
+ break;
+ }
+ } else if (IS_GEN6(ring->dev)) {
+ mmio = RING_HWS_PGA_GEN6(ring->mmio_base);
+ } else {
+ /* XXX: gen8 returns to sanity */
+ mmio = RING_HWS_PGA(ring->mmio_base);
}
- } else if (IS_GEN6(ring->dev)) {
- mmio = RING_HWS_PGA_GEN6(ring->mmio_base);
- } else {
- /* XXX: gen8 returns to sanity */
- mmio = RING_HWS_PGA(ring->mmio_base);
}
- I915_WRITE(mmio, (u32)ring->status_page.gfx_addr);
+ I915_WRITE(mmio, addr);
POSTING_READ(mmio);
/*
--
1.9.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 08/11] drm/i915: clean up PPGTT checking logic
2014-07-14 12:18 [PATCH 00/11] drm-intel-collector - update Rodrigo Vivi
` (6 preceding siblings ...)
2014-07-14 12:18 ` [PATCH 07/11] drm/i915: Refactor the physical and virtual page hws setup Rodrigo Vivi
@ 2014-07-14 12:18 ` Rodrigo Vivi
2014-07-14 12:18 ` [PATCH 09/11] drm/i915: re-order ppgtt sanitize logic v2 Rodrigo Vivi
` (2 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2014-07-14 12:18 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Jesse Barnes <jbarnes@virtuousgeek.org>
sanitize_enable_ppgtt is the function that checks all the conditions,
honoring a forced ppgtt status or doing auto-detect as necessary. Just
make sure it returns the right value in all cases and use that in the
macros instead of the confusing intel_enable_ppgtt() function.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 4 ++--
drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++-----------
drivers/gpu/drm/i915/i915_gem_gtt.h | 1 -
3 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 804ea1b..428c374 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2032,8 +2032,8 @@ struct drm_i915_cmd_table {
#define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6)
#define HAS_ALIASING_PPGTT(dev) (INTEL_INFO(dev)->gen >= 6)
#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && !IS_GEN8(dev))
-#define USES_PPGTT(dev) intel_enable_ppgtt(dev, false)
-#define USES_FULL_PPGTT(dev) intel_enable_ppgtt(dev, true)
+#define USES_PPGTT(dev) (i915.enable_ppgtt)
+#define USES_FULL_PPGTT(dev) (i915.enable_ppgtt == 2)
#define HAS_OVERLAY(dev) (INTEL_INFO(dev)->has_overlay)
#define OVERLAY_NEEDS_PHYSICAL(dev) (INTEL_INFO(dev)->overlay_needs_physical)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a4153ee..ea08de7 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -33,17 +33,6 @@
static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv);
static void chv_setup_private_ppat(struct drm_i915_private *dev_priv);
-bool intel_enable_ppgtt(struct drm_device *dev, bool full)
-{
- if (i915.enable_ppgtt == 0)
- return false;
-
- if (i915.enable_ppgtt == 1 && full)
- return false;
-
- return true;
-}
-
static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
{
if (enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
@@ -69,6 +58,9 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
return 0;
}
+ if (HAS_PPGTT(dev))
+ return 2;
+
return HAS_ALIASING_PPGTT(dev) ? 1 : 0;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 8d6f7c1..666c938 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -272,7 +272,6 @@ void i915_gem_init_global_gtt(struct drm_device *dev);
void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start,
unsigned long mappable_end, unsigned long end);
-bool intel_enable_ppgtt(struct drm_device *dev, bool full);
int i915_gem_init_ppgtt(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt);
void i915_check_and_clear_faults(struct drm_device *dev);
--
1.9.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 09/11] drm/i915: re-order ppgtt sanitize logic v2
2014-07-14 12:18 [PATCH 00/11] drm-intel-collector - update Rodrigo Vivi
` (7 preceding siblings ...)
2014-07-14 12:18 ` [PATCH 08/11] drm/i915: clean up PPGTT checking logic Rodrigo Vivi
@ 2014-07-14 12:18 ` Rodrigo Vivi
2014-07-14 12:18 ` [PATCH 10/11] drm/i915: Bring GPU Freq to min while suspending Rodrigo Vivi
2014-07-14 12:18 ` [PATCH 11/11] drm/i915/bdw: Map unused PDPs to a scratch page Rodrigo Vivi
10 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2014-07-14 12:18 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Jesse Barnes <jbarnes@virtuousgeek.org>
Put hw limitations first, disabling ppgtt if necessary right away.
After that, check user passed args or auto-detect and do the right
thing, falling back to aliasing PPGTT if the user tries to enable full
PPGTT but it isn't available.
v2: simplify auto-detect case since we already caught the no PPGTT case early
on (Jesse)
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ea08de7..743512e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -35,15 +35,6 @@ static void chv_setup_private_ppat(struct drm_i915_private *dev_priv);
static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
{
- if (enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
- return 0;
-
- if (enable_ppgtt == 1)
- return 1;
-
- if (enable_ppgtt == 2 && HAS_PPGTT(dev))
- return 2;
-
#ifdef CONFIG_INTEL_IOMMU
/* Disable ppgtt on SNB if VT-d is on. */
if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped) {
@@ -58,10 +49,20 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
return 0;
}
- if (HAS_PPGTT(dev))
- return 2;
+ if (!HAS_ALIASING_PPGTT(dev))
+ return 0;
- return HAS_ALIASING_PPGTT(dev) ? 1 : 0;
+ /* Check user passed enable_ppgtt param and try to honor it */
+ switch (enable_ppgtt) {
+ case 0:
+ return 0;
+ case 1:
+ return 1; /* caught any hw limits above */
+ case 2:
+ /* fall through to auto-detect */
+ default: /* auto-detect */
+ return HAS_PPGTT(dev) ? 2 : 1;
+ }
}
--
1.9.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 10/11] drm/i915: Bring GPU Freq to min while suspending.
2014-07-14 12:18 [PATCH 00/11] drm-intel-collector - update Rodrigo Vivi
` (8 preceding siblings ...)
2014-07-14 12:18 ` [PATCH 09/11] drm/i915: re-order ppgtt sanitize logic v2 Rodrigo Vivi
@ 2014-07-14 12:18 ` Rodrigo Vivi
2014-07-14 12:18 ` [PATCH 11/11] drm/i915/bdw: Map unused PDPs to a scratch page Rodrigo Vivi
10 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2014-07-14 12:18 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
From: Deepak S <deepak.s@linux.intel.com>
We might be leaving the PGU Frequency (and thus vnn) high during the suspend.
Flusing the delayed work queue should take care of this.
Signed-off-by: Deepak S <deepak.s@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c3e7e8f..aafb382 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4559,7 +4559,7 @@ i915_gem_suspend(struct drm_device *dev)
del_timer_sync(&dev_priv->gpu_error.hangcheck_timer);
cancel_delayed_work_sync(&dev_priv->mm.retire_work);
- cancel_delayed_work_sync(&dev_priv->mm.idle_work);
+ flush_delayed_work(&dev_priv->mm.idle_work);
return 0;
--
1.9.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 11/11] drm/i915/bdw: Map unused PDPs to a scratch page
2014-07-14 12:18 [PATCH 00/11] drm-intel-collector - update Rodrigo Vivi
` (9 preceding siblings ...)
2014-07-14 12:18 ` [PATCH 10/11] drm/i915: Bring GPU Freq to min while suspending Rodrigo Vivi
@ 2014-07-14 12:18 ` Rodrigo Vivi
2014-07-16 8:33 ` Ben Widawsky
10 siblings, 1 reply; 13+ messages in thread
From: Rodrigo Vivi @ 2014-07-14 12:18 UTC (permalink / raw)
To: intel-gfx; +Cc: Dave Gordon, Rodrigo Vivi
From: Bob Beckett <robert.beckett@intel.com>
Create a scratch page for the two unused PDPs and set all the PTEs
for them to point to it.
This patch addresses a page fault, and subsequent hang in pipe
control flush. In these cases, the Main Graphic Arbiter Error
register [0x40A0] showed a TLB Page Fault error, and a high memory
address (higher than the size of our PPGTT) was reported in the
Fault TLB RD Data0 register (0x4B10).
PDP2 & PDP3 were not set because, in theory, they aren't required
for our PPGTT size, but they should be mapped to a scratch page
anyway.
v2: Rebase on latest nightly.
Signed-off-by: Michel Thierry <michel.thierry@intel.com> (v1)
Signed-off-by: Dave Gordon <david.s.gordon@intel.com> (v2)
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 79 +++++++++++++++++++++++++++++--------
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +
2 files changed, 65 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 743512e..7743e4f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -364,6 +364,11 @@ static void gen8_ppgtt_free(const struct i915_hw_ppgtt *ppgtt)
kfree(ppgtt->gen8_pt_dma_addr[i]);
}
+ /* Unused PDPs are always assigned to scratch page */
+ for (i = ppgtt->num_pd_pages; i < GEN8_LEGACY_PDPS; i++)
+ kfree(ppgtt->gen8_pt_dma_addr[i]);
+ __free_page(ppgtt->scratch_page);
+
__free_pages(ppgtt->pd_pages, get_order(ppgtt->num_pd_pages << PAGE_SHIFT));
}
@@ -388,6 +393,13 @@ static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt *ppgtt)
PCI_DMA_BIDIRECTIONAL);
}
}
+
+ /* Unused PDPs are always assigned to scratch page */
+ for (i = ppgtt->num_pd_pages; i < GEN8_LEGACY_PDPS; i++) {
+ if (ppgtt->pd_dma_addr[i])
+ pci_unmap_page(hwdev, ppgtt->pd_dma_addr[i],
+ PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
+ }
}
static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
@@ -474,10 +486,21 @@ static int gen8_ppgtt_allocate_dma(struct i915_hw_ppgtt *ppgtt)
static int gen8_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt,
const int max_pdp)
{
- ppgtt->pd_pages = alloc_pages(GFP_KERNEL, get_order(max_pdp << PAGE_SHIFT));
- if (!ppgtt->pd_pages)
+ /* Scratch page for unmapped PDP's */
+ ppgtt->scratch_page = alloc_page(GFP_KERNEL);
+ if (!ppgtt->scratch_page)
return -ENOMEM;
+ /* Must allocate space for all 4 PDPs. HW has implemented cache which
+ * pre-fetches entries; that pre-fetch can attempt access for entries
+ * even if no resources are located in that range.
+ */
+ ppgtt->pd_pages = alloc_pages(GFP_KERNEL, GEN8_LEGACY_PDPS);
+ if (!ppgtt->pd_pages) {
+ __free_page(ppgtt->scratch_page);
+ return -ENOMEM;
+ }
+
ppgtt->num_pd_pages = 1 << get_order(max_pdp << PAGE_SHIFT);
BUG_ON(ppgtt->num_pd_pages > GEN8_LEGACY_PDPS);
@@ -495,6 +518,7 @@ static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt,
ret = gen8_ppgtt_allocate_page_tables(ppgtt, max_pdp);
if (ret) {
+ __free_page(ppgtt->scratch_page);
__free_pages(ppgtt->pd_pages, get_order(max_pdp << PAGE_SHIFT));
return ret;
}
@@ -529,18 +553,25 @@ static int gen8_ppgtt_setup_page_directories(struct i915_hw_ppgtt *ppgtt,
static int gen8_ppgtt_setup_page_tables(struct i915_hw_ppgtt *ppgtt,
const int pd,
- const int pt)
+ const int pt,
+ const int max_pdp)
{
dma_addr_t pt_addr;
struct page *p;
int ret;
- p = ppgtt->gen8_pt_pages[pd][pt];
- pt_addr = pci_map_page(ppgtt->base.dev->pdev,
- p, 0, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
- ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pt_addr);
- if (ret)
- return ret;
+ /* Unused PDPs need to have their ptes pointing to the
+ * existing scratch page.
+ */
+ if (pd < max_pdp) {
+ p = ppgtt->gen8_pt_pages[pd][pt];
+ pt_addr = pci_map_page(ppgtt->base.dev->pdev,
+ p, 0, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
+ ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pt_addr);
+ if (ret)
+ return ret;
+ } else
+ pt_addr = ppgtt->scratch_dma_addr;
ppgtt->gen8_pt_dma_addr[pd][pt] = pt_addr;
@@ -562,6 +593,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
const int max_pdp = DIV_ROUND_UP(size, 1 << 30);
const int min_pt_pages = GEN8_PDES_PER_PAGE * max_pdp;
int i, j, ret;
+ gen8_gtt_pte_t *pt_vaddr, scratch_pte;
if (size % (1<<30))
DRM_INFO("Pages will be wasted unless GTT size (%llu) is divisible by 1GB\n", size);
@@ -571,30 +603,38 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
if (ret)
return ret;
- /*
- * 2. Create DMA mappings for the page directories and page tables.
- */
- for (i = 0; i < max_pdp; i++) {
+ /* 2. Map the scratch page */
+ ppgtt->scratch_dma_addr =
+ pci_map_page(ppgtt->base.dev->pdev,
+ ppgtt->scratch_page, 0, PAGE_SIZE,
+ PCI_DMA_BIDIRECTIONAL);
+
+ ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, ppgtt->scratch_dma_addr);
+ if (ret)
+ goto bail;
+
+ /* 3. Create DMA mappings for the page directories and page tables. */
+ for (i = 0; i < GEN8_LEGACY_PDPS; i++) {
ret = gen8_ppgtt_setup_page_directories(ppgtt, i);
if (ret)
goto bail;
for (j = 0; j < GEN8_PDES_PER_PAGE; j++) {
- ret = gen8_ppgtt_setup_page_tables(ppgtt, i, j);
+ ret = gen8_ppgtt_setup_page_tables(ppgtt, i, j, max_pdp);
if (ret)
goto bail;
}
}
/*
- * 3. Map all the page directory entires to point to the page tables
+ * 4. Map all the page directory entries to point to the page tables
* we've allocated.
*
* For now, the PPGTT helper functions all require that the PDEs are
* plugged in correctly. So we do that now/here. For aliasing PPGTT, we
* will never need to touch the PDEs again.
*/
- for (i = 0; i < max_pdp; i++) {
+ for (i = 0; i < GEN8_LEGACY_PDPS; i++) {
gen8_ppgtt_pde_t *pd_vaddr;
pd_vaddr = kmap_atomic(&ppgtt->pd_pages[i]);
for (j = 0; j < GEN8_PDES_PER_PAGE; j++) {
@@ -617,6 +657,13 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
ppgtt->base.clear_range(&ppgtt->base, 0, ppgtt->base.total, true);
+ scratch_pte = gen8_pte_encode(ppgtt->base.scratch.addr,
+ I915_CACHE_LLC, true);
+ pt_vaddr = kmap_atomic(ppgtt->scratch_page);
+ for (i = 0; i < GEN8_PTES_PER_PAGE; i++)
+ pt_vaddr[i] = scratch_pte;
+ kunmap_atomic(pt_vaddr);
+
DRM_DEBUG_DRIVER("Allocated %d pages for page directories (%d wasted)\n",
ppgtt->num_pd_pages, ppgtt->num_pd_pages - max_pdp);
DRM_DEBUG_DRIVER("Allocated %d pages for page tables (%lld wasted)\n",
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 666c938..02032b3 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -249,6 +249,7 @@ struct i915_hw_ppgtt {
struct page **gen8_pt_pages[GEN8_LEGACY_PDPS];
};
struct page *pd_pages;
+ struct page *scratch_page;
union {
uint32_t pd_offset;
dma_addr_t pd_dma_addr[GEN8_LEGACY_PDPS];
@@ -258,6 +259,7 @@ struct i915_hw_ppgtt {
dma_addr_t *gen8_pt_dma_addr[4];
};
+ dma_addr_t scratch_dma_addr;
struct intel_context *ctx;
int (*enable)(struct i915_hw_ppgtt *ppgtt);
--
1.9.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 11/11] drm/i915/bdw: Map unused PDPs to a scratch page
2014-07-14 12:18 ` [PATCH 11/11] drm/i915/bdw: Map unused PDPs to a scratch page Rodrigo Vivi
@ 2014-07-16 8:33 ` Ben Widawsky
0 siblings, 0 replies; 13+ messages in thread
From: Ben Widawsky @ 2014-07-16 8:33 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: Dave Gordon, intel-gfx
On Mon, 14 Jul 2014 05:18:44 -0700
Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> From: Bob Beckett <robert.beckett@intel.com>
>
> Create a scratch page for the two unused PDPs and set all the PTEs
> for them to point to it.
>
> This patch addresses a page fault, and subsequent hang in pipe
> control flush. In these cases, the Main Graphic Arbiter Error
> register [0x40A0] showed a TLB Page Fault error, and a high memory
> address (higher than the size of our PPGTT) was reported in the
> Fault TLB RD Data0 register (0x4B10).
>
> PDP2 & PDP3 were not set because, in theory, they aren't required
> for our PPGTT size, but they should be mapped to a scratch page
> anyway.
>
> v2: Rebase on latest nightly.
>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com> (v1)
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com> (v2)
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Just as an FYI, this patch is definitely needed (I confirmed with
design), and the same "fix" was in my 64b PPGTT series.
One thing which appears different with my fix, though I didn't check
closely, is I allocate an extra page per PDP. I can't remember exactly
why I did it, but I may have put it in the commit message.
I'd like to see this fix get merged (and I'd prefer we just merge 64b
series, but since that's unlikely...). CC'ing James who should pick
this up.
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 79
> +++++++++++++++++++++++++++++--------
> drivers/gpu/drm/i915/i915_gem_gtt.h | 2 + 2 files changed, 65
> insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c index 743512e..7743e4f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -364,6 +364,11 @@ static void gen8_ppgtt_free(const struct
> i915_hw_ppgtt *ppgtt) kfree(ppgtt->gen8_pt_dma_addr[i]);
> }
>
> + /* Unused PDPs are always assigned to scratch page */
> + for (i = ppgtt->num_pd_pages; i < GEN8_LEGACY_PDPS; i++)
> + kfree(ppgtt->gen8_pt_dma_addr[i]);
> + __free_page(ppgtt->scratch_page);
> +
> __free_pages(ppgtt->pd_pages, get_order(ppgtt->num_pd_pages
> << PAGE_SHIFT)); }
>
> @@ -388,6 +393,13 @@ static void gen8_ppgtt_unmap_pages(struct
> i915_hw_ppgtt *ppgtt) PCI_DMA_BIDIRECTIONAL);
> }
> }
> +
> + /* Unused PDPs are always assigned to scratch page */
> + for (i = ppgtt->num_pd_pages; i < GEN8_LEGACY_PDPS; i++) {
> + if (ppgtt->pd_dma_addr[i])
> + pci_unmap_page(hwdev, ppgtt->pd_dma_addr[i],
> + PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
> + }
> }
>
> static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
> @@ -474,10 +486,21 @@ static int gen8_ppgtt_allocate_dma(struct
> i915_hw_ppgtt *ppgtt) static int
> gen8_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt,
> const int max_pdp) {
> - ppgtt->pd_pages = alloc_pages(GFP_KERNEL, get_order(max_pdp
> << PAGE_SHIFT));
> - if (!ppgtt->pd_pages)
> + /* Scratch page for unmapped PDP's */
> + ppgtt->scratch_page = alloc_page(GFP_KERNEL);
> + if (!ppgtt->scratch_page)
> return -ENOMEM;
>
> + /* Must allocate space for all 4 PDPs. HW has implemented
> cache which
> + * pre-fetches entries; that pre-fetch can attempt access
> for entries
> + * even if no resources are located in that range.
> + */
> + ppgtt->pd_pages = alloc_pages(GFP_KERNEL, GEN8_LEGACY_PDPS);
> + if (!ppgtt->pd_pages) {
> + __free_page(ppgtt->scratch_page);
> + return -ENOMEM;
> + }
> +
> ppgtt->num_pd_pages = 1 << get_order(max_pdp << PAGE_SHIFT);
> BUG_ON(ppgtt->num_pd_pages > GEN8_LEGACY_PDPS);
>
> @@ -495,6 +518,7 @@ static int gen8_ppgtt_alloc(struct i915_hw_ppgtt
> *ppgtt,
> ret = gen8_ppgtt_allocate_page_tables(ppgtt, max_pdp);
> if (ret) {
> + __free_page(ppgtt->scratch_page);
> __free_pages(ppgtt->pd_pages, get_order(max_pdp <<
> PAGE_SHIFT)); return ret;
> }
> @@ -529,18 +553,25 @@ static int
> gen8_ppgtt_setup_page_directories(struct i915_hw_ppgtt *ppgtt,
> static int gen8_ppgtt_setup_page_tables(struct i915_hw_ppgtt *ppgtt,
> const int pd,
> - const int pt)
> + const int pt,
> + const int max_pdp)
> {
> dma_addr_t pt_addr;
> struct page *p;
> int ret;
>
> - p = ppgtt->gen8_pt_pages[pd][pt];
> - pt_addr = pci_map_page(ppgtt->base.dev->pdev,
> - p, 0, PAGE_SIZE,
> PCI_DMA_BIDIRECTIONAL);
> - ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pt_addr);
> - if (ret)
> - return ret;
> + /* Unused PDPs need to have their ptes pointing to the
> + * existing scratch page.
> + */
> + if (pd < max_pdp) {
> + p = ppgtt->gen8_pt_pages[pd][pt];
> + pt_addr = pci_map_page(ppgtt->base.dev->pdev,
> + p, 0, PAGE_SIZE,
> PCI_DMA_BIDIRECTIONAL);
> + ret = pci_dma_mapping_error(ppgtt->base.dev->pdev,
> pt_addr);
> + if (ret)
> + return ret;
> + } else
> + pt_addr = ppgtt->scratch_dma_addr;
>
> ppgtt->gen8_pt_dma_addr[pd][pt] = pt_addr;
>
> @@ -562,6 +593,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt
> *ppgtt, uint64_t size) const int max_pdp = DIV_ROUND_UP(size, 1 <<
> 30); const int min_pt_pages = GEN8_PDES_PER_PAGE * max_pdp;
> int i, j, ret;
> + gen8_gtt_pte_t *pt_vaddr, scratch_pte;
>
> if (size % (1<<30))
> DRM_INFO("Pages will be wasted unless GTT size
> (%llu) is divisible by 1GB\n", size); @@ -571,30 +603,38 @@ static
> int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size) if
> (ret) return ret;
>
> - /*
> - * 2. Create DMA mappings for the page directories and page
> tables.
> - */
> - for (i = 0; i < max_pdp; i++) {
> + /* 2. Map the scratch page */
> + ppgtt->scratch_dma_addr =
> + pci_map_page(ppgtt->base.dev->pdev,
> + ppgtt->scratch_page, 0, PAGE_SIZE,
> + PCI_DMA_BIDIRECTIONAL);
> +
> + ret = pci_dma_mapping_error(ppgtt->base.dev->pdev,
> ppgtt->scratch_dma_addr);
> + if (ret)
> + goto bail;
> +
> + /* 3. Create DMA mappings for the page directories and page
> tables. */
> + for (i = 0; i < GEN8_LEGACY_PDPS; i++) {
> ret = gen8_ppgtt_setup_page_directories(ppgtt, i);
> if (ret)
> goto bail;
>
> for (j = 0; j < GEN8_PDES_PER_PAGE; j++) {
> - ret = gen8_ppgtt_setup_page_tables(ppgtt, i,
> j);
> + ret = gen8_ppgtt_setup_page_tables(ppgtt, i,
> j, max_pdp); if (ret)
> goto bail;
> }
> }
>
> /*
> - * 3. Map all the page directory entires to point to the
> page tables
> + * 4. Map all the page directory entries to point to the
> page tables
> * we've allocated.
> *
> * For now, the PPGTT helper functions all require that the
> PDEs are
> * plugged in correctly. So we do that now/here. For
> aliasing PPGTT, we
> * will never need to touch the PDEs again.
> */
> - for (i = 0; i < max_pdp; i++) {
> + for (i = 0; i < GEN8_LEGACY_PDPS; i++) {
> gen8_ppgtt_pde_t *pd_vaddr;
> pd_vaddr = kmap_atomic(&ppgtt->pd_pages[i]);
> for (j = 0; j < GEN8_PDES_PER_PAGE; j++) {
> @@ -617,6 +657,13 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt
> *ppgtt, uint64_t size)
> ppgtt->base.clear_range(&ppgtt->base, 0, ppgtt->base.total,
> true);
> + scratch_pte = gen8_pte_encode(ppgtt->base.scratch.addr,
> + I915_CACHE_LLC, true);
> + pt_vaddr = kmap_atomic(ppgtt->scratch_page);
> + for (i = 0; i < GEN8_PTES_PER_PAGE; i++)
> + pt_vaddr[i] = scratch_pte;
> + kunmap_atomic(pt_vaddr);
> +
> DRM_DEBUG_DRIVER("Allocated %d pages for page directories
> (%d wasted)\n", ppgtt->num_pd_pages, ppgtt->num_pd_pages - max_pdp);
> DRM_DEBUG_DRIVER("Allocated %d pages for page tables (%lld
> wasted)\n", diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
> b/drivers/gpu/drm/i915/i915_gem_gtt.h index 666c938..02032b3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -249,6 +249,7 @@ struct i915_hw_ppgtt {
> struct page **gen8_pt_pages[GEN8_LEGACY_PDPS];
> };
> struct page *pd_pages;
> + struct page *scratch_page;
> union {
> uint32_t pd_offset;
> dma_addr_t pd_dma_addr[GEN8_LEGACY_PDPS];
> @@ -258,6 +259,7 @@ struct i915_hw_ppgtt {
> dma_addr_t *gen8_pt_dma_addr[4];
> };
>
> + dma_addr_t scratch_dma_addr;
> struct intel_context *ctx;
>
> int (*enable)(struct i915_hw_ppgtt *ppgtt);
^ permalink raw reply [flat|nested] 13+ messages in thread