Re: [Intel-gfx] screen update problems with Intel HD 4600 + virtual screen
On Mon, Jun 23, 2014 at 09:49:07AM +0200, Krzysztof Halasa wrote: khal...@piap.pl (Krzysztof Hałasa) writes: I'm having screen update problems problems with an Intel HD 4600 with panning + virtual screen. Fedora 20 + updates, CPU is Core i7 4770K, I'm using xrandr --output HDMI1 --panning 4096x2404. The physical screen size is 1920x1200. Another setup also experiencing this problem is Core i5 4200M with a 1600x900 physical screen (LVDS) and (a bit larger) virtual screen (again with panning). This happens with both UXA and SNA. ATM I'm now using SNA as UXA(?) had some probably unrelated problems with Xvideo (non)updates. Screen 0: minimum 320 x 200, current 4096 x 2404, maximum 32767 x 32767 VGA1 disconnected (normal left inverted right x axis y axis) HDMI1 connected 4096x2404+0+0 (normal left inverted right x axis y axis) 518mm x 324mm panning 4096x2404+0+0 1920x1200 59.95*+ [etc.] DP1 disconnected (normal left inverted right x axis y axis) HDMI2 disconnected (normal left inverted right x axis y axis) HDMI3 disconnected (normal left inverted right x axis y axis) It looks with SNA, all is good as long as the screen isn't moved (panned) down more than 7 pixels (i.e. the screen y offset must be 0 - 7 and x offset doesn't matter): switch to mode 1920x1200@60.0 on pipe 0 using HDMI1, position (2176, 0), rotation normal switch to mode 1920x1200@60.0 on pipe 0 using HDMI1, position (2176, 1), rotation normal switch to mode 1920x1200@60.0 on pipe 0 using HDMI1, position (2176, 2), rotation normal ... switch to mode 1920x1200@60.0 on pipe 0 using HDMI1, position (2176, 6), rotation normal switch to mode 1920x1200@60.0 on pipe 0 using HDMI1, position (2176, 7), rotation normal Now I scroll down 1 pixel: switch to mode 1920x1200@60.0 on pipe 0 using HDMI1, position (2176, 8), rotation normal and immediately screen isn't updated correctly. For example, an application window is created normally, but when I move it (the app window) down, the top part of the window, max 8 pixels, is left on the screen (the moved window is ok). It almost looks like the code somewhere adds the vertical screen offset twice. 8 is significant as it is the tile row height. The kernel tries to be smart and extract the panning from the surface address so that we can display a larger virtual framebuffer than the hardware can actually use. Can you reproduce the about sequence with drm.debug=6 dmesg (i.e. echo 6 /sys/module/drm/parameters/debug ; xrandr --panning... ; dmesg)? -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 1/3] drm/i915: Added a return type for panel fitter config functions
On Fri, 2014-06-20 at 15:03 +0100, Chris Wilson wrote:
On Fri, Jun 20, 2014 at 07:25:52PM +0530, akash.g...@intel.com wrote:
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index 912e9c4..6117639 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -824,12 +824,15 @@ intel_dp_compute_config(struct intel_encoder *encoder,
if (is_edp(intel_dp) intel_connector-panel.fixed_mode) {
intel_fixed_panel_mode(intel_connector-panel.fixed_mode,
adjusted_mode);
- if (!HAS_PCH_SPLIT(dev))
- intel_gmch_panel_fitting(intel_crtc, pipe_config,
-
intel_connector-panel.fitting_mode);
- else
- intel_pch_panel_fitting(intel_crtc, pipe_config,
-
intel_connector-panel.fitting_mode);
+ if (!HAS_PCH_SPLIT(dev)) {
+ if (!intel_gmch_panel_fitting(intel_crtc, pipe_config,
+
intel_connector-panel.fitting_mode))
+ return false;
+ } else {
+ if (!intel_pch_panel_fitting(intel_crtc, pipe_config,
+
intel_connector-panel.fitting_mode))
+ return false;
+ }
}
if (adjusted_mode-flags DRM_MODE_FLAG_DBLCLK)
@@ -3782,7 +3785,7 @@ intel_dp_set_property(struct drm_connector *connector,
done:
if (intel_encoder-base.crtc)
- intel_crtc_restore_mode(intel_encoder-base.crtc);
+ return intel_crtc_restore_mode(intel_encoder-base.crtc);
But you don't unwind the property change after failure.
Sorry, will handle this in next version of the patch.
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index ab5962b..860d531 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -727,7 +727,7 @@ void intel_mark_busy(struct drm_device *dev);
void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
struct intel_engine_cs *ring);
void intel_mark_idle(struct drm_device *dev);
-void intel_crtc_restore_mode(struct drm_crtc *crtc);
+int intel_crtc_restore_mode(struct drm_crtc *crtc);
void intel_crtc_update_dpms(struct drm_crtc *crtc);
void intel_encoder_destroy(struct drm_encoder *encoder);
void intel_connector_dpms(struct drm_connector *, int mode);
@@ -918,10 +918,10 @@ int intel_panel_init(struct intel_panel *panel,
void intel_panel_fini(struct intel_panel *panel);
void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
struct drm_display_mode *adjusted_mode);
-void intel_pch_panel_fitting(struct intel_crtc *crtc,
+bool intel_pch_panel_fitting(struct intel_crtc *crtc,
struct intel_crtc_config *pipe_config,
int fitting_mode);
-void intel_gmch_panel_fitting(struct intel_crtc *crtc,
+bool intel_gmch_panel_fitting(struct intel_crtc *crtc,
struct intel_crtc_config *pipe_config,
int fitting_mode);
Only make significnt changes like this to one interface at a time.
Fine will split this patch in 2 parts.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] New drm crtc properties
On Fri, 2014-06-20 at 15:07 +0100, Damien Lespiau wrote: On Fri, Jun 20, 2014 at 07:25:51PM +0530, akash.g...@intel.com wrote: From: Akash Goel akash.g...@intel.com Added new drm crtc properties which provides control to vary the size of horizontal vertical borders. With this the size of the Panel fitter output or display window can be controlled. Also added a return type to panel fitter config functions, so that an error could be returned to User space for an invalid configuration. Akash Goel (3): drm/i915: Added a return type for panel fitter config functions drm/i915: Initialized 'set_property' fn pointer field of intel_crtc_funcs structure drm/i915: New drm crtc property for varying the size of borders As an aside, you're missing the new property documentation in Documentation/DocBook/drm.tmpl. Sorry, will float the corresponding Documentation patch in the next series. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm/i915: New drm crtc property for varying the size of borders
From: Akash Goel akash.g...@intel.com
This patch adds a new drm crtc property for varying the size of
the horizontal vertical borers of the output/display window.
This will control the output of Panel fitter.
There are actually 4 separate properties so as to allow a
control on the size of each border left/top/bottom/right
Testcase: igt/kms_panel_fitter_test
Signed-off-by: Akash Goel akash.g...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 10 ++
drivers/gpu/drm/i915/intel_display.c | 118 ++-
drivers/gpu/drm/i915/intel_drv.h | 12 ++
drivers/gpu/drm/i915/intel_panel.c | 219 ---
4 files changed, 344 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8cea596..9ec9823 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1530,6 +1530,16 @@ struct drm_i915_private {
struct drm_property *broadcast_rgb_property;
struct drm_property *force_audio_property;
+ /*
+* Properties to dynamically vary the size of the
+* borders. This will indirectly control the size
+* of the display window i.e Panel fitter output
+*/
+ struct drm_property *left_border_property;
+ struct drm_property *right_border_property;
+ struct drm_property *top_border_property;
+ struct drm_property *bottom_border_property;
+
uint32_t hw_context_size;
struct list_head context_list;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8f60513..9ea76e3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11250,12 +11250,124 @@ out_config:
return ret;
}
+static void intel_create_crtc_properties(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);
+
+ /* Create properties*/
+ if (!dev_priv-left_border_property)
+ dev_priv-left_border_property =
+ drm_property_create_range(dev, 0, left border,
+ 0, (uint64_t)MAX_BORDER_VALUE);
+
+ if (!dev_priv-right_border_property)
+ dev_priv-right_border_property =
+ drm_property_create_range(dev, 0, right border,
+ 0, (uint64_t)MAX_BORDER_VALUE);
+
+ if (!dev_priv-top_border_property)
+ dev_priv-top_border_property =
+ drm_property_create_range(dev, 0, top border,
+ 0, (uint64_t)MAX_BORDER_VALUE);
+
+ if (!dev_priv-bottom_border_property)
+ dev_priv-bottom_border_property =
+ drm_property_create_range(dev, 0, bottom border,
+ 0, (uint64_t)MAX_BORDER_VALUE);
+
+ /* Attach to properties*/
+ if (dev_priv-left_border_property)
+ drm_object_attach_property(intel_crtc-base.base,
+ dev_priv-left_border_property,
+ 0);
+
+ if (dev_priv-right_border_property)
+ drm_object_attach_property(intel_crtc-base.base,
+ dev_priv-right_border_property,
+ 0);
+
+ if (dev_priv-top_border_property)
+ drm_object_attach_property(intel_crtc-base.base,
+ dev_priv-top_border_property,
+ 0);
+
+ if (dev_priv-bottom_border_property)
+ drm_object_attach_property(intel_crtc-base.base,
+ dev_priv-bottom_border_property,
+ 0);
+}
+
static int intel_crtc_set_property(struct drm_crtc *crtc,
struct drm_property *property, uint64_t val)
{
- int ret = -ENOENT;
+ struct drm_device *dev = crtc-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- return ret;
+ if (property == dev_priv-left_border_property) {
+ if (val == (uint64_t)intel_crtc-border[PANEL_BORDER_LEFT])
+ return 0;
+ if (val 1) {
+ DRM_ERROR(Odd border value not supported\n);
+ return -EINVAL;
+ }
+
+ intel_crtc-border[PANEL_BORDER_LEFT] = (uint32_t)val;
+ goto done;
+ }
+
+ if (property == dev_priv-right_border_property) {
+ if (val == (uint64_t)intel_crtc-border[PANEL_BORDER_RIGHT])
+ return 0;
+ if (val 1) {
+
[Intel-gfx] [PATCH 4/5] Documentation/drm: Describing border size property
From: Akash Goel akash.g...@intel.com
Updated drm documentation to include description of
border size property
Signed-off-by: Akash Goel akash.g...@intel.com
---
Documentation/DocBook/drm.tmpl | 36 ++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index b314a42..2237fa78 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2648,8 +2648,8 @@ void intel_crt_init(struct drm_device *dev)
td valign=top TBD/td
/tr
tr
- td rowspan=21 valign=top i915/td
- td rowspan=3 valign=top Generic/td
+ td rowspan=25 valign=top i915/td
+ td rowspan=7 valign=top Generic/td
td valign=top Broadcast RGB/td
td valign=top ENUM/td
td valign=top { Automatic, Full, Limited 16:235 }/td
@@ -2664,6 +2664,38 @@ void intel_crt_init(struct drm_device *dev)
td valign=top TBD/td
/tr
tr
+ td valign=top “left border”/td
+ td valign=top RANGE/td
+ td valign=top Min=0, Max=256/td
+ td valign=top Crtc/td
+ td valign=top DRM CRTC property for varying the size of
+ the horizontal, vertical borders of the output/display
+ window.There are actually 4 separate properties so as
+ to allow an individual control on the size of each border
+ left/top/bottom/right/td
+ /tr
+ tr
+ td valign=top “right border”/td
+ td valign=top RANGE/td
+ td valign=top Min=0, Max=256/td
+ td valign=top Crtc/td
+ td valign=top same as above/td
+ /tr
+ tr
+ td valign=top “top border”/td
+ td valign=top RANGE/td
+ td valign=top Min=0, Max=256/td
+ td valign=top Crtc/td
+ td valign=top same as above/td
+ /tr
+ tr
+ td valign=top “bottom border”/td
+ td valign=top RANGE/td
+ td valign=top Min=0, Max=256/td
+ td valign=top Crtc/td
+ td valign=top same as above/td
+ /tr
+ tr
td valign=top Standard name as in DRM/td
td valign=top Standard type as in DRM/td
td valign=top Standard value as in DRM/td
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/5] drm/i915: Initialized 'set_property' fn pointer field of intel_crtc_funcs structure
From: Akash Goel akash.g...@intel.com
This patch defines a new function assigns that to the 'set_property'
function pointer field of the 'intel_crtc_funcs' structure.
Signed-off-by: Akash Goel akash.g...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index f39a538..8f60513 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11250,11 +11250,20 @@ out_config:
return ret;
}
+static int intel_crtc_set_property(struct drm_crtc *crtc,
+ struct drm_property *property, uint64_t val)
+{
+ int ret = -ENOENT;
+
+ return ret;
+}
+
static const struct drm_crtc_funcs intel_crtc_funcs = {
.gamma_set = intel_crtc_gamma_set,
.set_config = intel_crtc_set_config,
.destroy = intel_crtc_destroy,
.page_flip = intel_crtc_page_flip,
+ .set_property = intel_crtc_set_property,
};
static void intel_cpu_pll_init(struct drm_device *dev)
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/5] drm/i915: Added a return type for the restore crtc mode function
From: Akash Goel akash.g...@intel.com
This patch changes the return type of 'crtc_restore_mode'
function from 'void', so that an error could be returned back to
User space, from the set property ioctl call, if the configuation
is not valid.
Signed-off-by: Akash Goel akash.g...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 5 ++--
drivers/gpu/drm/i915/intel_dp.c | 30 +++
drivers/gpu/drm/i915/intel_drv.h | 2 +-
drivers/gpu/drm/i915/intel_hdmi.c| 28 +++---
drivers/gpu/drm/i915/intel_lvds.c| 7 -
drivers/gpu/drm/i915/intel_sdvo.c| 57 +---
drivers/gpu/drm/i915/intel_tv.c | 26 ++--
7 files changed, 136 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index fa77557..f39a538 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10817,9 +10817,10 @@ static int intel_set_mode(struct drm_crtc *crtc,
return ret;
}
-void intel_crtc_restore_mode(struct drm_crtc *crtc)
+int intel_crtc_restore_mode(struct drm_crtc *crtc)
{
- intel_set_mode(crtc, crtc-mode, crtc-x, crtc-y, crtc-primary-fb);
+ return intel_set_mode(crtc, crtc-mode, crtc-x, crtc-y,
+ crtc-primary-fb);
}
#undef for_each_intel_crtc_masked
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3e04e15..e0186d0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3680,6 +3680,9 @@ intel_dp_set_property(struct drm_connector *connector,
struct intel_encoder *intel_encoder = intel_attached_encoder(connector);
struct intel_dp *intel_dp = enc_to_intel_dp(intel_encoder-base);
int ret;
+ bool old_has_audio = 0, old_auto = 0;
+ uint32_t old_range = 0;
+ int old_force_audio = 0, old_fitting_mode = 0;
ret = drm_object_property_set_value(connector-base, property, val);
if (ret)
@@ -3692,6 +3695,9 @@ intel_dp_set_property(struct drm_connector *connector,
if (i == intel_dp-force_audio)
return 0;
+ old_force_audio = intel_dp-force_audio;
+ old_has_audio = intel_dp-has_audio;
+
intel_dp-force_audio = i;
if (i == HDMI_AUDIO_AUTO)
@@ -3707,8 +3713,8 @@ intel_dp_set_property(struct drm_connector *connector,
}
if (property == dev_priv-broadcast_rgb_property) {
- bool old_auto = intel_dp-color_range_auto;
- uint32_t old_range = intel_dp-color_range;
+ old_auto = intel_dp-color_range_auto;
+ old_range = intel_dp-color_range;
switch (val) {
case INTEL_BROADCAST_RGB_AUTO:
@@ -3744,6 +3750,7 @@ intel_dp_set_property(struct drm_connector *connector,
/* the eDP scaling property is not changed */
return 0;
}
+ old_fitting_mode = intel_connector-panel.fitting_mode;
intel_connector-panel.fitting_mode = val;
goto done;
@@ -3752,9 +3759,22 @@ intel_dp_set_property(struct drm_connector *connector,
return -EINVAL;
done:
- if (intel_encoder-base.crtc)
- intel_crtc_restore_mode(intel_encoder-base.crtc);
-
+ if (intel_encoder-base.crtc) {
+ ret = intel_crtc_restore_mode(intel_encoder-base.crtc);
+ if (ret) {
+ if (property == dev_priv-force_audio_property) {
+ intel_dp-force_audio = old_force_audio;
+ intel_dp-has_audio = old_has_audio;
+ }
+ else if (property == dev_priv-broadcast_rgb_property) {
+ intel_dp-color_range_auto = old_auto;
+ intel_dp-color_range = old_range;
+ }
+ else if (property ==
connector-dev-mode_config.scaling_mode_property)
+ intel_connector-panel.fitting_mode =
old_fitting_mode;
+ }
+ return ret;
+ }
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 47d4827..074f6d8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -752,7 +752,7 @@ void intel_frontbuffer_flip(struct drm_device *dev,
void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire);
void intel_mark_idle(struct drm_device *dev);
-void intel_crtc_restore_mode(struct drm_crtc *crtc);
+int intel_crtc_restore_mode(struct drm_crtc *crtc);
void intel_crtc_update_dpms(struct drm_crtc *crtc);
void intel_encoder_destroy(struct drm_encoder *encoder);
void intel_connector_dpms(struct drm_connector *, int mode);
[Intel-gfx] [PATCH 0/5] New drm crtc properties
From: Akash Goel akash.g...@intel.com Added new drm crtc properties which provides control to vary the size of horizontal vertical borders. With this the size of the Panel fitter output or display window can be controlled. Also added a return type to panel fitter config crtc retsore mode functions, so that an error could be returned to User space for an invalid configuration. Akash Goel (5): drm/i915: Added a return type for panel fitter config functions drm/i915: Added a return type for the restore crtc mode function drm/i915: Initialized 'set_property' fn pointer field of intel_crtc_funcs structure Documentation/drm: Describing border size property drm/i915: New drm crtc property for varying the size of borders Documentation/DocBook/drm.tmpl | 36 +- drivers/gpu/drm/i915/i915_drv.h | 10 ++ drivers/gpu/drm/i915/intel_display.c | 128 +++- drivers/gpu/drm/i915/intel_dp.c | 45 +-- drivers/gpu/drm/i915/intel_drv.h | 18 ++- drivers/gpu/drm/i915/intel_hdmi.c| 28 - drivers/gpu/drm/i915/intel_lvds.c| 18 ++- drivers/gpu/drm/i915/intel_panel.c | 229 --- drivers/gpu/drm/i915/intel_sdvo.c| 57 - drivers/gpu/drm/i915/intel_tv.c | 26 +++- 10 files changed, 544 insertions(+), 51 deletions(-) -- 1.9.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Only mark the ctx as initialised after a SET_CONTEXT operation
On Fri, 30 May 2014, Chris Wilson ch...@chris-wilson.co.uk wrote:
Fallout from
commit 46470fc932ac8a0e8317a220b3f4ea4ed903338e
Author: Mika Kuoppala mika.kuopp...@linux.intel.com
Date: Wed May 21 19:01:06 2014 +0300
drm/i915: Add null state batch to active list
undid the earlier fix of only marking the ctx as initialised after it is
saved by the hardware during a SET_CONTEXT operation.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Mika Kuoppala mika.kuopp...@intel.com
Cc: Ben Widawsky b...@bwidawsk.net
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79108
---
drivers/gpu/drm/i915/i915_gem_context.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 5a71ef1975b3..34a0b49e6add 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -641,6 +641,7 @@ static int do_switch(struct intel_engine_cs *ring,
struct intel_context *from = ring-last_context;
struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(to);
u32 hw_flags = 0;
+ bool uninitialized = false;
int ret, i;
if (from != NULL ring == dev_priv-ring[RCS]) {
@@ -739,18 +740,19 @@ static int do_switch(struct intel_engine_cs *ring,
i915_gem_context_unreference(from);
}
+ uninitialized = !to-is_initialized from == NULL;
+ to-is_initialized = true;
+
done:
i915_gem_context_reference(to);
ring-last_context = to;
- if (ring-id == RCS !to-is_initialized from == NULL) {
+ if (uninitialized) {
ret = i915_gem_render_state_init(ring);
if (ret)
DRM_ERROR(init render state: %d\n, ret);
}
- to-is_initialized = true;
-
return 0;
unpin_out:
--
2.0.0.rc4
___
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] screen update problems with Intel HD 4600 + virtual screen
On Mon, Jun 23, 2014 at 11:33:52AM +0200, Krzysztof Halasa wrote: Chris Wilson ch...@chris-wilson.co.uk writes: switch to mode 1920x1200@60.0 on pipe 0 using HDMI1, position (2176, 7), rotation normal Now I scroll down 1 pixel: switch to mode 1920x1200@60.0 on pipe 0 using HDMI1, position (2176, 8), rotation normal and immediately screen isn't updated correctly. For example, an application window is created normally, but when I move it (the app window) down, the top part of the window, max 8 pixels, is left on the screen (the moved window is ok). It almost looks like the code somewhere adds the vertical screen offset twice. 8 is significant as it is the tile row height. The kernel tries to be smart and extract the panning from the surface address so that we can display a larger virtual framebuffer than the hardware can actually use. Can you reproduce the about sequence with drm.debug=6 dmesg (i.e. echo 6 /sys/module/drm/parameters/debug ; xrandr --panning... ; dmesg)? I'm panning with the mouse. Doesn't look like a lot of output: no problem yet: [617996.928] (II) intel(0): switch to mode 1920x1200@60.0 on pipe 0 using HDMI1, position (2176, 7), rotation normal [618556.836459] [drm:drm_mode_setcrtc], [CRTC:3] [618556.836472] [drm:drm_mode_setcrtc], [CONNECTOR:12:HDMI-A-1] [618556.836478] [drm:intel_crtc_set_config], [CRTC:3] [FB:55] #connectors=1 (x y) (2176 7) [618556.836484] [drm:intel_set_config_compute_mode_changes], computed changes for [CRTC:3], mode_changed=0, fb_changed=1 [618556.836489] [drm:intel_modeset_stage_output_state], [CONNECTOR:12:HDMI-A-1] to [CRTC:3] [618556.836500] [drm:ironlake_update_plane], Writing base 013CA000 D200 0 7 16384 [618556.836511] [drm:i915_setup_compression], reserved 39583744 bytes of contiguous stolen space for FBC [618556.836515] [drm:intel_update_fbc], disabling active FBC for update [618556.836520] [drm:ironlake_disable_fbc], disabled FBC [618556.836594] [drm:intel_crtc_cursor_set], cursor off [618556.885876] [drm:gen7_enable_fbc], enabled fbc on plane A [618565.988566] [drm:intel_crtc_cursor_set], cursor off problems: [618016.920] (II) intel(0): switch to mode 1920x1200@60.0 on pipe 0 using HDMI1, position (2176, 8), rotation normal [618576.846085] [drm:drm_mode_setcrtc], [CRTC:3] [618576.846094] [drm:drm_mode_setcrtc], [CONNECTOR:12:HDMI-A-1] [618576.846098] [drm:intel_crtc_set_config], [CRTC:3] [FB:55] #connectors=1 (x y) (2176 8) [618576.846103] [drm:intel_set_config_compute_mode_changes], computed changes for [CRTC:3], mode_changed=0, fb_changed=1 [618576.846106] [drm:intel_modeset_stage_output_state], [CONNECTOR:12:HDMI-A-1] to [CRTC:3] [618576.846114] [drm:ironlake_update_plane], Writing base 013CA000 1200 0 0 16384 [618576.846122] [drm:i915_setup_compression], reserved 39583744 bytes of contiguous stolen space for FBC [618576.846125] [drm:intel_update_fbc], disabling active FBC for update [618576.846128] [drm:ironlake_disable_fbc], disabled FBC [618576.846181] [drm:intel_crtc_cursor_set], cursor off [618576.895986] [drm:gen7_enable_fbc], enabled fbc on plane A [618588.640637] [drm:intel_crtc_cursor_set], cursor off Hmm. Whilst it seems odd to have a negative linear offset, I have seen it work elsewhere. Could you try setting options i915 i915_enable_fbc=0 enable_fbc=0 in /etc/modprobe.conf/intel.conf and rebuilding your initramfs? -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: Unpin last_context at reset
On Wed, Jun 18, 2014 at 10:04:48PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com We're forgetting to unpin the last_context from the ggtt at GPU reset time. This leads to the vma pin_count leaking at every reset if the last context wasn't the ring default context. Further use of the same context will trigger the pin_count check in i915_gem_object_pin() and userspace will be faced with EBUSY as a result. This plaques kms_flip rather badly since it performs lots of resets, and every fd has its own default context these days. Fix the problem by properly unpinning the last context at reset. Testcase: igt/gem_ctx_exec/reset-pin-leak Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com I pushed the igt, and also kms_flip hits this easily. It looks to me that the bug has been there since the introduction of i915_gem_context_reset() in [1], so I think we may want this patch merged with cc: stable. Afterwards people can work on killing i915_gem_context_reset() if they so wish. [1] commit acce9ffa4807027965ebd948456fa8385bbee32e Author: Ben Widawsky b...@bwidawsk.net Date: Fri Dec 6 14:11:03 2013 -0800 drm/i915: Better reset handling for contexts --- drivers/gpu/drm/i915/i915_gem_context.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 3ffe308..e362c96 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -382,6 +382,9 @@ void i915_gem_context_reset(struct drm_device *dev) dctx-obj-active = 0; } + if (ring-last_context-obj i == RCS) + i915_gem_object_ggtt_unpin(ring-last_context-obj); + i915_gem_context_unreference(ring-last_context); i915_gem_context_reference(dctx); ring-last_context = dctx; -- 1.8.5.5 -- 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 v3 1/2] drm/i915: Set M2_N2 registers during mode set
For Gen 8, set M2_N2 registers on every mode set. This is required to make
sure M2_N2 registers are set during boot, resume from sleep for cross-
checking the state. The register is set only if DRRS is supported.
v2: Patch rebased
v3: Daniel's review comments
- Removed HAS_DRRS(dev) and added bool has_drrs to pipe_config to
track drrs support
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_display.c | 36
drivers/gpu/drm/i915/intel_dp.c | 16 ++--
drivers/gpu/drm/i915/intel_drv.h | 2 ++
3 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5e8e711..2759be5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3987,8 +3987,12 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
if (intel_crtc-config.has_pch_encoder)
intel_prepare_shared_dpll(intel_crtc);
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 intel_crtc-config.has_drrs)
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -4097,8 +4101,12 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
if (intel_crtc-active)
return;
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 intel_crtc-config.has_drrs)
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -4614,8 +4622,12 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
/* Set up the display plane register */
dspcntr = DISPPLANE_GAMMA_ENABLE;
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 intel_crtc-config.has_drrs)
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -4706,8 +4718,12 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
else
dspcntr |= DISPPLANE_SEL_PIPE_B;
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 intel_crtc-config.has_drrs)
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -5494,6 +5510,18 @@ static void intel_dp_set_m_n(struct intel_crtc *crtc)
intel_cpu_transcoder_set_m_n(crtc, crtc-config.dp_m_n);
}
+void intel_dp_set_m2_n2(struct intel_crtc *crtc, struct intel_link_m_n *m_n)
+{
+ struct drm_device *dev = crtc-base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ enum transcoder transcoder = crtc-config.cpu_transcoder;
+
+ I915_WRITE(PIPE_DATA_M2(transcoder), TU_SIZE(m_n-tu) | m_n-gmch_m);
+ I915_WRITE(PIPE_DATA_N2(transcoder), m_n-gmch_n);
+ I915_WRITE(PIPE_LINK_M2(transcoder), m_n-link_m);
+ I915_WRITE(PIPE_LINK_N2(transcoder), m_n-link_n);
+}
+
static void vlv_update_pll(struct intel_crtc *crtc)
{
u32 dpll, dpll_md;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 912e9c4..963ca49 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -780,20 +780,6 @@ intel_dp_set_clock(struct intel_encoder *encoder,
}
}
-static void
-intel_dp_set_m2_n2(struct intel_crtc *crtc, struct intel_link_m_n *m_n)
-{
- struct drm_device *dev = crtc-base.dev;
- struct drm_i915_private *dev_priv = dev-dev_private;
- enum transcoder transcoder = crtc-config.cpu_transcoder;
-
- I915_WRITE(PIPE_DATA_M2(transcoder),
- TU_SIZE(m_n-tu) | m_n-gmch_m);
- I915_WRITE(PIPE_DATA_N2(transcoder), m_n-gmch_n);
- I915_WRITE(PIPE_LINK_M2(transcoder), m_n-link_m);
- I915_WRITE(PIPE_LINK_N2(transcoder), m_n-link_n);
-}
-
bool
intel_dp_compute_config(struct intel_encoder *encoder,
struct intel_crtc_config *pipe_config)
@@ -819,6 +805,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
pipe_config-has_pch_encoder = true;
pipe_config-has_dp_encoder = true;
+ pipe_config-has_drrs = false;
Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not reliable for valleyview platform.
-Original Message- From: Jani Nikula [mailto:jani.nik...@linux.intel.com] Sent: Tuesday, June 17, 2014 11:38 PM To: Wang, Quanxian Cc: intel-gfx@lists.freedesktop.org; Daniel Vetter Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not reliable for valleyview platform. On Tue, 17 Jun 2014, Wang, Quanxian quanxian.w...@intel.com wrote: File dmesg_normal_20140617.log will contain dmesg log when boot the machine and start weston. (Previous is overwrite, but it is enough for graphics boot message) File dmesg_error_20140617.log contains dmesg log after Weston exit when it found no connector available. I disable log for hotplug event from valleyview_irq_handler. There are so many. Maybe you can find some private debug log. Don't need to care that. Per DP spec we need to read the SINK_COUNT. Perhaps your problem is the hotplug irqs? [Wang, Quanxian] Hi, Jani The log event is about the transaction event instead of hotplug event. It is general since they will use I2c to read byte one by one. It will generate many transaction. But the root cause is really about hotplug(intel_hpd_irq_handler). When we switch to console, there will be a hotplug event happens after a while. After that, the system will continually get the hotplug event to switch sink device on and off periodly. With DisplayPort 1.2 spec, '' This bit is used for either monitor hotplug/unplug or for notification of a sink event., I am not sure if it is notification of a sink event or real hotplug event. We have no code to identify the difference between hotplug/unplug and notification of a sink event. I check display port spec and also not found how to identify them differently. Question is: In function intel_dp_detect_dpcd, before checking SINK_COUNT, we will use intel_dp_get_dpcd to get the dpcd. Could we think there is an active sink device attached to branch device if dpcd content is not null. According to the display port spec, only sink device has dpcd, if we get one, there must be a sink device attached to branch device or source device. Right? BR, Jani. Thanks Regards Quanxian Wang -Original Message- From: Wang, Quanxian Sent: Tuesday, June 17, 2014 10:14 AM To: 'Jani Nikula' Cc: intel-gfx@lists.freedesktop.org; Daniel Vetter Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not reliable for valleyview platform. -Original Message- From: Jani Nikula [mailto:jani.nik...@linux.intel.com] Sent: Monday, June 16, 2014 4:18 PM To: Wang, Quanxian; Daniel Vetter Cc: intel-gfx@lists.freedesktop.org Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not reliable for valleyview platform. On Mon, 16 Jun 2014, Wang, Quanxian quanxian.w...@intel.com wrote: -Original Message- From: Jani Nikula [mailto:jani.nik...@linux.intel.com] Sent: Friday, June 13, 2014 5:12 PM To: Daniel Vetter; Wang, Quanxian Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not reliable for valleyview platform. On Fri, 13 Jun 2014, Daniel Vetter dan...@ffwll.ch wrote: On Fri, Jun 13, 2014 at 02:52:04PM +0800, Quanxian Wang wrote: DP connector will be disconnected after chvt to another console for 10 minutes or more on valleyview platform VTC1010. This needs _much_ more detail, really. Also it smells like we work around a sink issue, which means the correct quirk is to use some sink id (like OUI), _not_ the platform. Since this way you break all DP1.1+ stuff on vlv and if someone puts this panel onto a different platform it still doesn't work. Furthermore you should end up in this code path *only* if you have a DP branch device. This shouldn't happen for eDP or native DP displays. Please confirm what kind of setup you're experiencing issues with. Frankly I wouldn't be surpised if we do have issues with branch devices, but this is not the fix. [Wang, Quanxian] Any idea how to do it? Currently in VTC1010 device, we use native DP to connect HDMI monitor.(DP2HDMI) This case will happen. So it's an active adapter? [Wang, Quanxian] yes. Please send full dmesg from early booth with drm.debug=0xe module parameter set, exhibiting the problem. [Wang, Quanxian] I will send the dmesg log soon. If open drm.debug=0xe, irq log will overwrite all the dmesg output. I will have some change to get the complete log for you. Just wait for a while. After checking with hardware spec, I have some comment for registers of Display Port In i915_reg.h, I found we use PCB_DP_x(address 0xe4100+??, control, data...) to do the communication and check what the SINK_COUNT. (I found it was defined in Ivybridge spec 2012) The process focus on South Display Engine to do the communication. But in valleyview
[Intel-gfx] [PATCH v7 2/2] drm/i915: State readout and cross-checking for dp_m2_n2
Adding relevant read out comparison code, in check_crtc_state, for the new
member of crtc_config, dp_m2_n2, which was introduced to store link_m_n
values for a DP downclock mode (if available). Suggested by Daniel.
v2: Changed patch title.
Daniel's review comments incorporated.
Added relevant state readout code for M2_N2. dp_m2_n2 comparison to be done
only when high RR is not in use (This is because alternate m_n register
programming will be done only when low RR is being used).
v3: Modified call to get_m2_n2 which had dp_m_n as param by mistake.
Compare dp_m_n and dp_m2_n2 for gen 7 and below. compare the structures
based on DRRS state for gen 8 and above.
Save and restore M2 N2 registers for gen 7 and below
v4: For Gen=8, check M_N registers against dp_m_n and dp_m2_n2 as there is
only one set of M_N registers
v5: Removed the chunk which saves and restores M2_N2 registers. Modified
get_m_n() to get M2_N2 registers as well. Modified the macro which compares
hw.dp_m_n against sw.dp_m2_n2/sw.dp_m_n for gen 8.
v6: Added check to compare dp_m2_n2 only when DRRS is enabled
v7: Modified drrs check to use has_drrs
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Jani Nikula jani.nik...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 70 +++-
1 file changed, 62 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2759be5..559a570 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7116,7 +7116,8 @@ static void intel_pch_transcoder_get_m_n(struct
intel_crtc *crtc,
static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc,
enum transcoder transcoder,
-struct intel_link_m_n *m_n)
+struct intel_link_m_n *m_n,
+struct intel_link_m_n *m2_n2)
{
struct drm_device *dev = crtc-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -7130,6 +7131,15 @@ static void intel_cpu_transcoder_get_m_n(struct
intel_crtc *crtc,
m_n-gmch_n = I915_READ(PIPE_DATA_N1(transcoder));
m_n-tu = ((I915_READ(PIPE_DATA_M1(transcoder))
TU_SIZE_MASK) TU_SIZE_SHIFT) + 1;
+ if (m2_n2 INTEL_INFO(dev)-gen 8) {
+ m2_n2-link_m = I915_READ(PIPE_LINK_M2(transcoder));
+ m2_n2-link_n = I915_READ(PIPE_LINK_N2(transcoder));
+ m2_n2-gmch_m = I915_READ(PIPE_DATA_M2(transcoder))
+~TU_SIZE_MASK;
+ m2_n2-gmch_n = I915_READ(PIPE_DATA_N2(transcoder));
+ m2_n2-tu = ((I915_READ(PIPE_DATA_M2(transcoder))
+TU_SIZE_MASK) TU_SIZE_SHIFT) + 1;
+ }
} else {
m_n-link_m = I915_READ(PIPE_LINK_M_G4X(pipe));
m_n-link_n = I915_READ(PIPE_LINK_N_G4X(pipe));
@@ -7148,14 +7158,15 @@ void intel_dp_get_m_n(struct intel_crtc *crtc,
intel_pch_transcoder_get_m_n(crtc, pipe_config-dp_m_n);
else
intel_cpu_transcoder_get_m_n(crtc, pipe_config-cpu_transcoder,
-pipe_config-dp_m_n);
+pipe_config-dp_m_n,
+pipe_config-dp_m2_n2);
}
static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc,
struct intel_crtc_config *pipe_config)
{
intel_cpu_transcoder_get_m_n(crtc, pipe_config-cpu_transcoder,
-pipe_config-fdi_m_n);
+pipe_config-fdi_m_n, NULL);
}
static void ironlake_get_pfit_config(struct intel_crtc *crtc,
@@ -9755,6 +9766,15 @@ static void intel_dump_pipe_config(struct intel_crtc
*crtc,
pipe_config-dp_m_n.gmch_m, pipe_config-dp_m_n.gmch_n,
pipe_config-dp_m_n.link_m, pipe_config-dp_m_n.link_n,
pipe_config-dp_m_n.tu);
+
+ DRM_DEBUG_KMS(dp: %i, gmch_m2: %u, gmch_n2: %u, link_m2: %u, link_n2:
%u, tu2: %u\n,
+ pipe_config-has_dp_encoder,
+ pipe_config-dp_m2_n2.gmch_m,
+ pipe_config-dp_m2_n2.gmch_n,
+ pipe_config-dp_m2_n2.link_m,
+ pipe_config-dp_m2_n2.link_n,
+ pipe_config-dp_m2_n2.tu);
+
DRM_DEBUG_KMS(requested mode:\n);
drm_mode_debug_printmodeline(pipe_config-requested_mode);
DRM_DEBUG_KMS(adjusted mode:\n);
@@ -10135,6 +10155,22 @@ intel_pipe_config_compare(struct drm_device *dev,
return false; \
}
+/* This is required for BDW+
Re: [Intel-gfx] screen update problems with Intel HD 4600 + virtual screen
On Mon, Jun 23, 2014 at 12:47:13PM +0200, Krzysztof Halasa wrote: Chris Wilson ch...@chris-wilson.co.uk writes: Hmm. Whilst it seems odd to have a negative linear offset, I have seen it work elsewhere. Could you try setting options i915 i915_enable_fbc=0 enable_fbc=0 in /etc/modprobe.conf/intel.conf and rebuilding your initramfs? This fixed it. The negative offset isn't exactly gone, though. Many thanks. Would you like me to try something else? I'll leave that to Daniel to try and combine FBC with CRTC viewports... -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: Restrict GPU boost to the RCS engine
Hi Chris/Daniel,
The patch is helping in some of the side-effects due to gpu boost. I still
need to get more data. I will keep the thread updated.
Thanks
Deepak
On Thursday 12 June 2014 03:02 PM, Daniel Vetter wrote:
Adding Deepak for testing, this hopefully alleviates the bad
side-effects of the gpu booster he's seeing.
-Daniel
On Thu, Jun 12, 2014 at 11:28 AM, Chris Wilson ch...@chris-wilson.co.uk wrote:
Make the assumption that media workloads are not as latency sensitive
for __wait_seqno, and that upclocking the GPU does not affect the BLT
engine. Under that assumption, we only wait to forcibly upclock the GPU
when we are stalling for results from the render pipeline.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
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 5951618a6b08..242b595a0403 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1409,7 +1409,7 @@ static int __wait_seqno(struct intel_engine_cs *ring, u32
seqno,
timeout_expire = timeout ? jiffies +
timespec_to_jiffies_timeout(timeout) : 0;
- if (INTEL_INFO(dev)-gen = 6 can_wait_boost(file_priv)) {
+ if (INTEL_INFO(dev)-gen = 6 ring-id == RCS
can_wait_boost(file_priv)) {
gen6_rps_boost(dev_priv);
if (file_priv)
mod_delayed_work(dev_priv-wq,
--
2.0.0
___
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 12/53] drm/i915/bdw: Populate LR contexts (somewhat)
-Original Message-
From: Volkin, Bradley D
Sent: Thursday, June 19, 2014 12:24 AM
To: Mateo Lozano, Oscar
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 12/53] drm/i915/bdw: Populate LR contexts
(somewhat)
On Fri, Jun 13, 2014 at 08:37:30AM -0700, oscar.ma...@intel.com wrote:
From: Oscar Mateo oscar.ma...@intel.com
For the most part, logical ring context objects are similar to
hardware contexts in that the backing object is meant to be opaque.
There are some exceptions where we need to poke certain offsets of the
object for initialization, updating the tail pointer or updating the PDPs.
For our basic execlist implementation we'll only need our PPGTT PDs,
and ringbuffer addresses in order to set up the context. With previous
patches, we have both, so start prepping the context to be load.
Before running a context for the first time you must populate some
fields in the context object. These fields begin 1 PAGE + LRCA, ie.
the first page (in 0 based counting) of the context image. These same
fields will be read and written to as contexts are saved and restored
once the system is up and running.
Many of these fields are completely reused from previous global
registers: ringbuffer head/tail/control, context control matches some
previous MI_SET_CONTEXT flags, and page directories. There are other
fields which we don't touch which we may want in the future.
v2: CTX_LRI_HEADER_0 is MI_LOAD_REGISTER_IMM(14) for render and
(11)
for other engines.
v3: Several rebases and general changes to the code.
v4: Squash with Extract LR context object populating
Also, Damien's review comments:
- Set the Force Posted bit on the LRI header, as the BSpec suggest we do.
- Prevent warning when compiling a 32-bits kernel without HIGHMEM64.
- Add a clarifying comment to the context population code.
v5: Damien's review comments:
- The third MI_LOAD_REGISTER_IMM in the context does not set Force
Posted.
- Remove dead code.
Signed-off-by: Ben Widawsky b...@bwidawsk.net (v1)
Signed-off-by: Rafael Barbalho rafael.barba...@intel.com (v2)
Signed-off-by: Oscar Mateo oscar.ma...@intel.com (v3-5)
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_lrc.c | 154
++-
2 files changed, 151 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h index 286f05c..9c8692a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -277,6 +277,7 @@
* address/value pairs. Don't overdue it, though, x = 2^4 must hold!
*/
#define MI_LOAD_REGISTER_IMM(x)MI_INSTR(0x22, 2*(x)-1)
+#define MI_LRI_FORCE_POSTED (112)
#define MI_STORE_REGISTER_MEM(x) MI_INSTR(0x24, 2*(x)-1) #define
MI_STORE_REGISTER_MEM_GEN8(x) MI_INSTR(0x24, 3*(x)-1)
#define MI_SRM_LRM_GLOBAL_GTT(122)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c
b/drivers/gpu/drm/i915/intel_lrc.c
index b3a23e0..b96bb45 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -46,6 +46,38 @@
#define GEN8_LR_CONTEXT_ALIGN 4096
+#define RING_ELSP(ring)((ring)-mmio_base+0x230)
+#define RING_CONTEXT_CONTROL(ring) ((ring)-mmio_base+0x244)
+
+#define CTX_LRI_HEADER_0 0x01
+#define CTX_CONTEXT_CONTROL0x02
+#define CTX_RING_HEAD 0x04
+#define CTX_RING_TAIL 0x06
+#define CTX_RING_BUFFER_START 0x08
+#define CTX_RING_BUFFER_CONTROL0x0a
+#define CTX_BB_HEAD_U 0x0c
+#define CTX_BB_HEAD_L 0x0e
+#define CTX_BB_STATE 0x10
+#define CTX_SECOND_BB_HEAD_U 0x12
+#define CTX_SECOND_BB_HEAD_L 0x14
+#define CTX_SECOND_BB_STATE0x16
+#define CTX_BB_PER_CTX_PTR 0x18
+#define CTX_RCS_INDIRECT_CTX 0x1a
+#define CTX_RCS_INDIRECT_CTX_OFFSET0x1c
+#define CTX_LRI_HEADER_1 0x21
+#define CTX_CTX_TIMESTAMP 0x22
+#define CTX_PDP3_UDW 0x24
+#define CTX_PDP3_LDW 0x26
+#define CTX_PDP2_UDW 0x28
+#define CTX_PDP2_LDW 0x2a
+#define CTX_PDP1_UDW 0x2c
+#define CTX_PDP1_LDW 0x2e
+#define CTX_PDP0_UDW 0x30
+#define CTX_PDP0_LDW 0x32
+#define CTX_LRI_HEADER_2 0x41
+#define CTX_R_PWR_CLK_STATE0x42
+#define CTX_GPGPU_CSR_BASE_ADDRESS 0x44
+
bool intel_enable_execlists(struct drm_device *dev) {
if (!i915.enable_execlists)
@@ -54,6 +86,110 @@ bool intel_enable_execlists(struct drm_device
*dev)
return HAS_LOGICAL_RING_CONTEXTS(dev)
Re: [Intel-gfx] [PATCH 15/53] drm/i915/bdw: Don't write PDP in the legacy way when using LRCs
- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 -Original Message- From: Volkin, Bradley D Sent: Thursday, June 19, 2014 12:43 AM To: Mateo Lozano, Oscar Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 15/53] drm/i915/bdw: Don't write PDP in the legacy way when using LRCs On Fri, Jun 13, 2014 at 08:37:33AM -0700, oscar.ma...@intel.com wrote: From: Oscar Mateo oscar.ma...@intel.com This is mostly for correctness so that we know we are running the LR context correctly (this is, the PDPs are contained inside the context object). v2: Move the check to inside the enable PPGTT function. The switch happens in two places: the legacy context switch (that we won't hit when Execlists are enabled) and the PPGTT enable, which unfortunately we need. This would look much nicer if the ppgtt-enable was part of the ring init, where it logically belongs. Signed-off-by: Oscar Mateo oscar.ma...@intel.com --- drivers/gpu/drm/i915/i915_gem_gtt.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 8b3cde7..9f0c69e 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -844,6 +844,11 @@ static int gen8_ppgtt_enable(struct i915_hw_ppgtt *ppgtt) if (USES_FULL_PPGTT(dev)) continue; + /* In the case of Execlists, we don't want to write the PDPs +* in the legacy way (they live inside the context now) */ + if (intel_enable_execlists(dev)) + return 0; Along the lines of one of Daniel's comments about the module parameter, I think we could use some clarity on when to use intel_enable_execlists() vs lrc_enabled vs i915.enable_execlists. Yep. I´ll look at this in v4. It´s probably better doing the early sanitize as Daniel suggested and then just use i915.enable_execlists everywhere. -- Oscar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: make system freeze support depend on CONFIG_ACPI_SLEEP
To achieve further power savings during system freeze (aka connected standby, or s0ix) we have to send a PCI_D1 opregion notification. As the information about the state we're entering (system freeze, suspend to ram or suspend to disk) is only available through the ACPI subsystem, make this support depend on the relevant kconfig option. Things will still work if this option isn't set, albeit with less than optimial power saving. This also fixes a compile breakage when the option is not set introduced in commit e5747e3adcd67ae27105003ec99fb58cba180105 Author: Jesse Barnes jbar...@virtuousgeek.org Date: Thu Jun 12 08:35:47 2014 -0700 drm/i915: send proper opregion notifications on suspend/resume Reported-by: Randy Dunlap rdun...@infradead.org Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 7ae4e2a..43dc8f7 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -544,10 +544,11 @@ static int i915_drm_freeze(struct drm_device *dev) i915_save_state(dev); - if (acpi_target_system_state() = ACPI_STATE_S3) - opregion_target_state = PCI_D3cold; - else + opregion_target_state = PCI_D3cold; +#if IS_ENABLED(CONFIG_ACPI_SLEEP) + if (acpi_target_system_state() ACPI_STATE_S3) opregion_target_state = PCI_D1; +#endif intel_opregion_notify_adapter(dev, opregion_target_state); intel_uncore_forcewake_reset(dev, false); -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 25/53] drm/i915/bdw: GEN-specific logical ring submit context (somewhat)
-Original Message-
From: Volkin, Bradley D
Sent: Friday, June 20, 2014 9:28 PM
To: Mateo Lozano, Oscar
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 25/53] drm/i915/bdw: GEN-specific logical
ring submit context (somewhat)
On Fri, Jun 13, 2014 at 08:37:43AM -0700, oscar.ma...@intel.com wrote:
From: Oscar Mateo oscar.ma...@intel.com
For the moment, just mark the place (we still need to do a lot of
preparation before execlists are ready to start submitting things).
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
drivers/gpu/drm/i915/intel_lrc.c| 11 +++
drivers/gpu/drm/i915/intel_ringbuffer.h | 6 ++
2 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c
b/drivers/gpu/drm/i915/intel_lrc.c
index 6c62ae5..02fc3d0 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -139,6 +139,12 @@ static void gen8_set_seqno(struct intel_engine_cs
*ring, u32 seqno)
intel_write_status_page(ring, I915_GEM_HWS_INDEX, seqno); }
+static void gen8_submit_ctx(struct intel_engine_cs *ring,
+ struct intel_context *ctx, u32 value) {
+ DRM_ERROR(Execlists still not ready!\n); }
+
void intel_logical_ring_cleanup(struct intel_engine_cs *ring) {
if (!intel_ring_initialized(ring))
@@ -213,6 +219,7 @@ static int logical_render_ring_init(struct
drm_device *dev)
ring-cleanup = intel_fini_pipe_control;
ring-get_seqno = gen8_get_seqno;
ring-set_seqno = gen8_set_seqno;
+ ring-submit_ctx = gen8_submit_ctx;
return logical_ring_init(dev, ring); } @@ -231,6 +238,7 @@ static
int logical_bsd_ring_init(struct drm_device *dev)
ring-init = gen8_init_common_ring;
ring-get_seqno = gen8_get_seqno;
ring-set_seqno = gen8_set_seqno;
+ ring-submit_ctx = gen8_submit_ctx;
return logical_ring_init(dev, ring); } @@ -249,6 +257,7 @@ static
int logical_bsd2_ring_init(struct drm_device *dev)
ring-init = gen8_init_common_ring;
ring-get_seqno = gen8_get_seqno;
ring-set_seqno = gen8_set_seqno;
+ ring-submit_ctx = gen8_submit_ctx;
return logical_ring_init(dev, ring); } @@ -267,6 +276,7 @@ static
int logical_blt_ring_init(struct drm_device *dev)
ring-init = gen8_init_common_ring;
ring-get_seqno = gen8_get_seqno;
ring-set_seqno = gen8_set_seqno;
+ ring-submit_ctx = gen8_submit_ctx;
return logical_ring_init(dev, ring); } @@ -285,6 +295,7 @@ static
int logical_vebox_ring_init(struct drm_device *dev)
ring-init = gen8_init_common_ring;
ring-get_seqno = gen8_get_seqno;
ring-set_seqno = gen8_set_seqno;
+ ring-submit_ctx = gen8_submit_ctx;
return logical_ring_init(dev, ring); } diff --git
a/drivers/gpu/drm/i915/intel_ringbuffer.h
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index ff8753c..1a6df42 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -79,6 +79,8 @@ struct intel_ringbuffer {
u32 last_retired_head;
};
+struct intel_context;
+
struct intel_engine_cs {
const char *name;
enum intel_ring_id {
@@ -146,6 +148,10 @@ struct intel_engine_cs {
unsigned int num_dwords);
} semaphore;
+ /* Execlists */
+ void(*submit_ctx)(struct intel_engine_cs *ring,
+ struct intel_context *ctx, u32 value);
+
Is it worth making this a vfunc in the refactored codebase? It ends up as the
same function for all engines...called in one place...the implementation of
which is a single call to another function that takes the same arguments.
Previously this was an implementation of the write_tail vfunc, so it made
sense. I'm not so sure now.
Now that you say it, no, it´s probably not worth it. This stuff has changed so
many times that sometimes it´s difficult to keep track :(
-- Oscar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism
-Original Message-
From: Volkin, Bradley D
Sent: Friday, June 20, 2014 10:01 PM
To: Mateo Lozano, Oscar
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring
submission mechanism
On Fri, Jun 13, 2014 at 08:37:44AM -0700, oscar.ma...@intel.com wrote:
From: Oscar Mateo oscar.ma...@intel.com
Well, new-ish: if all this code looks familiar, that's because it's a
clone of the existing submission mechanism (with some modifications
here and there to adapt it to LRCs and Execlists).
And why did we do this? Execlists offer several advantages, like
control over when the GPU is done with a given workload, that can help
simplify the submission mechanism, no doubt, but I am interested in
getting Execlists to work first and foremost. As we are creating a
parallel submission mechanism (even if itñś just a clone), we can now
start improving it without the fear of breaking old gens.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
drivers/gpu/drm/i915/intel_lrc.c | 214
+++
drivers/gpu/drm/i915/intel_lrc.h | 18
2 files changed, 232 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c
b/drivers/gpu/drm/i915/intel_lrc.c
index 02fc3d0..89aed7a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -86,6 +86,220 @@ bool intel_enable_execlists(struct drm_device
*dev)
return HAS_LOGICAL_RING_CONTEXTS(dev) USES_PPGTT(dev); }
+static inline struct intel_ringbuffer * logical_ringbuf_get(struct
+intel_engine_cs *ring, struct intel_context *ctx) {
+ return ctx-engine[ring-id].ringbuf; }
+
+void intel_logical_ring_advance_and_submit(struct intel_engine_cs *ring,
+ struct intel_context *ctx)
+{
+ struct intel_ringbuffer *ringbuf = logical_ringbuf_get(ring, ctx);
+
+ intel_logical_ring_advance(ringbuf);
+
+ if (intel_ring_stopped(ring))
+ return;
+
+ ring-submit_ctx(ring, ctx, ringbuf-tail); }
+
+static int logical_ring_alloc_seqno(struct intel_engine_cs *ring,
+ struct intel_context *ctx)
+{
+ if (ring-outstanding_lazy_seqno)
+ return 0;
+
+ if (ring-preallocated_lazy_request == NULL) {
+ struct drm_i915_gem_request *request;
+
+ request = kmalloc(sizeof(*request), GFP_KERNEL);
+ if (request == NULL)
+ return -ENOMEM;
+
+ ring-preallocated_lazy_request = request;
+ }
+
+ return i915_gem_get_seqno(ring-dev, ring-
outstanding_lazy_seqno);
+}
+
+static int logical_ring_wait_request(struct intel_engine_cs *ring,
+struct intel_ringbuffer *ringbuf,
+struct intel_context *ctx,
+int bytes)
+{
+ struct drm_i915_gem_request *request;
+ u32 seqno = 0;
+ int ret;
+
+ if (ringbuf-last_retired_head != -1) {
+ ringbuf-head = ringbuf-last_retired_head;
+ ringbuf-last_retired_head = -1;
+
+ ringbuf-space = intel_ring_space(ringbuf);
+ if (ringbuf-space = bytes)
+ return 0;
+ }
+
+ list_for_each_entry(request, ring-request_list, list) {
+ if (__intel_ring_space(request-tail, ringbuf-tail,
+ ringbuf-size) = bytes) {
+ seqno = request-seqno;
+ break;
+ }
+ }
+
+ if (seqno == 0)
+ return -ENOSPC;
+
+ ret = i915_wait_seqno(ring, seqno);
+ if (ret)
+ return ret;
+
+ /* TODO: make sure we update the right ringbuffer's
last_retired_head
+* when retiring requests */
+ i915_gem_retire_requests_ring(ring);
+ ringbuf-head = ringbuf-last_retired_head;
+ ringbuf-last_retired_head = -1;
+
+ ringbuf-space = intel_ring_space(ringbuf);
+ return 0;
+}
+
+static int logical_ring_wait_for_space(struct intel_engine_cs *ring,
+ struct intel_ringbuffer
*ringbuf,
+ struct intel_context *ctx,
+ int bytes)
+{
+ struct drm_device *dev = ring-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ unsigned long end;
+ int ret;
+
+ ret = logical_ring_wait_request(ring, ringbuf, ctx, bytes);
+ if (ret != -ENOSPC)
+ return ret;
+
+ /* Force the context submission in case we have been skipping it */
+ intel_logical_ring_advance_and_submit(ring, ctx);
+
+ /* With GEM the hangcheck timer should kick us out of the loop,
+* leaving it early runs the risk of corrupting GEM state (due
+* to running on almost untested codepaths). But on resume
+* timers
Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism
On Mon, Jun 23, 2014 at 01:09:37PM +, Mateo Lozano, Oscar wrote: So far, yes, but that´s only because I artificially made intel_lrc.c self-contained, as Daniel requested. What if we need to execute commands from somewhere else, like in intel_gen7_queue_flip()? And this takes me to another discussion: this logical ring vs legacy ring split is probably a good idea (time will tell), but we should provide a way of sending commands for execution without knowing if Execlists are enabled or not. In the early series that was easy because we reused the ring_begin, ring_emit ring_advance functions, but this is not the case anymore. And without this, sooner or later somebody will break legacy or execlists (this already happened last week, when somebody here was implementing native sync without knowing about Execlists). So, the questions is: how do you feel about a dev_priv.gt vfunc that takes a context, a ring, an array of DWORDS and a BB length and does the intel_(logical)_ring_begin/emit/advance based on i915.enable_execlists? I'm still baffled by the design. intel_ring_begin() and friends should be able to find their context (logical or legacy) from the ring and dtrt. -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 26/53] drm/i915/bdw: New logical ring submission mechanism
-Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, June 23, 2014 2:14 PM To: Mateo Lozano, Oscar Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism On Mon, Jun 23, 2014 at 01:09:37PM +, Mateo Lozano, Oscar wrote: So far, yes, but that´s only because I artificially made intel_lrc.c self- contained, as Daniel requested. What if we need to execute commands from somewhere else, like in intel_gen7_queue_flip()? And this takes me to another discussion: this logical ring vs legacy ring split is probably a good idea (time will tell), but we should provide a way of sending commands for execution without knowing if Execlists are enabled or not. In the early series that was easy because we reused the ring_begin, ring_emit ring_advance functions, but this is not the case anymore. And without this, sooner or later somebody will break legacy or execlists (this already happened last week, when somebody here was implementing native sync without knowing about Execlists). So, the questions is: how do you feel about a dev_priv.gt vfunc that takes a context, a ring, an array of DWORDS and a BB length and does the intel_(logical)_ring_begin/emit/advance based on i915.enable_execlists? I'm still baffled by the design. intel_ring_begin() and friends should be able to find their context (logical or legacy) from the ring and dtrt. -Chris Sorry, Chris, I obviously don´t have the same experience with 915 you have: how do you propose to extract the right context from the ring? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism
On Mon, Jun 23, 2014 at 01:18:35PM +, Mateo Lozano, Oscar wrote: -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, June 23, 2014 2:14 PM To: Mateo Lozano, Oscar Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism On Mon, Jun 23, 2014 at 01:09:37PM +, Mateo Lozano, Oscar wrote: So far, yes, but that´s only because I artificially made intel_lrc.c self- contained, as Daniel requested. What if we need to execute commands from somewhere else, like in intel_gen7_queue_flip()? And this takes me to another discussion: this logical ring vs legacy ring split is probably a good idea (time will tell), but we should provide a way of sending commands for execution without knowing if Execlists are enabled or not. In the early series that was easy because we reused the ring_begin, ring_emit ring_advance functions, but this is not the case anymore. And without this, sooner or later somebody will break legacy or execlists (this already happened last week, when somebody here was implementing native sync without knowing about Execlists). So, the questions is: how do you feel about a dev_priv.gt vfunc that takes a context, a ring, an array of DWORDS and a BB length and does the intel_(logical)_ring_begin/emit/advance based on i915.enable_execlists? I'm still baffled by the design. intel_ring_begin() and friends should be able to find their context (logical or legacy) from the ring and dtrt. -Chris Sorry, Chris, I obviously don´t have the same experience with 915 you have: how do you propose to extract the right context from the ring? The rings are a set of buffers and vfuncs that are associated with a context. Before you can call intel_ring_begin() you must know what context you want to operate on and therefore can pick the right logical/legacy ring and interface for RCS/BCS/VCS/etc -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 26/53] drm/i915/bdw: New logical ring submission mechanism
-Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, June 23, 2014 2:27 PM To: Mateo Lozano, Oscar Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism On Mon, Jun 23, 2014 at 01:18:35PM +, Mateo Lozano, Oscar wrote: -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, June 23, 2014 2:14 PM To: Mateo Lozano, Oscar Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism On Mon, Jun 23, 2014 at 01:09:37PM +, Mateo Lozano, Oscar wrote: So far, yes, but that´s only because I artificially made intel_lrc.c self- contained, as Daniel requested. What if we need to execute commands from somewhere else, like in intel_gen7_queue_flip()? And this takes me to another discussion: this logical ring vs legacy ring split is probably a good idea (time will tell), but we should provide a way of sending commands for execution without knowing if Execlists are enabled or not. In the early series that was easy because we reused the ring_begin, ring_emit ring_advance functions, but this is not the case anymore. And without this, sooner or later somebody will break legacy or execlists (this already happened last week, when somebody here was implementing native sync without knowing about Execlists). So, the questions is: how do you feel about a dev_priv.gt vfunc that takes a context, a ring, an array of DWORDS and a BB length and does the intel_(logical)_ring_begin/emit/advance based on i915.enable_execlists? I'm still baffled by the design. intel_ring_begin() and friends should be able to find their context (logical or legacy) from the ring and dtrt. -Chris Sorry, Chris, I obviously don´t have the same experience with 915 you have: how do you propose to extract the right context from the ring? The rings are a set of buffers and vfuncs that are associated with a context. Before you can call intel_ring_begin() you must know what context you want to operate on and therefore can pick the right logical/legacy ring and interface for RCS/BCS/VCS/etc -Chris Ok, but then you need to pass some extra stuff down together with the intel_engine_cs, either intel_context or intel_ringbuffer, right? Because that´s exactly what I did in previous versions, plumbing intel_context everywhere where it was needed (I could have plumbed intel_ringbuffer instead, it really doesn´t matter). This was rejected for being too intrusive and not allowing easy maintenance in the future. -- Oscar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism
On Mon, Jun 23, 2014 at 01:36:07PM +, Mateo Lozano, Oscar wrote: -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, June 23, 2014 2:27 PM To: Mateo Lozano, Oscar Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism On Mon, Jun 23, 2014 at 01:18:35PM +, Mateo Lozano, Oscar wrote: -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, June 23, 2014 2:14 PM To: Mateo Lozano, Oscar Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism On Mon, Jun 23, 2014 at 01:09:37PM +, Mateo Lozano, Oscar wrote: So far, yes, but that´s only because I artificially made intel_lrc.c self- contained, as Daniel requested. What if we need to execute commands from somewhere else, like in intel_gen7_queue_flip()? And this takes me to another discussion: this logical ring vs legacy ring split is probably a good idea (time will tell), but we should provide a way of sending commands for execution without knowing if Execlists are enabled or not. In the early series that was easy because we reused the ring_begin, ring_emit ring_advance functions, but this is not the case anymore. And without this, sooner or later somebody will break legacy or execlists (this already happened last week, when somebody here was implementing native sync without knowing about Execlists). So, the questions is: how do you feel about a dev_priv.gt vfunc that takes a context, a ring, an array of DWORDS and a BB length and does the intel_(logical)_ring_begin/emit/advance based on i915.enable_execlists? I'm still baffled by the design. intel_ring_begin() and friends should be able to find their context (logical or legacy) from the ring and dtrt. -Chris Sorry, Chris, I obviously don´t have the same experience with 915 you have: how do you propose to extract the right context from the ring? The rings are a set of buffers and vfuncs that are associated with a context. Before you can call intel_ring_begin() you must know what context you want to operate on and therefore can pick the right logical/legacy ring and interface for RCS/BCS/VCS/etc -Chris Ok, but then you need to pass some extra stuff down together with the intel_engine_cs, either intel_context or intel_ringbuffer, right? Because that´s exactly what I did in previous versions, plumbing intel_context everywhere where it was needed (I could have plumbed intel_ringbuffer instead, it really doesn´t matter). This was rejected for being too intrusive and not allowing easy maintenance in the future. Nope. You didn't redesign the ringbuffers to function as we expected but tacked on extra information and layering violations. -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 26/53] drm/i915/bdw: New logical ring submission mechanism
-Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, June 23, 2014 2:42 PM To: Mateo Lozano, Oscar Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism On Mon, Jun 23, 2014 at 01:36:07PM +, Mateo Lozano, Oscar wrote: -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, June 23, 2014 2:27 PM To: Mateo Lozano, Oscar Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism On Mon, Jun 23, 2014 at 01:18:35PM +, Mateo Lozano, Oscar wrote: -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, June 23, 2014 2:14 PM To: Mateo Lozano, Oscar Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism On Mon, Jun 23, 2014 at 01:09:37PM +, Mateo Lozano, Oscar wrote: So far, yes, but that´s only because I artificially made intel_lrc.c self- contained, as Daniel requested. What if we need to execute commands from somewhere else, like in intel_gen7_queue_flip()? And this takes me to another discussion: this logical ring vs legacy ring split is probably a good idea (time will tell), but we should provide a way of sending commands for execution without knowing if Execlists are enabled or not. In the early series that was easy because we reused the ring_begin, ring_emit ring_advance functions, but this is not the case anymore. And without this, sooner or later somebody will break legacy or execlists (this already happened last week, when somebody here was implementing native sync without knowing about Execlists). So, the questions is: how do you feel about a dev_priv.gt vfunc that takes a context, a ring, an array of DWORDS and a BB length and does the intel_(logical)_ring_begin/emit/advance based on i915.enable_execlists? I'm still baffled by the design. intel_ring_begin() and friends should be able to find their context (logical or legacy) from the ring and dtrt. -Chris Sorry, Chris, I obviously don´t have the same experience with 915 you have: how do you propose to extract the right context from the ring? The rings are a set of buffers and vfuncs that are associated with a context. Before you can call intel_ring_begin() you must know what context you want to operate on and therefore can pick the right logical/legacy ring and interface for RCS/BCS/VCS/etc -Chris Ok, but then you need to pass some extra stuff down together with the intel_engine_cs, either intel_context or intel_ringbuffer, right? Because that´s exactly what I did in previous versions, plumbing intel_context everywhere where it was needed (I could have plumbed intel_ringbuffer instead, it really doesn´t matter). This was rejected for being too intrusive and not allowing easy maintenance in the future. Nope. You didn't redesign the ringbuffers to function as we expected but tacked on extra information and layering violations. -Chris I know is no excuse, but as I said, I don´t know the code as well as you do. Let me explain to you the history of this one and maybe you can help me out discovering where I got it all wrong: - The original design I inherited from Ben and Jesse created a completely new struct intel_ring_buffer per context, and passed that one on instead of passing one of the dev_priv-ring[i] ones. When it was time to submit a context to the ELSP, they simply took it from ring-context. The problem with that was that creating an unbound number of struct intel_ring_buffer meant there were also an unbound number of things like struct list_head active_list or u32 irq_enable_mask, which made little sense. - What we really needed, I naively thought, is to get rid of the concept of ring: there are no rings, there are engines (like the render engine, the vebox, etc...) and there are ringbuffers (as in circular buffer with a head offset, tail offset, and control information). So I went on and renamed the old intel_ring_buffer to intel_engine_cs, then extracting a few things into a new intel_ringbuffer struct. Pre-Execlists, there was a 1:1 relationship between the ringbuffers and the engines. With Execlists, however, this 1:1 relationship is between the ringbuffers and the contexts. - I remember explaining this problem in a face-to-face meeting in the UK with some OTC people back in February (I think they tried to get you on the phone but they didn´t manage. I do remember they got Daniel though). Here, I proposed that an easy solution (easy, but maybe not right) was to plumb the
Re: [Intel-gfx] [PATCH 12/53] drm/i915/bdw: Populate LR contexts (somewhat)
On Mon, Jun 23, 2014 at 05:42:50AM -0700, Mateo Lozano, Oscar wrote:
-Original Message-
From: Volkin, Bradley D
+ reg_state[CTX_RING_HEAD+1] = 0;
+ reg_state[CTX_RING_TAIL] = RING_TAIL(ring-mmio_base);
+ reg_state[CTX_RING_TAIL+1] = 0;
+ reg_state[CTX_RING_BUFFER_START] = RING_START(ring-
mmio_base);
+ reg_state[CTX_RING_BUFFER_START+1] =
i915_gem_obj_ggtt_offset(ring_obj);
+ reg_state[CTX_RING_BUFFER_CONTROL] = RING_CTL(ring-
mmio_base);
+ reg_state[CTX_RING_BUFFER_CONTROL+1] = (31 * PAGE_SIZE) |
+RING_VALID;
The size here doesn't look right to me. Shouldn't it be (number of pages -
1)?
See init_ring_common()
But that´s exactly what it is, right?
Our ringbuf-size = 32 * PAGE_SIZE;
so we are setting 31 * PAGE_SIZE
Ok, on closer inspection, the result is correct because the Buffer Length
field happens to be bits 20:12. But it looked to me like a size-in-bytes
rather than the encoding I expected. So I guess I'd prefer that we do it
as in init_ring_common(), using ring_obj-base.size and the RING_NR_PAGES
mask so that it's a bit more obvious what's going on.
Thanks,
Brad
+ reg_state[CTX_BB_HEAD_U] = ring-mmio_base + 0x168;
+ reg_state[CTX_BB_HEAD_U+1] = 0;
+ reg_state[CTX_BB_HEAD_L] = ring-mmio_base + 0x140;
+ reg_state[CTX_BB_HEAD_L+1] = 0;
+ reg_state[CTX_BB_STATE] = ring-mmio_base + 0x110;
+ reg_state[CTX_BB_STATE+1] = (15);
+ reg_state[CTX_SECOND_BB_HEAD_U] = ring-mmio_base + 0x11c;
+ reg_state[CTX_SECOND_BB_HEAD_U+1] = 0;
+ reg_state[CTX_SECOND_BB_HEAD_L] = ring-mmio_base + 0x114;
+ reg_state[CTX_SECOND_BB_HEAD_L+1] = 0;
+ reg_state[CTX_SECOND_BB_STATE] = ring-mmio_base + 0x118;
+ reg_state[CTX_SECOND_BB_STATE+1] = 0;
+ if (ring-id == RCS) {
+ reg_state[CTX_BB_PER_CTX_PTR] = ring-mmio_base +
0x1c0;
+ reg_state[CTX_BB_PER_CTX_PTR+1] = 0;
+ reg_state[CTX_RCS_INDIRECT_CTX] = ring-mmio_base +
0x1c4;
+ reg_state[CTX_RCS_INDIRECT_CTX+1] = 0;
+ reg_state[CTX_RCS_INDIRECT_CTX_OFFSET] = ring-
mmio_base + 0x1c8;
+ reg_state[CTX_RCS_INDIRECT_CTX_OFFSET+1] = 0;
+ }
+ reg_state[CTX_LRI_HEADER_1] = MI_LOAD_REGISTER_IMM(9);
+ reg_state[CTX_LRI_HEADER_1] |= MI_LRI_FORCE_POSTED;
+ reg_state[CTX_CTX_TIMESTAMP] = ring-mmio_base + 0x3a8;
+ reg_state[CTX_CTX_TIMESTAMP+1] = 0;
+ reg_state[CTX_PDP3_UDW] = GEN8_RING_PDP_UDW(ring, 3);
+ reg_state[CTX_PDP3_LDW] = GEN8_RING_PDP_LDW(ring, 3);
+ reg_state[CTX_PDP2_UDW] = GEN8_RING_PDP_UDW(ring, 2);
+ reg_state[CTX_PDP2_LDW] = GEN8_RING_PDP_LDW(ring, 2);
+ reg_state[CTX_PDP1_UDW] = GEN8_RING_PDP_UDW(ring, 1);
+ reg_state[CTX_PDP1_LDW] = GEN8_RING_PDP_LDW(ring, 1);
+ reg_state[CTX_PDP0_UDW] = GEN8_RING_PDP_UDW(ring, 0);
+ reg_state[CTX_PDP0_LDW] = GEN8_RING_PDP_LDW(ring, 0);
+ reg_state[CTX_PDP3_UDW+1] = (u64)ppgtt-pd_dma_addr[3] 32;
+ reg_state[CTX_PDP3_LDW+1] = ppgtt-pd_dma_addr[3];
+ reg_state[CTX_PDP2_UDW+1] = (u64)ppgtt-pd_dma_addr[2] 32;
+ reg_state[CTX_PDP2_LDW+1] = ppgtt-pd_dma_addr[2];
+ reg_state[CTX_PDP1_UDW+1] = (u64)ppgtt-pd_dma_addr[1] 32;
+ reg_state[CTX_PDP1_LDW+1] = ppgtt-pd_dma_addr[1];
+ reg_state[CTX_PDP0_UDW+1] = (u64)ppgtt-pd_dma_addr[0] 32;
+ reg_state[CTX_PDP0_LDW+1] = ppgtt-pd_dma_addr[0];
Are we able to use upper_32_bits() and lower_32_bits() for these?
ACK
-- Oscar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/53] drm/i915/bdw: Populate LR contexts (somewhat)
-Original Message- From: Volkin, Bradley D Sent: Monday, June 23, 2014 4:06 PM To: Mateo Lozano, Oscar Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 12/53] drm/i915/bdw: Populate LR contexts (somewhat) On Mon, Jun 23, 2014 at 05:42:50AM -0700, Mateo Lozano, Oscar wrote: -Original Message- From: Volkin, Bradley D + reg_state[CTX_RING_HEAD+1] = 0; + reg_state[CTX_RING_TAIL] = RING_TAIL(ring-mmio_base); + reg_state[CTX_RING_TAIL+1] = 0; + reg_state[CTX_RING_BUFFER_START] = RING_START(ring- mmio_base); + reg_state[CTX_RING_BUFFER_START+1] = i915_gem_obj_ggtt_offset(ring_obj); + reg_state[CTX_RING_BUFFER_CONTROL] = RING_CTL(ring- mmio_base); + reg_state[CTX_RING_BUFFER_CONTROL+1] = (31 * PAGE_SIZE) | +RING_VALID; The size here doesn't look right to me. Shouldn't it be (number of pages - 1)? See init_ring_common() But that´s exactly what it is, right? Our ringbuf-size = 32 * PAGE_SIZE; so we are setting 31 * PAGE_SIZE Ok, on closer inspection, the result is correct because the Buffer Length field happens to be bits 20:12. But it looked to me like a size-in-bytes rather than the encoding I expected. So I guess I'd prefer that we do it as in init_ring_common(), using ring_obj-base.size and the RING_NR_PAGES mask so that it's a bit more obvious what's going on. Thanks, Brad Yeah, it probably looks less magical that way. Ok, will do. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 27/53] drm/i915/bdw: GEN-specific logical ring emit request
-Original Message-
From: Volkin, Bradley D
Sent: Friday, June 20, 2014 10:18 PM
To: Mateo Lozano, Oscar
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 27/53] drm/i915/bdw: GEN-specific logical
ring emit request
On Fri, Jun 13, 2014 at 08:37:45AM -0700, oscar.ma...@intel.com wrote:
From: Oscar Mateo oscar.ma...@intel.com
Very similar to the legacy add_request, only modified to account for
logical ringbuffer.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_lrc.c| 61
+
drivers/gpu/drm/i915/intel_ringbuffer.h | 2 ++
3 files changed, 64 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h index 9c8692a..63ec3ea 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -267,6 +267,7 @@
#define MI_FORCE_RESTORE (11)
#define MI_RESTORE_INHIBIT (10)
#define MI_STORE_DWORD_IMM MI_INSTR(0x20, 1)
+#define MI_STORE_DWORD_IMM_GEN8MI_INSTR(0x20, 2)
#define MI_MEM_VIRTUAL (1 22) /* 965+ only */
#define MI_STORE_DWORD_INDEX MI_INSTR(0x21, 1)
#define MI_STORE_DWORD_INDEX_SHIFT 2
diff --git a/drivers/gpu/drm/i915/intel_lrc.c
b/drivers/gpu/drm/i915/intel_lrc.c
index 89aed7a..3debe8b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -359,6 +359,62 @@ static void gen8_submit_ctx(struct
intel_engine_cs *ring,
DRM_ERROR(Execlists still not ready!\n); }
+static int gen8_emit_request(struct intel_engine_cs *ring,
+struct intel_context *ctx)
+{
+ struct intel_ringbuffer *ringbuf = logical_ringbuf_get(ring, ctx);
+ u32 cmd;
+ int ret;
+
+ ret = intel_logical_ring_begin(ring, ctx, 6);
+ if (ret)
+ return ret;
+
+ cmd = MI_FLUSH_DW + 1;
+ cmd |= MI_INVALIDATE_TLB;
Is the TLB invalidation truely required here? Otherwise it seems like we could
use the same function for all rings, like on gen6+.
Hm... this is inherited from back when we only had the simulator, and its
true meaning has been lost in the multiple rewrites:
drm/i915/bdw: Use MI_FLUSH_DW for requests
The primary reason for doing this is MI_STORE_DWORD_IDX doesn't work in
simulation. The simulator doesn't complain, it's just the seqno never
gets pushed to memory. The theory is (and AFAICT this may be broken on
existing platforms) we must issue an MI_FLUSH_DW after we emit the
seqno, if we want to be able to read it back coherently.
I´ll rewrite it to use the same MI_STORE_DATA_IMM for every ring and then test
it on read hardware.
Thanks for the heads up!
+ cmd |= MI_FLUSH_DW_OP_STOREDW;
+
+ intel_logical_ring_emit(ringbuf, cmd);
+ intel_logical_ring_emit(ringbuf,
+ (ring-status_page.gfx_addr +
+ (I915_GEM_HWS_INDEX
MI_STORE_DWORD_INDEX_SHIFT)) |
+ MI_FLUSH_DW_USE_GTT);
+ intel_logical_ring_emit(ringbuf, 0);
+ intel_logical_ring_emit(ringbuf, ring-outstanding_lazy_seqno);
+ intel_logical_ring_emit(ringbuf, MI_USER_INTERRUPT);
+ intel_logical_ring_emit(ringbuf, MI_NOOP);
+ intel_logical_ring_advance_and_submit(ring, ctx);
+
+ return 0;
+}
+
+static int gen8_emit_request_render(struct intel_engine_cs *ring,
+ struct intel_context *ctx)
+{
+ struct intel_ringbuffer *ringbuf = logical_ringbuf_get(ring, ctx);
+ u32 cmd;
+ int ret;
+
+ ret = intel_logical_ring_begin(ring, ctx, 6);
+ if (ret)
+ return ret;
+
+ cmd = MI_STORE_DWORD_IMM_GEN8;
+ cmd |= (1 22); /* use global GTT */
We could use MI_MEM_VIRTUAL or MI_GLOBAL_GTT instead.
Will do, thanks.
-- Oscar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Only mark the ctx as initialised after a SET_CONTEXT operation
On Mon, Jun 23, 2014 at 12:55:47PM +0300, Jani Nikula wrote: On Fri, 30 May 2014, Chris Wilson ch...@chris-wilson.co.uk wrote: Fallout from commit 46470fc932ac8a0e8317a220b3f4ea4ed903338e Author: Mika Kuoppala mika.kuopp...@linux.intel.com Date: Wed May 21 19:01:06 2014 +0300 drm/i915: Add null state batch to active list undid the earlier fix of only marking the ctx as initialised after it is saved by the hardware during a SET_CONTEXT operation. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Damien Lespiau damien.lesp...@intel.com Cc: Mika Kuoppala mika.kuopp...@intel.com Cc: Ben Widawsky b...@bwidawsk.net Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79108 Daniel put his IRC r-b on this on IIRC. [snip] -- Ben Widawsky, 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 26/53] drm/i915/bdw: New logical ring submission mechanism
On Mon, Jun 23, 2014 at 07:35:38AM -0700, Mateo Lozano, Oscar wrote: -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, June 23, 2014 2:42 PM To: Mateo Lozano, Oscar Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism On Mon, Jun 23, 2014 at 01:36:07PM +, Mateo Lozano, Oscar wrote: -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, June 23, 2014 2:27 PM To: Mateo Lozano, Oscar Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism On Mon, Jun 23, 2014 at 01:18:35PM +, Mateo Lozano, Oscar wrote: -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, June 23, 2014 2:14 PM To: Mateo Lozano, Oscar Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism On Mon, Jun 23, 2014 at 01:09:37PM +, Mateo Lozano, Oscar wrote: So far, yes, but that´s only because I artificially made intel_lrc.c self- contained, as Daniel requested. What if we need to execute commands from somewhere else, like in intel_gen7_queue_flip()? And this takes me to another discussion: this logical ring vs legacy ring split is probably a good idea (time will tell), but we should provide a way of sending commands for execution without knowing if Execlists are enabled or not. In the early series that was easy because we reused the ring_begin, ring_emit ring_advance functions, but this is not the case anymore. And without this, sooner or later somebody will break legacy or execlists (this already happened last week, when somebody here was implementing native sync without knowing about Execlists). So, the questions is: how do you feel about a dev_priv.gt vfunc that takes a context, a ring, an array of DWORDS and a BB length and does the intel_(logical)_ring_begin/emit/advance based on i915.enable_execlists? There are 3 cases of non-execbuffer submissions that I can think of: flips, render state, and clear-buffer (proposed patches on the list). I wonder if the right approach might be to use batchbuffers with a small wrapper around the dispatch_execbuffer/emit_bb_start vfuncs. Basically the rule would be to only touch a ringbuffer from within the intel_engine_cs vfuncs, which always know which set of functions to use. For flips, we could use MMIO flips. Render state already uses the existing dispatch_execbuffer() and add_request(). The clear code could potentially do the same. There would obviously be some overhead in using a batch buffer for what could end up being just a few commands. Perhaps the batch buffer pool code from the command parser would help though. I'm still baffled by the design. intel_ring_begin() and friends should be able to find their context (logical or legacy) from the ring and dtrt. -Chris Sorry, Chris, I obviously don´t have the same experience with 915 you have: how do you propose to extract the right context from the ring? The rings are a set of buffers and vfuncs that are associated with a context. Before you can call intel_ring_begin() you must know what context you want to operate on and therefore can pick the right logical/legacy ring and interface for RCS/BCS/VCS/etc -Chris Ok, but then you need to pass some extra stuff down together with the intel_engine_cs, either intel_context or intel_ringbuffer, right? Because that´s exactly what I did in previous versions, plumbing intel_context everywhere where it was needed (I could have plumbed intel_ringbuffer instead, it really doesn´t matter). This was rejected for being too intrusive and not allowing easy maintenance in the future. Nope. You didn't redesign the ringbuffers to function as we expected but tacked on extra information and layering violations. Not sure what your proposed alternative is here Chris. I'll elaborate on what I had proposed r.e. plumbing intel_ringbuffer instead of intel_context, which might be along the lines of what you envision. At this point, we're starting to go in circles, so I don't know if it's worth revisting beyond that. The earlier versions of the series modified all of the intel_ring_* functions to accept (engine, context) as parameters. At or near the bottom of the call chain (e.g. in the engine vfuncs), we called a new function, intel_ringbuffer_get(engine, context), to return the appropriate ringbuffer in both legacy and lrc modes. However, a given struct intel_ringbuffer is only ever
[Intel-gfx] i915 tracepoints - perf list shows none
Hi, $ perf list I see net, iommu and other trace events listed however I do not see any i915 ones. Is there anything in particular I need to do at build or runtime to make them show up? i915 is loaded. I am using the perf tool built under ~/tools/perf. Thanks, Mike ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Linux 3.16-rc2
Am Samstag, den 21.06.2014, 19:22 -1000 schrieb Linus Torvalds: It's a day early, but tomorrow ends up being inconvenient for me due to being on the road most of the day, so here you are. These days most people send me their pull requests and patches during the week, so it's not like I expect that a Sunday release would have made much of a difference. And it's also not like I didn't have enough changes for making a rc2 release. Anyway, enough excuses. 3.16-rc2 is out, and contains the usual assortment of fixes all over the map. The most unusual part at this point is how the sparc changes stand out (at almost 40% of the patch by bulk), but they are basically all just sparse warning cleanups. Similarly, some Nouveau drm changes standing out size-wise, but again those are largely due to small firmware fixes resulting in big generated changes. The actual real changes are fairly small. Ignoring those two unusually large changes (in lines), everything else looks fairly normal. There are driver changes, some tooling updates (particularly perf), and various other arch updates (arm, s390, unicore32, x86..). And just misc random stuff all over the place - rtmutex, btrfs, yadda yadda. The shortlog is not tiny, but small enough to include here, so you can see the details there if you care. Please do go test it out, the i915 driver is still broken in 3.16-rc2. Resume from ram crashes the X server. First bad commit is: # first bad commit: [78f2975eec9faff353a6194e854d3d39907bab68] drm/i915: Move all ring resets before setting the HWS page commit 78f2975eec9faff353a6194e854d3d39907bab68 Author: Chris Wilson ch...@chris-wilson.co.uk Date: Wed Apr 2 16:36:07 2014 +0100 drm/i915: Move all ring resets before setting the HWS page In commit a51435a3137ad8ae75c288c39bd2d8b2696bae8f Author: Naresh Kumar Kachhi naresh.kumar.kac...@intel.com Date: Wed Mar 12 16:39:40 2014 +0530 drm/i915: disable rings before HW status page setup we reordered stopping the rings to do so before we set the HWS register. However, there is an extra workaround for g45 to reset the rings twice, and for consistency we should apply that workaround before setting the HWS to be sure that the rings are truly stopped. Reference: http://lkml.kernel.org/r/20140423202248.ga3...@amd.pavel.ucw.cz Tested-by: Pavel Machek pa...@ucw.cz Cc: Naresh Kumar Kachhi naresh.kumar.kac...@intel.com Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Jani Nikula jani.nik...@intel.com Above commit is not revertable anymore on 3.16-rc2 without conflict. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism
On Mon, Jun 23, 2014 at 02:35:38PM +, Mateo Lozano, Oscar wrote: -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, June 23, 2014 2:42 PM To: Mateo Lozano, Oscar Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism On Mon, Jun 23, 2014 at 01:36:07PM +, Mateo Lozano, Oscar wrote: -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, June 23, 2014 2:27 PM To: Mateo Lozano, Oscar Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism On Mon, Jun 23, 2014 at 01:18:35PM +, Mateo Lozano, Oscar wrote: -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, June 23, 2014 2:14 PM To: Mateo Lozano, Oscar Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism On Mon, Jun 23, 2014 at 01:09:37PM +, Mateo Lozano, Oscar wrote: So far, yes, but that´s only because I artificially made intel_lrc.c self- contained, as Daniel requested. What if we need to execute commands from somewhere else, like in intel_gen7_queue_flip()? And this takes me to another discussion: this logical ring vs legacy ring split is probably a good idea (time will tell), but we should provide a way of sending commands for execution without knowing if Execlists are enabled or not. In the early series that was easy because we reused the ring_begin, ring_emit ring_advance functions, but this is not the case anymore. And without this, sooner or later somebody will break legacy or execlists (this already happened last week, when somebody here was implementing native sync without knowing about Execlists). So, the questions is: how do you feel about a dev_priv.gt vfunc that takes a context, a ring, an array of DWORDS and a BB length and does the intel_(logical)_ring_begin/emit/advance based on i915.enable_execlists? I'm still baffled by the design. intel_ring_begin() and friends should be able to find their context (logical or legacy) from the ring and dtrt. -Chris Sorry, Chris, I obviously don´t have the same experience with 915 you have: how do you propose to extract the right context from the ring? The rings are a set of buffers and vfuncs that are associated with a context. Before you can call intel_ring_begin() you must know what context you want to operate on and therefore can pick the right logical/legacy ring and interface for RCS/BCS/VCS/etc -Chris Ok, but then you need to pass some extra stuff down together with the intel_engine_cs, either intel_context or intel_ringbuffer, right? Because that´s exactly what I did in previous versions, plumbing intel_context everywhere where it was needed (I could have plumbed intel_ringbuffer instead, it really doesn´t matter). This was rejected for being too intrusive and not allowing easy maintenance in the future. Nope. You didn't redesign the ringbuffers to function as we expected but tacked on extra information and layering violations. -Chris I know is no excuse, but as I said, I don´t know the code as well as you do. Let me explain to you the history of this one and maybe you can help me out discovering where I got it all wrong: - The original design I inherited from Ben and Jesse created a completely new struct intel_ring_buffer per context, and passed that one on instead of passing one of the dev_priv-ring[i] ones. When it was time to submit a context to the ELSP, they simply took it from ring-context. The problem with that was that creating an unbound number of struct intel_ring_buffer meant there were also an unbound number of things like struct list_head active_list or u32 irq_enable_mask, which made little sense. - What we really needed, I naively thought, is to get rid of the concept of ring: there are no rings, there are engines (like the render engine, the vebox, etc...) and there are ringbuffers (as in circular buffer with a head offset, tail offset, and control information). So I went on and renamed the old intel_ring_buffer to intel_engine_cs, then extracting a few things into a new intel_ringbuffer struct. Pre-Execlists, there was a 1:1 relationship between the ringbuffers and the engines. With Execlists, however, this 1:1 relationship is between the ringbuffers and the contexts. - I remember explaining this problem in a face-to-face meeting in the UK with some OTC people back in February (I think they tried
[Intel-gfx] [PATCH 6/6] drm/i915/bdw: Enable full PPGTT
Broadwell is perfectly capable of full PPGTT. I've been using it for
some time, and seen no especially ill effects.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Conflicts:
drivers/gpu/drm/i915/i915_drv.h
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8cea596..e91f27a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1992,7 +1992,7 @@ 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 HAS_PPGTT(dev) (INTEL_INFO(dev)-gen = 7
!IS_CHERRYVIEW(dev))
#define USES_PPGTT(dev)intel_enable_ppgtt(dev, false)
#define USES_FULL_PPGTT(dev) intel_enable_ppgtt(dev, true)
--
2.0.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/6] drm/i915: Only mark the ctx as initialised after a SET_CONTEXT operation
From: Chris Wilson ch...@chris-wilson.co.uk
Fallout from
commit 46470fc932ac8a0e8317a220b3f4ea4ed903338e
Author: Mika Kuoppala mika.kuopp...@linux.intel.com
Date: Wed May 21 19:01:06 2014 +0300
drm/i915: Add null state batch to active list
undid the earlier fix of only marking the ctx as initialised after it is
saved by the hardware during a SET_CONTEXT operation.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Reviewed-by: Ben Widawsky b...@bwidawsk.net
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Damien Lespiau damien.lesp...@intel.com
Cc: Mika Kuoppala mika.kuopp...@intel.com
---
drivers/gpu/drm/i915/i915_gem_context.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 21eda88..0d2c75b 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -598,6 +598,7 @@ static int do_switch(struct intel_engine_cs *ring,
struct intel_context *from = ring-last_context;
struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(to);
u32 hw_flags = 0;
+ bool uninitialized = false;
int ret, i;
if (from != NULL ring == dev_priv-ring[RCS]) {
@@ -696,18 +697,19 @@ static int do_switch(struct intel_engine_cs *ring,
i915_gem_context_unreference(from);
}
+ uninitialized = !to-is_initialized from == NULL;
+ to-is_initialized = true;
+
done:
i915_gem_context_reference(to);
ring-last_context = to;
- if (ring-id == RCS !to-is_initialized from == NULL) {
+ if (uninitialized) {
ret = i915_gem_render_state_init(ring);
if (ret)
DRM_ERROR(init render state: %d\n, ret);
}
- to-is_initialized = true;
-
return 0;
unpin_out:
--
2.0.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/6] drm/i915/ppgtt: Load address space after mi_set_context
The simple explanation is, the docs say to do this for GEN8. Perhaps we
want to do this for GEN7 too, I am not certain.
PDPs are saved and restored with context. Contexts (without execlists)
only exist on the render ring. The docs say that PDPs are not power
context save/restored. I've learned that this actually means something
which SW doesn't care about. So pretend the statement doesn't exist.
For non RCS, nothing changes.
All this patch now does is change the ordering of LRI vs MI_SET_CONTEXT
for the initialization of the context. I do this because the docs say to
do it, and frankly, I cannot reason why it is necessary. I've thought
about it a lot, and tried, without success, to get a reason from design.
The answer I got more or less says, gen7 is different than gen8. I've
given up, and am adding this little bit of code to make it in sync with
the docs.
v2: Completely rewritten commit message that addresses the requests
Ville made for v1
Only load PDPs for initial context load (Ville)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_gem_context.c | 29 +++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index f2c4948..7c69738 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -649,6 +649,7 @@ static int do_switch_rcs(struct intel_engine_cs *ring,
struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(to);
u32 hw_flags = 0;
bool uninitialized = false;
+ bool needs_pd_load = (INTEL_INFO(ring-dev)-gen 8)
USES_FULL_PPGTT(ring-dev);
int ret;
if (from != NULL) {
@@ -668,7 +669,10 @@ static int do_switch_rcs(struct intel_engine_cs *ring,
*/
from = ring-last_context;
- if (USES_FULL_PPGTT(ring-dev)) {
+ if (needs_pd_load) {
+ /* Older GENs still want the load first, PP_DCLV followed by
+* PP_DIR_BASE register through Load Register Immediate commands
+* in Ring Buffer before submitting a context.*/
ret = ppgtt-switch_mm(ppgtt, ring, false);
if (ret)
goto unpin_out;
@@ -692,13 +696,34 @@ static int do_switch_rcs(struct intel_engine_cs *ring,
vma-bind_vma(vma, to-obj-cache_level, GLOBAL_BIND);
}
- if (!to-is_initialized || i915_gem_context_is_default(to))
+ if (!to-is_initialized || i915_gem_context_is_default(to)) {
hw_flags |= MI_RESTORE_INHIBIT;
+ needs_pd_load = USES_FULL_PPGTT(ring-dev)
IS_GEN8(ring-dev);
+ }
ret = mi_set_context(ring, to, hw_flags);
if (ret)
goto unpin_out;
+ /* GEN8 does *not* require an explicit reload if the PDPs have been
+* setup, and we do not wish to move them.
+*
+* XXX: If we implemented page directory eviction code, this
+* optimization needs to be removed.
+*/
+ if (needs_pd_load) {
+ ret = ppgtt-switch_mm(ppgtt, ring, false);
+ /* The hardware context switch is emitted, but we haven't
+* actually changed the state - so it's probably safe to bail
+* here. Still, let the user know something dangerous has
+* happened.
+*/
+ if (ret) {
+ DRM_ERROR(Failed to change address space on context
switch\n);
+ goto unpin_out;
+ }
+ }
+
remap_l3(ring, to);
/* The backing object for the context is done after switching to the
--
2.0.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/6] drm/i915: Prevent signals from interrupting close()
From: Chris Wilson ch...@chris-wilson.co.uk
We neither report any unfinished operations during releasing GEM objects
associated with the file, and even if we did, it is bad form to report
-EINTR from a close().
The root cause of the bug that first showed itself during close is that
we do not do proper live tracking of vma and contexts under full-ppgtt,
but this is useful piece of defensive programming enforcing our
userspace API contract.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Reviewed-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_dma.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5e583a1..f6aca71 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1959,9 +1959,18 @@ void i915_driver_lastclose(struct drm_device *dev)
void i915_driver_preclose(struct drm_device *dev, struct drm_file *file_priv)
{
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ bool was_interruptible;
+
mutex_lock(dev-struct_mutex);
+ was_interruptible = dev_priv-mm.interruptible;
+ WARN_ON(!was_interruptible);
+ dev_priv-mm.interruptible = false;
+
i915_gem_context_close(dev, file_priv);
i915_gem_release(dev, file_priv);
+
+ dev_priv-mm.interruptible = was_interruptible;
mutex_unlock(dev-struct_mutex);
}
--
2.0.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/6] drm/i915: Extract l3 remapping out of ctx switch
This is just a cosmetic change to try to put do_switch_rcs on a diet. As
it stands, the function was quite complex, and error prone.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_gem_context.c | 32
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 35985ad..f2c4948 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -623,6 +623,24 @@ static int do_switch_xcs(struct intel_engine_cs *ring,
return 0;
}
+static void remap_l3(struct intel_engine_cs *ring,
+struct intel_context *ctx)
+{
+ int ret, i;
+
+ for (i = 0; i MAX_L3_SLICES; i++) {
+ if (!(ctx-remap_slice (1i)))
+ continue;
+
+ ret = i915_gem_l3_remap(ring, i);
+ /* If it failed, try again next round */
+ if (ret)
+ DRM_DEBUG_DRIVER(L3 remapping failed\n);
+ else
+ ctx-remap_slice = ~(1i);
+ }
+}
+
static int do_switch_rcs(struct intel_engine_cs *ring,
struct intel_context *from,
struct intel_context *to)
@@ -631,7 +649,7 @@ static int do_switch_rcs(struct intel_engine_cs *ring,
struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(to);
u32 hw_flags = 0;
bool uninitialized = false;
- int ret, i;
+ int ret;
if (from != NULL) {
BUG_ON(from-obj == NULL);
@@ -681,17 +699,7 @@ static int do_switch_rcs(struct intel_engine_cs *ring,
if (ret)
goto unpin_out;
- for (i = 0; i MAX_L3_SLICES; i++) {
- if (!(to-remap_slice (1i)))
- continue;
-
- ret = i915_gem_l3_remap(ring, i);
- /* If it failed, try again next round */
- if (ret)
- DRM_DEBUG_DRIVER(L3 remapping failed\n);
- else
- to-remap_slice = ~(1i);
- }
+ remap_l3(ring, to);
/* The backing object for the context is done after switching to the
* *next* context. Therefore we cannot retire the previous context until
--
2.0.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/opregion: ignore firmware requests for backlight change
Some Thinkpad laptops' firmware will initiate a backlight level change
request through operation region on the events of AC plug/unplug, but
since we are not using firmware's interface to do the backlight setting
on these affected laptops, we do not want the firmware to use some
arbitrary value from its ASL variable to set the backlight level on
AC plug/unplug either.
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=76491
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=77091
Reported-and-tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
Reported-and-tested-by: Anton Gubarkov anton.gubar...@gmail.com
Signed-off-by: Aaron Lu aaron...@intel.com
---
drivers/acpi/video.c | 3 ++-
drivers/gpu/drm/i915/intel_opregion.c | 7 +++
include/acpi/video.h | 2 ++
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index fb9ffe9adc64..cf99d6d2d491 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -241,13 +241,14 @@ static bool acpi_video_use_native_backlight(void)
return use_native_backlight_dmi;
}
-static bool acpi_video_verify_backlight_support(void)
+bool acpi_video_verify_backlight_support(void)
{
if (acpi_osi_is_win8() acpi_video_use_native_backlight()
backlight_device_registered(BACKLIGHT_RAW))
return false;
return acpi_video_backlight_support();
}
+EXPORT_SYMBOL(acpi_video_verify_backlight_support);
/* backlight device sysfs support */
static int acpi_video_get_brightness(struct backlight_device *bd)
diff --git a/drivers/gpu/drm/i915/intel_opregion.c
b/drivers/gpu/drm/i915/intel_opregion.c
index 2e2c71fcc9ed..02943d93e88e 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -403,6 +403,13 @@ static u32 asle_set_backlight(struct drm_device *dev, u32
bclp)
DRM_DEBUG_DRIVER(bclp = 0x%08x\n, bclp);
+ /*
+* If the acpi_video interface is not supposed to be used, don't
+* bother processing backlight level change requests from firmware.
+*/
+ if (!acpi_video_verify_backlight_support())
+ return 0;
+
if (!(bclp ASLE_BCLP_VALID))
return ASLC_BACKLIGHT_FAILED;
diff --git a/include/acpi/video.h b/include/acpi/video.h
index ea4c7bbded4d..92f8c4bffefb 100644
--- a/include/acpi/video.h
+++ b/include/acpi/video.h
@@ -22,6 +22,7 @@ extern void acpi_video_unregister(void);
extern void acpi_video_unregister_backlight(void);
extern int acpi_video_get_edid(struct acpi_device *device, int type,
int device_id, void **edid);
+extern bool acpi_video_verify_backlight_support(void);
#else
static inline int acpi_video_register(void) { return 0; }
static inline void acpi_video_unregister(void) { return; }
@@ -31,6 +32,7 @@ static inline int acpi_video_get_edid(struct acpi_device
*device, int type,
{
return -ENODEV;
}
+static bool acpi_video_verify_backlight_support() { return false; }
#endif
#endif
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type
On 2014.06.19 17:53:51 +0800, Tiejun Chen wrote: Originally the reason to probe ISA bridge instead of Dev31:Fun0 is to make graphics device passthrough work easy for VMM, that only need to expose ISA bridge to let driver know the real hardware underneath. This is a requirement from virtualization team. Especially in that virtualized environments, XEN, there is irrelevant ISA bridge in the system with that legacy qemu version specific to xen, qemu-xen-traditional. So to work reliably, we should scan through all the ISA bridge devices and check for the first match, instead of only checking the first one. But actually, qemu-xen-traditional, is always enumerated with Dev31:Fun0, 00:1f.0 as follows: hw/pt-graphics.c: intel_pch_init() | + pci_isa_bridge_init(bus, PCI_DEVFN(0x1f, 0), ...); so this mean that isa bridge is still represented with Dev31:Func0 like the native OS. Furthermore, currently we're pushing VGA passthrough support into qemu upstream, and with some discussion, we wouldn't set the bridge class type and just expose this devfn. So we just go back to check devfn to make life normal. Signed-off-by: Tiejun Chen tiejun.c...@intel.com This was added historically when supporting graphics device passthrough. Looks qemu upstream can't accept multiple ISA bridge and our PCH is always on device 31: func0 as far as I know. Looks good to me. Reviewed-by: Zhenyu Wang zhen...@linux.intel.com -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 signature.asc Description: Digital signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
