Re: [Intel-gfx] [PATCH 1/4] drm/i915: get power domain in case the BIOS enabled eDP VDD
On Tue, Apr 22, 2014 at 07:55:42PM -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
If I unplug the eDP monitor, the BIOS of my machine will enable the
VDD bit, then when the driver loads it will think VDD is enabled. It
will detect that the eDP is not enabled and return false from
intel_edp_init_connector. This will trigger a call to
edp_panel_vdd_off_sync(), which trigger a WARN saying that the
refcount of the power domain is less than zero.
The problem happens because the driver gets a refcount whenever it
enables the VDD bit, and puts the refcount whenever it disables the
VDD bit. But on this case, the BIOS enabled VDD, so all we do is to
call put() without calling get() first, so the code added is there to
make sure we always have the get() in case the BIOS enabled the bit.
v2: - Rebase
Tested-by: Chris Wilson ch...@chris-wilson.co.uk (v1)
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
This regression was introduced in
commit e9cb81a22841908b1c075156b409a538d09c8466
Author: Paulo Zanoni paulo.r.zan...@intel.com
Date: Thu Nov 21 13:47:23 2013 -0200
drm/i915: get a runtime PM reference when the panel VDD is on
Please don't forget the regression notice! With that this is
Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
Cc: sta...@vger.kernel.org (v3.13+)
---
drivers/gpu/drm/i915/intel_dp.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 80e5598..44df493 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3785,7 +3785,8 @@ static bool intel_edp_init_connector(struct intel_dp
*intel_dp,
{
struct drm_connector *connector = intel_connector-base;
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
- struct drm_device *dev = intel_dig_port-base.base.dev;
+ struct intel_encoder *intel_encoder = intel_dig_port-base;
+ struct drm_device *dev = intel_encoder-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_display_mode *fixed_mode = NULL;
struct drm_display_mode *downclock_mode = NULL;
@@ -3798,6 +3799,14 @@ static bool intel_edp_init_connector(struct intel_dp
*intel_dp,
if (!is_edp(intel_dp))
return true;
+ /* The VDD bit needs a power domain reference, so if the bit is already
+ * enabled when we boot, grab this reference. */
+ if (edp_have_panel_vdd(intel_dp)) {
+ enum intel_display_power_domain power_domain;
+ power_domain = intel_display_port_power_domain(intel_encoder);
+ intel_display_power_get(dev_priv, power_domain);
+ }
+
/* Cache DPCD and EDID for edp. */
intel_edp_panel_vdd_on(intel_dp);
has_dpcd = intel_dp_get_dpcd(intel_dp);
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert drm/i915: fix infinite loop at gen6_update_ring_freq
On Tue, Apr 22, 2014 at 06:25:12PM -0300, Paulo Zanoni wrote: 2014-04-11 6:02 GMT-03:00 Daniel Vetter dan...@ffwll.ch: On Thu, Apr 10, 2014 at 10:52:26AM -0700, Ben Widawsky wrote: On Thu, Apr 10, 2014 at 10:50:43AM -0700, Ben Widawsky wrote: On Thu, Apr 10, 2014 at 09:04:47AM +0200, Daniel Vetter wrote: This reverts commit 4b28a1f3ef55a3b0b68dbab1fe6dbaf18e186710. This patch duct-tapes over some issue in the current bdw rps patches which must wait with enabling rc6/rps until the very first batch has been submitted by userspace. But those patches aren't merged yet, and for upstream we need to have an in-kernel emission of the very first batch. I shouldn't have merged this patch so let's revert it again. I said this on the mailing last before you merged the patch. 20140402050338.gb13...@bwidawsk.net 20140402145813.GV7225@phenom.ffwll.local will explain things. There's now a regression report pointing to the revert: https://bugs.freedesktop.org/show_bug.cgi?id=77565 . What is the proposed solution now? Just WARN and still avoid the infinite loop? Or keep the infinite loop and leave the bug open? Disable BDW runtime PM? I've thought that we can only hit this with the as-yet unmerged rc6 patches on bdw, so I'm really confused why this blows up now? In any case I've thought Imre has stumbled over a similar issue on byt and he has a fix to prevent runtime pm until the delayed rps init has run. I've assigned the bug to him. Still confused why this suddenly blew up ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915: get a runtime PM ref for the deferred GPU reset work
On Wed, Apr 23, 2014 at 01:13:40AM +0300, Imre Deak wrote:
Atm we can end up in the GPU reset deferred work in D3 state if the last
runtime PM reference is dropped between detecting a hang/scheduling the
work and executing the work. At least one such case I could trigger is
the simulated reset via the i915_wedged debugfs entry. Fix this by
getting an RPM reference around accessing the HW in the reset work.
v2:
- Instead of getting/putting the RPM reference in the reset work itself,
get it already before scheduling the work. By this we also prevent
going to D3 before the work gets to run, in addition to making sure
that we run the work itself in D0. (Ville, Daniel)
v3:
- fix inverted logic fail when putting the RPM ref on behalf of a
cancelled GPU reset work (Ville)
v4:
- Taking the RPM ref in the interrupt handler isn't really needed b/c
it's already guaranteed that we hold an RPM ref until the end of the
reset work in all cases we care about. So take the ref in the reset
work (for cases like i915_wedged_set). (Daniel)
Signed-off-by: Imre Deak imre.d...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a651d0d..0e47111 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2175,6 +2175,14 @@ static void i915_error_work_func(struct work_struct
*work)
reset_event);
/*
+ * In most cases it's guaranteed that we get here with an RPM
+ * reference held, for example because there is a pending GPU
+ * request that won't finish until the reset is done. This
+ * isn't the case at least when we get here by doing a
+ * simulated reset via debugs, so get an RPM reference.
^debugfs
Also maybe ..., so get a RPM reference just to quiet the warnings.
-Daniel
+ */
+ intel_runtime_pm_get(dev_priv);
+ /*
* All state reset _must_ be completed before we update the
* reset counter, for otherwise waiters might miss the reset
* pending state and not properly drop locks, resulting in
@@ -2184,6 +2192,8 @@ static void i915_error_work_func(struct work_struct
*work)
intel_display_handle_reset(dev);
+ intel_runtime_pm_put(dev_priv);
+
if (ret == 0) {
/*
* After all the gem state is reset, increment the reset
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Discard BIOS framebuffers too small to accommodate chosen mode
On Tue, Apr 22, 2014 at 10:05:58PM +0100, Chris Wilson wrote:
If the inherited BIOS framebuffer is smaller than the mode selected for
fbdev, then if we continue to use it then we cause display corruption as
we do not setup the panel fitter to upscale.
Regression from commit d978ef14456a38034f6c0e94a794129501f89200
Author: Jesse Barnes jbar...@virtuousgeek.org
Date: Fri Mar 7 08:57:51 2014 -0800
drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS
fbcon v12
Reported-by: Knut Petersen knut_peter...@t-online.de
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77767
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/intel_fbdev.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index b16116db6c37..28220ca838df 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -133,6 +133,12 @@ static int intelfb_create(struct drm_fb_helper *helper,
mutex_lock(dev-struct_mutex);
+ if (intel_fb
+ (sizes-fb_width intel_fb-base.width ||
+ sizes-fb_height intel_fb-base.height)) {
+ drm_framebuffer_reference(ifbdev-fb-base);
+ intel_fb = ifbdev-fb = NULL;
Hm, maybe add a
DRM_DEBUG_KMS(BIOS fb to small, releasing it\n);
here. With or without this is
Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
+ }
if (!intel_fb || WARN_ON(!intel_fb-obj)) {
DRM_DEBUG_KMS(no BIOS fb, allocating a new one\n);
ret = intelfb_alloc(helper, sizes);
--
1.9.2
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm: Introduce drm_fb_helper_prepare()
On Tue, Apr 22, 2014 at 05:54:06PM +0200, Daniel Vetter wrote: On Tue, Apr 22, 2014 at 04:42:20PM +0200, Thierry Reding wrote: [...] diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c [...] @@ -502,6 +503,33 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) } /** + * drm_fb_helper_prepare - setup a drm_fb_helper structure + * @dev: DRM device + * @helper: driver-allocated fbdev helper structure to set up + * @funcs: pointer to structure of functions associate with this helper + * + * Sets up the bare minimum to make the framebuffer helper usable. This is + * useful to implement race-free initialization of the polling helpers. In + * that case a typical sequence would be: + * + * - call drm_fb_helper_prepare() + * - set dev-mode_config.funcs This step is done in drm_fb_helper_prepare already. drm_fb_helper_prepare() sets fb_helper-funcs. dev-mode_config.funcs needs to be set because it gets called by drm_kms_helper_hotplug_event() which in turn is called by output_poll_execute(), which can be called at any point after drm_kms_helper_poll_init(). It could be scheduled immediately by drm_kms_helper_poll_enable(). I wonder if perhaps we should be wrapping that into a function as well. Currently this is only documented in code by the drivers that call this. But since it's only a single step it doesn't seem worth it. Perhaps if we rolled the min/max_width/height into that function as well it would be more worth it? That could be difficult to do since a couple of drivers need to set those depending on the chipset generation. + * - perform driver-specific setup + * - call drm_kms_helper_poll_init() Maybe mention that after this you can start to handle hpd events using the probe helpers? Isn't that somewhat implied already? The poll helpers call directly the dev-mode_config.funcs-output_poll_changed() function, which has already been set up earlier. + * - call drm_fb_helper_init() + * - call drm_fb_helper_single_add_all_connectors() + * - call drm_fb_helper_initial_config() Does this list look sane in the generated DocBook? Afaik kerneldoc unfortunately lacks any form of markup, at least afaik :( I must admit I didn't check. I'll make sure I do that before sending out the next version. Thierry pgppQ9Eb9BdhW.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't check gmch state on inherited configs
On Sun, Apr 13, 2014 at 12:00:33PM +0200, Daniel Vetter wrote:
... our current modeset code isn't good enough yet to handle this. The
scenario is:
1. BIOS sets up a cloned config with lvds+external screen on the same
pipe, e.g. pipe B.
2. We read out that state for pipe B and assign the gmch_pfit state to
it.
3. The initial modeset switches the lvds to pipe A but due to lack of
atomic modeset we don't recompute the config of pipe B.
- both pipes now claim (in the sw pipe config structure) to use the
gmch_pfit, which just won't work.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74081
Tested-by: max maniku...@gmail.com
Cc: Alan Stern st...@rowland.harvard.edu
Cc: sta...@vger.kernel.org
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_display.c | 23 ++-
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 1390ab5e00dc..7b7987dc65ba 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9655,11 +9655,22 @@ intel_pipe_config_compare(struct drm_device *dev,
PIPE_CONF_CHECK_I(pipe_src_w);
PIPE_CONF_CHECK_I(pipe_src_h);
- PIPE_CONF_CHECK_I(gmch_pfit.control);
- /* pfit ratios are autocomputed by the hw on gen4+ */
- if (INTEL_INFO(dev)-gen 4)
- PIPE_CONF_CHECK_I(gmch_pfit.pgm_ratios);
- PIPE_CONF_CHECK_I(gmch_pfit.lvds_border_bits);
+ /*
+ * FIXME: BIOS likes to set up a cloned config with lvds+external
+ * screen. Since we don't yet re-compute the pipe config when moving
+ * just the lvds port away to another pipe the sw tracking won't match.
+ *
+ * Proper atomic modesets with recomputed global state will fix this.
+ * Until then just don't check gmch state for inherited modes.
+ */
+ if (!PIPE_CONF_QUIRK(PIPE_CONFIG_QUIRK_INHERITED_MODE)) {
+ PIPE_CONF_CHECK_I(gmch_pfit.control);
+ /* pfit ratios are autocomputed by the hw on gen4+ */
+ if (INTEL_INFO(dev)-gen 4)
+ PIPE_CONF_CHECK_I(gmch_pfit.pgm_ratios);
+ PIPE_CONF_CHECK_I(gmch_pfit.lvds_border_bits);
+ }
+
PIPE_CONF_CHECK_I(pch_pfit.enabled);
if (current_config-pch_pfit.enabled) {
PIPE_CONF_CHECK_I(pch_pfit.pos);
@@ -11618,6 +11629,8 @@ static void intel_modeset_readout_hw_state(struct
drm_device *dev)
base.head) {
memset(crtc-config, 0, sizeof(crtc-config));
+ crtc-config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
+
crtc-active = dev_priv-display.get_pipe_config(crtc,
crtc-config);
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index c551472b892e..b885df150910 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -236,7 +236,8 @@ struct intel_crtc_config {
* tracked with quirk flags so that fastboot and state checker can act
* accordingly.
*/
-#define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS (10) /* unreliable sync
mode.flags */
+#define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS(10) /* unreliable sync
mode.flags */
+#define PIPE_CONFIG_QUIRK_INHERITED_MODE (11) /* mode inherited from
firmware */
unsigned long quirks;
/* User requested mode, only valid as a starting point to
--
1.8.4.rc3
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Sanitize the enable_ppgtt module option once
On Tue, Apr 22, 2014 at 10:22:15PM +0100, Chris Wilson wrote:
On Tue, Apr 22, 2014 at 10:17:50PM +0200, Daniel Vetter wrote:
Otherwise we'll end up spamming dmesg on every context creation on snb
with vt-d enabled. This regression was introduced in
commit 246cbfb5fb9a1ca0997fbb135464c1ff5bb9c549
Author: Ben Widawsky benjamin.widaw...@intel.com
Date: Fri Dec 6 14:11:14 2013 -0800
drm/i915: Reorganize intel_enable_ppgtt
I started to consider what would happen if i915.enable_ppgtt changed on
the fly, but then saw that it is 0400 and this pre-initialisation makes
a lot of sense. So maybe we could mention that here:
As the i915.enable_ppgtt is read-only it cannot be changed after the
module is loaded and so we can perform an early sanitization of the
values.
References: https://lkml.org/lkml/2014/4/17/599
Cc: Alessandro Suardi alessandro.sua...@gmail.com
Cc: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
If you care to add in some of the comments,
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
Fully agreed on both since the first thing I've checked is that the
enable_ppgtt option is indeed 0400 ;-)
Jani, can you please apply Chris' commit message text and comment when
merging?
-Daniel
drivers/gpu/drm/i915/i915_gem_gtt.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0d514ff9b94c..47491c4a1181 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -34,25 +34,35 @@ static void gen8_setup_private_ppat(struct
drm_i915_private *dev_priv);
bool intel_enable_ppgtt(struct drm_device *dev, bool full)
{
- if (i915.enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
+ if (i915.enable_ppgtt == 0)
return false;
if (i915.enable_ppgtt == 1 full)
return false;
+ return true;
+}
+
/* i915.enable_ppgtt is read-only, so do an early pass to validate
* the user's requested state against the hardware/driver capabilities.
* We do this now so that we can print out any log messages once rather
* than every time we check intel_enable_ppgtt().
*/
+static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
+{
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm: Introduce drm_fb_helper_prepare()
On Wed, Apr 23, 2014 at 09:14:41AM +0200, Thierry Reding wrote: On Tue, Apr 22, 2014 at 05:54:06PM +0200, Daniel Vetter wrote: On Tue, Apr 22, 2014 at 04:42:20PM +0200, Thierry Reding wrote: [...] diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c [...] @@ -502,6 +503,33 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) } /** + * drm_fb_helper_prepare - setup a drm_fb_helper structure + * @dev: DRM device + * @helper: driver-allocated fbdev helper structure to set up + * @funcs: pointer to structure of functions associate with this helper + * + * Sets up the bare minimum to make the framebuffer helper usable. This is + * useful to implement race-free initialization of the polling helpers. In + * that case a typical sequence would be: + * + * - call drm_fb_helper_prepare() + * - set dev-mode_config.funcs This step is done in drm_fb_helper_prepare already. drm_fb_helper_prepare() sets fb_helper-funcs. dev-mode_config.funcs needs to be set because it gets called by drm_kms_helper_hotplug_event() which in turn is called by output_poll_execute(), which can be called at any point after drm_kms_helper_poll_init(). It could be scheduled immediately by drm_kms_helper_poll_enable(). I wonder if perhaps we should be wrapping that into a function as well. Currently this is only documented in code by the drivers that call this. But since it's only a single step it doesn't seem worth it. Perhaps if we rolled the min/max_width/height into that function as well it would be more worth it? That could be difficult to do since a couple of drivers need to set those depending on the chipset generation. Oh I've misread this step for the fb_helper-funcs assignment. Makes sense and I don't think we need to augment your kerneldoc more to explain this. + * - perform driver-specific setup Hm, maybe clarify this as initialize modeset objects like crtcs, encoders and connectors? Since that's the important part we need to get done here. + * - call drm_kms_helper_poll_init() Maybe mention that after this you can start to handle hpd events using the probe helpers? Isn't that somewhat implied already? The poll helpers call directly the dev-mode_config.funcs-output_poll_changed() function, which has already been set up earlier. I've more meant that after this it's save for drivers to enable hpd interrupts and start to process them. I.e. - enable interrupts and start processing hpd events as an additional step to make it really cleear how it all fits together. Otherwise driver authors are left wondering wtf this isn't just one function call to do it all for them ;-) + * - call drm_fb_helper_init() + * - call drm_fb_helper_single_add_all_connectors() + * - call drm_fb_helper_initial_config() Does this list look sane in the generated DocBook? Afaik kerneldoc unfortunately lacks any form of markup, at least afaik :( I must admit I didn't check. I'll make sure I do that before sending out the next version. If it looks ugly I'm ok as-is, just wondered. Kerneldoc is a bit simplistic ime. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Discard BIOS framebuffers too small to accommodate chosen mode
If the inherited BIOS framebuffer is smaller than the mode selected for
fbdev, then if we continue to use it then we cause display corruption as
we do not setup the panel fitter to upscale.
Regression from commit d978ef14456a38034f6c0e94a794129501f89200
Author: Jesse Barnes jbar...@virtuousgeek.org
Date: Fri Mar 7 08:57:51 2014 -0800
drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon
v12
v2: Add a debug message to track the discard of the BIOS fb.
Reported-by: Knut Petersen knut_peter...@t-online.de
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77767
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Jesse Barnes jbar...@virtuousgeek.org
Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_fbdev.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index b16116db6c37..dbd3f7ce6354 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -133,6 +133,16 @@ static int intelfb_create(struct drm_fb_helper *helper,
mutex_lock(dev-struct_mutex);
+ if (intel_fb
+ (sizes-fb_width intel_fb-base.width ||
+sizes-fb_height intel_fb-base.height)) {
+ DRM_DEBUG_KMS(BIOS fb too small (%dx%d), we require (%dx%d),
+ releasing it\n,
+ intel_fb-base.width, intel_fb-base.height,
+ sizes-fb_width, sizes-fb_height);
+ drm_framebuffer_reference(ifbdev-fb-base);
+ intel_fb = ifbdev-fb = NULL;
+ }
if (!intel_fb || WARN_ON(!intel_fb-obj)) {
DRM_DEBUG_KMS(no BIOS fb, allocating a new one\n);
ret = intelfb_alloc(helper, sizes);
--
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: Discard BIOS framebuffers too small to accommodate chosen mode
On Tue, Apr 22, 2014 at 10:05:58PM +0100, Chris Wilson wrote:
If the inherited BIOS framebuffer is smaller than the mode selected for
fbdev, then if we continue to use it then we cause display corruption as
we do not setup the panel fitter to upscale.
Regression from commit d978ef14456a38034f6c0e94a794129501f89200
Author: Jesse Barnes jbar...@virtuousgeek.org
Date: Fri Mar 7 08:57:51 2014 -0800
drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS
fbcon v12
Reported-by: Knut Petersen knut_peter...@t-online.de
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77767
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/intel_fbdev.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index b16116db6c37..28220ca838df 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -133,6 +133,12 @@ static int intelfb_create(struct drm_fb_helper *helper,
mutex_lock(dev-struct_mutex);
+ if (intel_fb
+ (sizes-fb_width intel_fb-base.width ||
+ sizes-fb_height intel_fb-base.height)) {
+ drm_framebuffer_reference(ifbdev-fb-base);
unreference
I know intel_fb == ifbdev-fb, but still I think it would look a bit
less confusing if you passed intel_fb-base to
drm_framebuffer_unreference() instead of ifbdev-fb-base. Simply
because you use intel_fb-base in the size checks just before.
+ intel_fb = ifbdev-fb = NULL;
+ }
if (!intel_fb || WARN_ON(!intel_fb-obj)) {
DRM_DEBUG_KMS(no BIOS fb, allocating a new one\n);
ret = intelfb_alloc(helper, sizes);
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915: get a runtime PM ref for the deferred GPU reset work
On Wed, 2014-04-23 at 09:07 +0200, Daniel Vetter wrote:
On Wed, Apr 23, 2014 at 01:13:40AM +0300, Imre Deak wrote:
Atm we can end up in the GPU reset deferred work in D3 state if the last
runtime PM reference is dropped between detecting a hang/scheduling the
work and executing the work. At least one such case I could trigger is
the simulated reset via the i915_wedged debugfs entry. Fix this by
getting an RPM reference around accessing the HW in the reset work.
v2:
- Instead of getting/putting the RPM reference in the reset work itself,
get it already before scheduling the work. By this we also prevent
going to D3 before the work gets to run, in addition to making sure
that we run the work itself in D0. (Ville, Daniel)
v3:
- fix inverted logic fail when putting the RPM ref on behalf of a
cancelled GPU reset work (Ville)
v4:
- Taking the RPM ref in the interrupt handler isn't really needed b/c
it's already guaranteed that we hold an RPM ref until the end of the
reset work in all cases we care about. So take the ref in the reset
work (for cases like i915_wedged_set). (Daniel)
Signed-off-by: Imre Deak imre.d...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index a651d0d..0e47111 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2175,6 +2175,14 @@ static void i915_error_work_func(struct work_struct
*work)
reset_event);
/*
+* In most cases it's guaranteed that we get here with an RPM
+* reference held, for example because there is a pending GPU
+* request that won't finish until the reset is done. This
+* isn't the case at least when we get here by doing a
+* simulated reset via debugs, so get an RPM reference.
^debugfs
Also maybe ..., so get a RPM reference just to quiet the warnings.
Do you mean it's not a problem to perform the reset in D3 state other
than getting warnings?
--Imre
-Daniel
+*/
+ intel_runtime_pm_get(dev_priv);
+ /*
* All state reset _must_ be completed before we update the
* reset counter, for otherwise waiters might miss the reset
* pending state and not properly drop locks, resulting in
@@ -2184,6 +2192,8 @@ static void i915_error_work_func(struct work_struct
*work)
intel_display_handle_reset(dev);
+ intel_runtime_pm_put(dev_priv);
+
if (ret == 0) {
/*
* After all the gem state is reset, increment the reset
--
1.8.4
___
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 v3 11/25] drm/i915: add missing error capturing of the PIPESTAT reg
On Fri, Apr 18, 2014 at 03:55:04PM +0300, Imre Deak wrote:
While checking the error capture path I noticed that we lacked the
power domain-on check for PIPESTAT so fix this by moving that to where
the rest of pipe registers are captured.
The move also revealed that we actually don't include this register in
the error report, so fix that too.
v2:
- patch introduced in v2 of the patchset
v3:
- add back !HAS_PCH_SPLIT check (Ville)
Signed-off-by: Imre Deak imre.d...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
[ Ignore my previous comment about the gen=5 || vlv check, I realized
that it's the same as !HAS_PCH_SPLIT. ]
---
drivers/gpu/drm/i915/i915_drv.h | 1 -
drivers/gpu/drm/i915/i915_gpu_error.c | 3 ---
drivers/gpu/drm/i915/intel_display.c | 5 +
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7d6acb4..5254f4b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -325,7 +325,6 @@ struct drm_i915_error_state {
u32 gab_ctl;
u32 gfx_mode;
u32 extra_instdone[I915_NUM_INSTDONE_REG];
- u32 pipestat[I915_MAX_PIPES];
u64 fence[I915_MAX_NUM_FENCES];
struct intel_overlay_error_state *overlay;
struct intel_display_error_state *display;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index ba79b59..7b5cc08 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1028,7 +1028,6 @@ static void i915_capture_reg_state(struct
drm_i915_private *dev_priv,
struct drm_i915_error_state *error)
{
struct drm_device *dev = dev_priv-dev;
- int pipe;
/* General organization
* 1. Registers specific to a single generation
@@ -1080,8 +1079,6 @@ static void i915_capture_reg_state(struct
drm_i915_private *dev_priv,
error-ier = I915_READ16(IER);
else
error-ier = I915_READ(IER);
- for_each_pipe(pipe)
- error-pipestat[pipe] = I915_READ(PIPESTAT(pipe));
}
/* 4: Everything else */
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index cd68a24..a2f3790 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11901,6 +11901,7 @@ struct intel_display_error_state {
struct intel_pipe_error_state {
bool power_domain_on;
u32 source;
+ u32 stat;
} pipe[I915_MAX_PIPES];
struct intel_plane_error_state {
@@ -11982,6 +11983,9 @@ intel_display_capture_error_state(struct drm_device
*dev)
}
error-pipe[i].source = I915_READ(PIPESRC(i));
+
+ if (!HAS_PCH_SPLIT(dev))
+ error-pipe[i].stat = I915_READ(PIPESTAT(i));
}
error-num_transcoders = INTEL_INFO(dev)-num_pipes;
@@ -12032,6 +12036,7 @@ intel_display_print_error_state(struct
drm_i915_error_state_buf *m,
err_printf(m, Power: %s\n,
error-pipe[i].power_domain_on ? on : off);
err_printf(m, SRC: %08x\n, error-pipe[i].source);
+ err_printf(m, STAT: %08x\n, error-pipe[i].stat);
err_printf(m, Plane [%d]:\n, i);
err_printf(m, CNTR: %08x\n, error-plane[i].control);
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Discard BIOS framebuffers too small to accommodate chosen mode
If the inherited BIOS framebuffer is smaller than the mode selected for
fbdev, then if we continue to use it then we cause display corruption as
we do not setup the panel fitter to upscale.
Regression from commit d978ef14456a38034f6c0e94a794129501f89200
Author: Jesse Barnes jbar...@virtuousgeek.org
Date: Fri Mar 7 08:57:51 2014 -0800
drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon
v12
v2: Add a debug message to track the discard of the BIOS fb.
v3: Ville pointed out the difference between ref/unref
Reported-by: Knut Petersen knut_peter...@t-online.de
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77767
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Jesse Barnes jbar...@virtuousgeek.org
Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_fbdev.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index b16116db6c37..fbe7941f88c8 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -133,6 +133,16 @@ static int intelfb_create(struct drm_fb_helper *helper,
mutex_lock(dev-struct_mutex);
+ if (intel_fb
+ (sizes-fb_width intel_fb-base.width ||
+sizes-fb_height intel_fb-base.height)) {
+ DRM_DEBUG_KMS(BIOS fb too small (%dx%d), we require (%dx%d),
+ releasing it\n,
+ intel_fb-base.width, intel_fb-base.height,
+ sizes-fb_width, sizes-fb_height);
+ drm_framebuffer_unreference(intel_fb-base);
+ intel_fb = ifbdev-fb = NULL;
+ }
if (!intel_fb || WARN_ON(!intel_fb-obj)) {
DRM_DEBUG_KMS(no BIOS fb, allocating a new one\n);
ret = intelfb_alloc(helper, sizes);
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 14/25] drm/i915: sanitize enable_rc6 option
On Fri, Apr 18, 2014 at 04:01:02PM +0300, Imre Deak wrote:
Atm, an invalid enable_rc6 module option will be silently ignored, so
emit an info message about it. Doing an early sanitization we can also
reuse intel_enable_rc6() in a follow-up patch to see if RC6 is actually
enabled. Currently the caller would have to filter a non-zero return
value based on the platform we are running on. For example on VLV with
i915.enable_rc6 set to 2, RC6 won't be enabled but atm
intel_enable_rc6() would still return 2 in this case.
v2:
- simplify the platform check condition (Ville)
Signed-off-by: Imre Deak imre.d...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 30 +++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a56f6b1..075405a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3262,15 +3262,32 @@ static void intel_print_rc6_info(struct drm_device
*dev, u32 mode)
(mode GEN6_RC_CTL_RC6pp_ENABLE) ? on : off);
}
-int intel_enable_rc6(const struct drm_device *dev)
+static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
{
/* No RC6 before Ironlake */
if (INTEL_INFO(dev)-gen 5)
return 0;
+ /* RC6 is only on Ironlake mobile not on desktop */
+ if (INTEL_INFO(dev)-gen == 5 !IS_IRONLAKE_M(dev))
+ return 0;
+
/* Respect the kernel parameter if it is set */
- if (i915.enable_rc6 = 0)
- return i915.enable_rc6;
+ if (enable_rc6 = 0) {
+ int mask;
+
+ if (INTEL_INFO(dev)-gen == 6 || IS_IVYBRIDGE(dev))
+ mask = INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE |
+INTEL_RC6pp_ENABLE;
+ else
+ mask = INTEL_RC6_ENABLE;
+
+ if ((enable_rc6 mask) != enable_rc6)
+ DRM_INFO(Adjusting RC6 mask to %d (requested %d, valid
%d)\n,
+ enable_rc6, enable_rc6 mask, mask);
+
+ return enable_rc6 mask;
+ }
/* Disable RC6 on Ironlake */
if (INTEL_INFO(dev)-gen == 5)
@@ -3282,6 +3299,11 @@ int intel_enable_rc6(const struct drm_device *dev)
return INTEL_RC6_ENABLE;
}
+int intel_enable_rc6(const struct drm_device *dev)
+{
+ return i915.enable_rc6;
+}
+
static void gen6_enable_rps_interrupts(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -4496,6 +4518,8 @@ static void intel_init_emon(struct drm_device *dev)
void intel_init_gt_powersave(struct drm_device *dev)
{
+ i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
+
if (IS_VALLEYVIEW(dev))
valleyview_setup_pctx(dev);
}
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 17/25] drm/i915: factor out gen6_update_ring_freq
On Fri, Apr 18, 2014 at 04:16:23PM +0300, Imre Deak wrote:
This is needed by the next patch moving the call out from platform
specific RPM callbacks to platform independent code.
No functional change.
v2:
- patch introduce in v2 of the patchset
v3:
- simplify platform check condition (Ville)
Signed-off-by: Imre Deak imre.d...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_drv.c | 2 --
drivers/gpu/drm/i915/intel_display.c | 2 --
drivers/gpu/drm/i915/intel_pm.c | 18 +++---
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f3f9a33..afc31e3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -899,9 +899,7 @@ static void snb_runtime_resume(struct drm_i915_private
*dev_priv)
intel_init_pch_refclk(dev);
i915_gem_init_swizzling(dev);
- mutex_lock(dev_priv-rps.hw_lock);
gen6_update_ring_freq(dev);
- mutex_unlock(dev_priv-rps.hw_lock);
}
static void hsw_runtime_resume(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index a902e13..bb7671d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7065,9 +7065,7 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv)
intel_prepare_ddi(dev);
i915_gem_init_swizzling(dev);
- mutex_lock(dev_priv-rps.hw_lock);
gen6_update_ring_freq(dev);
- mutex_unlock(dev_priv-rps.hw_lock);
}
static void snb_modeset_global_resources(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4e30b15..46f7b1a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3525,7 +3525,7 @@ static void gen6_enable_rps(struct drm_device *dev)
gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
}
-void gen6_update_ring_freq(struct drm_device *dev)
+static void __gen6_update_ring_freq(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
int min_freq = 15;
@@ -3595,6 +3595,18 @@ void gen6_update_ring_freq(struct drm_device *dev)
}
}
+void gen6_update_ring_freq(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (INTEL_INFO(dev)-gen 6 || IS_VALLEYVIEW(dev))
+ return;
+
+ mutex_lock(dev_priv-rps.hw_lock);
+ __gen6_update_ring_freq(dev);
+ mutex_unlock(dev_priv-rps.hw_lock);
+}
+
int valleyview_rps_max_freq(struct drm_i915_private *dev_priv)
{
u32 val, rp0;
@@ -4566,10 +4578,10 @@ static void intel_gen6_powersave_work(struct
work_struct *work)
valleyview_enable_rps(dev);
} else if (IS_BROADWELL(dev)) {
gen8_enable_rps(dev);
- gen6_update_ring_freq(dev);
+ __gen6_update_ring_freq(dev);
} else {
gen6_enable_rps(dev);
- gen6_update_ring_freq(dev);
+ __gen6_update_ring_freq(dev);
}
dev_priv-rps.enabled = true;
mutex_unlock(dev_priv-rps.hw_lock);
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 19/25] drm/i915: reinit GT power save during resume
On Tue, Apr 22, 2014 at 08:21:07PM +0300, Imre Deak wrote: During runtime suspend there can be a last pending rps.work, so make sure it's canceled. Note that in the runtime suspend callback we can't get any RPS interrupts since it's called only after the GPU goes idle and we set the minimum RPS frequency. The next possibility for an RPS interrupt is only after getting an RPM ref (for example because of a new GPU command) and calling the RPM resume callback. v2: - patch introduced in v2 of the patchset v3: - Change the order of canceling the rps.work and disabling interrupts to avoid the race between interrupt disabling and the the rps.work. Race spotted by Ville. Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index b87109c..edd4ab8 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -919,6 +919,12 @@ static int intel_runtime_suspend(struct device *device) DRM_DEBUG_KMS(Suspending device\n); + /* + * rps.work can't be rearmed here, since we get here only after making + * sure the GPU is idle and the RPS freq is set to the minimum. See + * intel_mark_idle(). + */ + cancel_work_sync(dev_priv-rps.work); Yeah makes sense. Well, unless the hardware is bonkers and generates an UP interrupt while idle. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com intel_runtime_pm_disable_interrupts(dev); if (IS_GEN6(dev)) @@ -970,6 +976,7 @@ static int intel_runtime_resume(struct device *device) gen6_update_ring_freq(dev); intel_runtime_pm_restore_interrupts(dev); + intel_reset_gt_powersave(dev); DRM_DEBUG_KMS(Device resumed\n); return 0; -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 21/25] drm/i915: vlv: factor out vlv_force_gfx_clock and check for pending force-off
On Fri, Apr 18, 2014 at 04:35:02PM +0300, Imre Deak wrote:
This will be needed by the VLV runtime PM helpers too, so factor it out.
Also add a safety check for the case where the previous force-off is
still pending, since I'm not sure if Punit can handle a new setting
while the previous one hasn't settled yet.
v2:
- unchanged
v3:
- add a note to the commit message about the safety check (Ville)
Signed-off-by: Imre Deak imre.d...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_drv.c | 37 +
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 16 ++--
3 files changed, 40 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1f88917..795caea 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -905,6 +905,43 @@ static void hsw_runtime_resume(struct drm_i915_private
*dev_priv)
hsw_disable_pc8(dev_priv);
}
+int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
+{
+ u32 val;
+ int err;
+
+ val = I915_READ(VLV_GTLC_SURVIVABILITY_REG);
+ WARN_ON(!!(val VLV_GFX_CLK_FORCE_ON_BIT) == force_on);
+
+#define COND (I915_READ(VLV_GTLC_SURVIVABILITY_REG) VLV_GFX_CLK_STATUS_BIT)
+ /* Wait for a previous force-off to settle */
+ if (force_on) {
+ err = wait_for(!COND, 5);
+ if (err) {
+ DRM_ERROR(timeout waiting for GFX clock force-off
(%08x)\n,
+ I915_READ(VLV_GTLC_SURVIVABILITY_REG));
+ return err;
+ }
+ }
+
+ val = I915_READ(VLV_GTLC_SURVIVABILITY_REG);
+ val = ~VLV_GFX_CLK_FORCE_ON_BIT;
+ if (force_on)
+ val |= VLV_GFX_CLK_FORCE_ON_BIT;
+ I915_WRITE(VLV_GTLC_SURVIVABILITY_REG, val);
+
+ if (!force_on)
+ return 0;
+
+ err = wait_for(COND, 5);
+ if (err)
+ DRM_ERROR(timeout waiting for GFX clock force-on (%08x)\n,
+ I915_READ(VLV_GTLC_SURVIVABILITY_REG));
+
+ return err;
+#undef COND
+}
+
static int intel_runtime_suspend(struct device *device)
{
struct pci_dev *pdev = to_pci_dev(device);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5254f4b..3cac434 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1968,6 +1968,7 @@ extern unsigned long i915_chipset_val(struct
drm_i915_private *dev_priv);
extern unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
extern unsigned long i915_gfx_val(struct drm_i915_private *dev_priv);
extern void i915_update_gfx_val(struct drm_i915_private *dev_priv);
+int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on);
extern void intel_console_resume(struct work_struct *work);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c45e5c1..d64ac32 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3129,16 +3129,7 @@ static void vlv_set_rps_idle(struct drm_i915_private
*dev_priv)
/* Mask turbo interrupt so that they will not come in between */
I915_WRITE(GEN6_PMINTRMSK, 0x);
- /* Bring up the Gfx clock */
- I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
- I915_READ(VLV_GTLC_SURVIVABILITY_REG) |
- VLV_GFX_CLK_FORCE_ON_BIT);
-
- if (wait_for(((VLV_GFX_CLK_STATUS_BIT
- I915_READ(VLV_GTLC_SURVIVABILITY_REG)) != 0), 5)) {
- DRM_ERROR(GFX_CLK_ON request timed out\n);
- return;
- }
+ vlv_force_gfx_clock(dev_priv, true);
dev_priv-rps.cur_freq = dev_priv-rps.min_freq_softlimit;
@@ -3149,10 +3140,7 @@ static void vlv_set_rps_idle(struct drm_i915_private
*dev_priv)
GENFREQSTATUS) == 0, 5))
DRM_ERROR(timed out waiting for Punit\n);
- /* Release the Gfx clock */
- I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
- I915_READ(VLV_GTLC_SURVIVABILITY_REG)
- ~VLV_GFX_CLK_FORCE_ON_BIT);
+ vlv_force_gfx_clock(dev_priv, false);
I915_WRITE(GEN6_PMINTRMSK,
gen6_rps_pm_mask(dev_priv, dev_priv-rps.cur_freq));
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't check gmch state on inherited configs
On Sun, Apr 13, 2014 at 12:00:33PM +0200, Daniel Vetter wrote:
... our current modeset code isn't good enough yet to handle this. The
scenario is:
1. BIOS sets up a cloned config with lvds+external screen on the same
pipe, e.g. pipe B.
2. We read out that state for pipe B and assign the gmch_pfit state to
it.
3. The initial modeset switches the lvds to pipe A but due to lack of
atomic modeset we don't recompute the config of pipe B.
- both pipes now claim (in the sw pipe config structure) to use the
gmch_pfit, which just won't work.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74081
Cc: Alan Stern st...@rowland.harvard.edu
Cc: sta...@vger.kernel.org
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
Right. Until we get atomic modeset this seems like the least hackish
thing to do.
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 23 ++-
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 1390ab5e00dc..7b7987dc65ba 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9655,11 +9655,22 @@ intel_pipe_config_compare(struct drm_device *dev,
PIPE_CONF_CHECK_I(pipe_src_w);
PIPE_CONF_CHECK_I(pipe_src_h);
- PIPE_CONF_CHECK_I(gmch_pfit.control);
- /* pfit ratios are autocomputed by the hw on gen4+ */
- if (INTEL_INFO(dev)-gen 4)
- PIPE_CONF_CHECK_I(gmch_pfit.pgm_ratios);
- PIPE_CONF_CHECK_I(gmch_pfit.lvds_border_bits);
+ /*
+ * FIXME: BIOS likes to set up a cloned config with lvds+external
+ * screen. Since we don't yet re-compute the pipe config when moving
+ * just the lvds port away to another pipe the sw tracking won't match.
+ *
+ * Proper atomic modesets with recomputed global state will fix this.
+ * Until then just don't check gmch state for inherited modes.
+ */
+ if (!PIPE_CONF_QUIRK(PIPE_CONFIG_QUIRK_INHERITED_MODE)) {
+ PIPE_CONF_CHECK_I(gmch_pfit.control);
+ /* pfit ratios are autocomputed by the hw on gen4+ */
+ if (INTEL_INFO(dev)-gen 4)
+ PIPE_CONF_CHECK_I(gmch_pfit.pgm_ratios);
+ PIPE_CONF_CHECK_I(gmch_pfit.lvds_border_bits);
+ }
+
PIPE_CONF_CHECK_I(pch_pfit.enabled);
if (current_config-pch_pfit.enabled) {
PIPE_CONF_CHECK_I(pch_pfit.pos);
@@ -11618,6 +11629,8 @@ static void intel_modeset_readout_hw_state(struct
drm_device *dev)
base.head) {
memset(crtc-config, 0, sizeof(crtc-config));
+ crtc-config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
+
crtc-active = dev_priv-display.get_pipe_config(crtc,
crtc-config);
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index c551472b892e..b885df150910 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -236,7 +236,8 @@ struct intel_crtc_config {
* tracked with quirk flags so that fastboot and state checker can act
* accordingly.
*/
-#define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS (10) /* unreliable sync
mode.flags */
+#define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS(10) /* unreliable sync
mode.flags */
+#define PIPE_CONFIG_QUIRK_INHERITED_MODE (11) /* mode inherited from
firmware */
unsigned long quirks;
/* User requested mode, only valid as a starting point to
--
1.8.4.rc3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Discard BIOS framebuffers too small to accommodate chosen mode
On Wed, Apr 23, 2014 at 08:54:31AM +0100, Chris Wilson wrote:
If the inherited BIOS framebuffer is smaller than the mode selected for
fbdev, then if we continue to use it then we cause display corruption as
we do not setup the panel fitter to upscale.
Regression from commit d978ef14456a38034f6c0e94a794129501f89200
Author: Jesse Barnes jbar...@virtuousgeek.org
Date: Fri Mar 7 08:57:51 2014 -0800
drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS
fbcon v12
v2: Add a debug message to track the discard of the BIOS fb.
v3: Ville pointed out the difference between ref/unref
Reported-by: Knut Petersen knut_peter...@t-online.de
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77767
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Jesse Barnes jbar...@virtuousgeek.org
Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_fbdev.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index b16116db6c37..fbe7941f88c8 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -133,6 +133,16 @@ static int intelfb_create(struct drm_fb_helper *helper,
mutex_lock(dev-struct_mutex);
+ if (intel_fb
+ (sizes-fb_width intel_fb-base.width ||
+ sizes-fb_height intel_fb-base.height)) {
+ DRM_DEBUG_KMS(BIOS fb too small (%dx%d), we require (%dx%d),
+releasing it\n,
+ intel_fb-base.width, intel_fb-base.height,
+ sizes-fb_width, sizes-fb_height);
+ drm_framebuffer_unreference(intel_fb-base);
+ intel_fb = ifbdev-fb = NULL;
+ }
if (!intel_fb || WARN_ON(!intel_fb-obj)) {
DRM_DEBUG_KMS(no BIOS fb, allocating a new one\n);
ret = intelfb_alloc(helper, sizes);
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Sanitize the enable_ppgtt module option once
On Tue, 22 Apr 2014, Daniel Vetter daniel.vet...@ffwll.ch wrote:
Otherwise we'll end up spamming dmesg on every context creation on snb
with vt-d enabled. This regression was introduced in
commit 246cbfb5fb9a1ca0997fbb135464c1ff5bb9c549
Author: Ben Widawsky benjamin.widaw...@intel.com
Date: Fri Dec 6 14:11:14 2013 -0800
drm/i915: Reorganize intel_enable_ppgtt
References: https://lkml.org/lkml/2014/4/17/599
Cc: Alessandro Suardi alessandro.sua...@gmail.com
Cc: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0d514ff9b94c..47491c4a1181 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -34,25 +34,35 @@ static void gen8_setup_private_ppat(struct
drm_i915_private *dev_priv);
bool intel_enable_ppgtt(struct drm_device *dev, bool full)
{
- if (i915.enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
+ if (i915.enable_ppgtt == 0)
return false;
if (i915.enable_ppgtt == 1 full)
return false;
+ return true;
+}
+
+static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
+{
+ if (enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
+ return 0;
+
+ if (i915.enable_ppgtt == 1)
+ return 1;
+
+ if (i915.enable_ppgtt == 2 HAS_PPGTT(dev))
+ return 2;
You should probably either pass enable_ppgtt as parameter and use that
exclusively, or not pass it and refer to i915.enable_ppgtt directly, but
not mix them.
+
#ifdef CONFIG_INTEL_IOMMU
/* Disable ppgtt on SNB if VT-d is on. */
if (INTEL_INFO(dev)-gen == 6 intel_iommu_gfx_mapped) {
DRM_INFO(Disabling PPGTT because VT-d is on\n);
- return false;
+ return 0;
}
#endif
- /* Full ppgtt disabled by default for now due to issues. */
- if (full)
- return HAS_PPGTT(dev) (i915.enable_ppgtt == 2);
- else
- return HAS_ALIASING_PPGTT(dev);
+ return HAS_ALIASING_PPGTT(dev) ? 1 : 0;
This conflicts in -fixes due to
commit 8d214b7d9c45f4af23ce41b2bc74f79c44f760de
Author: Ben Widawsky benjamin.widaw...@linux.intel.com
Date: Mon Mar 24 18:06:00 2014 -0700
drm/i915: Allow full PPGTT with param override
Should I incorporate that in the conflict resolution for simplicity,
letting 3.15 users also play with full ppgtt with the module param?
BR,
Jani.
}
@@ -1157,6 +1167,8 @@ int i915_gem_init_ppgtt(struct drm_device *dev, struct
i915_hw_ppgtt *ppgtt)
struct drm_i915_private *dev_priv = dev-dev_private;
int ret = 0;
+ i915.enable_ppgtt = sanitize_enable_ppgtt(dev, i915.enable_ppgtt);
+
ppgtt-base.dev = dev;
ppgtt-base.scratch = dev_priv-gtt.base.scratch;
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH I-g-t V2 1/2] tests: Add one ring sync case based on multi drm_fd to test ring semaphore sync under multi BSD rings
On Wed, 2014-04-23 at 09:13 +0800, Zhao Yakui wrote:
On Tue, 2014-04-22 at 13:44 -0600, Daniel Vetter wrote:
On Tue, Apr 22, 2014 at 02:52:04PM +0300, Imre Deak wrote:
On Tue, 2014-04-15 at 10:38 +0800, Zhao Yakui wrote:
The Broadwell GT3 machine has two independent BSD rings in kernel
driver while
it is transparent to the user-space driver. In such case it needs to
check
the ring sync between the two BSD rings. At the same time it also needs
to
check the sync among the second BSD ring and the other rings.
Signed-off-by: Zhao Yakui yakui.z...@intel.com
---
tests/Makefile.sources |1 +
tests/gem_multi_bsd_sync_loop.c | 172
+++
2 files changed, 173 insertions(+)
create mode 100644 tests/gem_multi_bsd_sync_loop.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c957ace..7cd9ca8 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -105,6 +105,7 @@ TESTS_progs = \
gem_render_tiled_blits \
gem_ring_sync_copy \
gem_ring_sync_loop \
+ gem_multi_bsd_sync_loop \
gem_seqno_wrap \
gem_set_tiling_vs_gtt \
gem_set_tiling_vs_pwrite \
diff --git a/tests/gem_multi_bsd_sync_loop.c
b/tests/gem_multi_bsd_sync_loop.c
new file mode 100644
index 000..7f5b832
--- /dev/null
+++ b/tests/gem_multi_bsd_sync_loop.c
@@ -0,0 +1,172 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person
obtaining a
+ * copy of this software and associated documentation files (the
Software),
+ * to deal in the Software without restriction, including without
limitation
+ * the rights to use, copy, modify, merge, publish, distribute,
sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom
the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including
the next
+ * paragraph) shall be included in all copies or substantial portions
of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND,
EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Daniel Vetter daniel.vet...@ffwll.ch (based on
gem_ring_sync_loop_*.c)
+ *Zhao Yakui yakui.z...@intel.com
+ *
+ */
+
+#include stdlib.h
+#include stdio.h
+#include string.h
+#include fcntl.h
+#include inttypes.h
+#include errno.h
+#include sys/stat.h
+#include sys/time.h
+#include drm.h
+#include ioctl_wrappers.h
+#include drmtest.h
+#include intel_bufmgr.h
+#include intel_batchbuffer.h
+#include intel_io.h
+#include i830_reg.h
+#include intel_chipset.h
+
+static drm_intel_bufmgr *bufmgr;
+struct intel_batchbuffer *batch;
+static drm_intel_bo *target_buffer;
+
+#define NUM_FD 50
+
+static int mfd[NUM_FD];
+static drm_intel_bufmgr *mbufmgr[NUM_FD];
+static struct intel_batchbuffer *mbatch[NUM_FD];
+static drm_intel_bo *mbuffer[NUM_FD];
+
+
+/*
+ * Testcase: Basic check of ring-ring sync using a dummy reloc
+ *
+ * Extremely efficient at catching missed irqs with semaphores=0 ...
+ */
+
+#define MI_COND_BATCH_BUFFER_END (0x3623 | 1)
+#define MI_DO_COMPARE (121)
+
+static void
+store_dword_loop(int fd)
+{
+ int i;
+ int num_rings = gem_get_num_rings(fd);
+
+ srandom(0xdeadbeef);
+
+ for (i = 0; i SLOW_QUICK(0x10, 10); i++) {
+ int ring, mindex;
+ ring = random() % num_rings + 1;
+ mindex = random() % NUM_FD;
+ batch = mbatch[mindex];
+ if (ring == I915_EXEC_RENDER) {
+ BEGIN_BATCH(4);
+ OUT_BATCH(MI_COND_BATCH_BUFFER_END |
MI_DO_COMPARE);
+ OUT_BATCH(0x); /* compare dword */
+ OUT_RELOC(mbuffer[mindex],
I915_GEM_DOMAIN_RENDER,
+ I915_GEM_DOMAIN_RENDER, 0);
+ OUT_BATCH(MI_NOOP);
+ ADVANCE_BATCH();
+ } else {
+
Re: [Intel-gfx] [PATCH 2/4] drm/i915: add intel_dp_power_get/put
On Tue, Apr 22, 2014 at 07:55:43PM -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
This was suggested by Chris on his review to the first version of
drm/i915: get power domain in case the BIOS enabled eDP VDD. Well,
at least that's what I understood from his comment :)
The downside of this patch is that in a few places we will call
intel_display_port_power_domain() twice instead of once.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 61
-
1 file changed, 29 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 44df493..b385b03 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1063,13 +1063,30 @@ static u32 ironlake_get_pp_control(struct intel_dp
*intel_dp)
return control;
}
+static void intel_dp_power_get(struct intel_encoder *encoder)
+{
+ struct drm_i915_private *dev_priv = encoder-base.dev-dev_private;
+ enum intel_display_power_domain power_domain;
+
+ power_domain = intel_display_port_power_domain(encoder);
+ intel_display_power_get(dev_priv, power_domain);
+}
+
+static void intel_dp_power_put(struct intel_encoder *encoder)
+{
+ struct drm_i915_private *dev_priv = encoder-base.dev-dev_private;
+ enum intel_display_power_domain power_domain;
+
+ power_domain = intel_display_port_power_domain(encoder);
+ intel_display_power_put(dev_priv, power_domain);
+}
There's nothing DP specific about these functions, so they could be
moved somewhere else and used for other output types as well.
+
static bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct intel_encoder *intel_encoder = intel_dig_port-base;
struct drm_i915_private *dev_priv = dev-dev_private;
- enum intel_display_power_domain power_domain;
u32 pp;
u32 pp_stat_reg, pp_ctrl_reg;
bool need_to_disable = !intel_dp-want_panel_vdd;
@@ -1082,8 +1099,7 @@ static bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
if (edp_have_panel_vdd(intel_dp))
return need_to_disable;
- power_domain = intel_display_port_power_domain(intel_encoder);
- intel_display_power_get(dev_priv, power_domain);
+ intel_dp_power_get(intel_encoder);
DRM_DEBUG_KMS(Turning eDP VDD on\n);
@@ -1133,7 +1149,6 @@ static void edp_panel_vdd_off_sync(struct intel_dp
*intel_dp)
struct intel_digital_port *intel_dig_port =
dp_to_dig_port(intel_dp);
struct intel_encoder *intel_encoder = intel_dig_port-base;
- enum intel_display_power_domain power_domain;
DRM_DEBUG_KMS(Turning eDP VDD off\n);
@@ -1153,8 +1168,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp
*intel_dp)
if ((pp POWER_TARGET_ON) == 0)
intel_dp-last_power_cycle = jiffies;
- power_domain = intel_display_port_power_domain(intel_encoder);
- intel_display_power_put(dev_priv, power_domain);
+ intel_dp_power_put(intel_encoder);
}
}
@@ -1242,7 +1256,6 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
struct intel_encoder *intel_encoder = intel_dig_port-base;
struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct drm_i915_private *dev_priv = dev-dev_private;
- enum intel_display_power_domain power_domain;
u32 pp;
u32 pp_ctrl_reg;
@@ -1272,8 +1285,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
wait_panel_off(intel_dp);
/* We got a reference when we enabled the VDD. */
- power_domain = intel_display_port_power_domain(intel_encoder);
- intel_display_power_put(dev_priv, power_domain);
+ intel_dp_power_put(intel_encoder);
}
void intel_edp_backlight_on(struct intel_dp *intel_dp)
@@ -3161,13 +3173,10 @@ intel_dp_detect(struct drm_connector *connector, bool
force)
struct drm_device *dev = connector-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
enum drm_connector_status status;
- enum intel_display_power_domain power_domain;
struct edid *edid = NULL;
intel_runtime_pm_get(dev_priv);
-
- power_domain = intel_display_port_power_domain(intel_encoder);
- intel_display_power_get(dev_priv, power_domain);
+ intel_dp_power_get(intel_encoder);
DRM_DEBUG_KMS([CONNECTOR:%d:%s]\n,
connector-base.id, drm_get_connector_name(connector));
@@ -3199,8 +3208,7 @@ intel_dp_detect(struct drm_connector *connector, bool
force)
status = connector_status_connected;
out:
- intel_display_power_put(dev_priv,
Re: [Intel-gfx] [PATCH 4/4] drm/i915: remove useless runtime PM get call
On Tue, Apr 22, 2014 at 07:55:45PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com We already call intel_dp_power_get, which will get a power domain, and every power domain should get a runtime PM reference, which will wake up the machine. intel_crt_detect() could use the same treatment Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index a25f708..a0b1cda 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3171,11 +3171,9 @@ intel_dp_detect(struct drm_connector *connector, bool force) struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); struct intel_encoder *intel_encoder = intel_dig_port-base; struct drm_device *dev = connector-dev; - struct drm_i915_private *dev_priv = dev-dev_private; enum drm_connector_status status; struct edid *edid = NULL; - intel_runtime_pm_get(dev_priv); intel_dp_power_get(intel_encoder); DRM_DEBUG_KMS([CONNECTOR:%d:%s]\n, @@ -3209,7 +3207,6 @@ intel_dp_detect(struct drm_connector *connector, bool force) out: intel_dp_power_put(intel_encoder); - intel_runtime_pm_put(dev_priv); return status; } -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2 2/8] kms_cursor_crc: Move cursor enable and disable calls where they belong
On Thu, Apr 10, 2014 at 03:08:06PM +0300, Antti Koskipaa wrote:
We can't have the hw cursor enabled during software render tests.
Signed-off-by: Antti Koskipaa antti.koski...@linux.intel.com
---
tests/kms_cursor_crc.c | 54
--
1 file changed, 26 insertions(+), 28 deletions(-)
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index 52281d0..8802da6 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -67,6 +67,30 @@ static void draw_cursor(cairo_t *cr, int x, int y, int w)
igt_paint_color_alpha(cr, x + w, y + w, w, w, 0.5, 0.5, 0.5, 1.0);
}
+static void cursor_enable(test_data_t *test_data)
+{
+ data_t *data = test_data-data;
+ igt_display_t *display = data-display;
+ igt_output_t *output = test_data-output;
+ igt_plane_t *cursor;
+
+ cursor = igt_output_get_plane(output, IGT_PLANE_CURSOR);
+ igt_plane_set_fb(cursor, data-fb);
+ igt_display_commit(display);
+}
+
+static void cursor_disable(test_data_t *test_data)
+{
+ data_t *data = test_data-data;
+ igt_display_t *display = data-display;
+ igt_output_t *output = test_data-output;
+ igt_plane_t *cursor;
+
+ cursor = igt_output_get_plane(output, IGT_PLANE_CURSOR);
+ igt_plane_set_fb(cursor, NULL);
+ igt_display_commit(display);
+}
+
static igt_pipe_crc_t *create_crc(data_t *data, enum pipe pipe)
{
igt_pipe_crc_t *crc;
@@ -85,10 +109,12 @@ static void do_single_test(test_data_t *test_data, int
x, int y)
printf(.); fflush(stdout);
+ cursor_enable(test_data);
cursor = igt_output_get_plane(test_data-output, IGT_PLANE_CURSOR);
igt_plane_set_position(cursor, x, y);
igt_display_commit(display);
igt_wait_for_vblank(data-drm_fd, test_data-pipe);
+ cursor_disable(test_data);
igt_pipe_crc_collect_crc(pipe_crc, crc);
Cursor is getting disabled before we collect the crc:?
if (test_data-crc_must_match)
@@ -106,30 +132,6 @@ static void do_test(test_data_t *test_data,
do_single_test(test_data, left, bottom);
}
-static void cursor_enable(test_data_t *test_data)
-{
- data_t *data = test_data-data;
- igt_display_t *display = data-display;
- igt_output_t *output = test_data-output;
- igt_plane_t *cursor;
-
- cursor = igt_output_get_plane(output, IGT_PLANE_CURSOR);
- igt_plane_set_fb(cursor, data-fb);
- igt_display_commit(display);
-}
-
-static void cursor_disable(test_data_t *test_data)
-{
- data_t *data = test_data-data;
- igt_display_t *display = data-display;
- igt_output_t *output = test_data-output;
- igt_plane_t *cursor;
-
- cursor = igt_output_get_plane(output, IGT_PLANE_CURSOR);
- igt_plane_set_fb(cursor, NULL);
- igt_display_commit(display);
-}
-
static void test_crc(test_data_t *test_data,
bool onscreen, int cursor_w, int cursor_h)
{
@@ -138,8 +140,6 @@ static void test_crc(test_data_t *test_data,
int top = test_data-top;
int bottom = test_data-bottom;
- cursor_enable(test_data);
-
if (onscreen) {
/* cursor onscreen, crc should match, except when white visible
cursor is used */
test_data-crc_must_match = false;
@@ -183,8 +183,6 @@ static void test_crc(test_data_t *test_data,
/* go nuts */
do_test(test_data, INT_MIN, INT_MAX, INT_MIN, INT_MAX);
}
-
- cursor_disable(test_data);
}
static bool prepare_crtc(test_data_t *test_data, igt_output_t *output,
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915: get power domain in case the BIOS enabled eDP VDD
On Wed, 23 Apr 2014, Paulo Zanoni przan...@gmail.com wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
If I unplug the eDP monitor, the BIOS of my machine will enable the
VDD bit, then when the driver loads it will think VDD is enabled. It
will detect that the eDP is not enabled and return false from
intel_edp_init_connector. This will trigger a call to
edp_panel_vdd_off_sync(), which trigger a WARN saying that the
refcount of the power domain is less than zero.
The problem happens because the driver gets a refcount whenever it
enables the VDD bit, and puts the refcount whenever it disables the
VDD bit. But on this case, the BIOS enabled VDD, so all we do is to
call put() without calling get() first, so the code added is there to
make sure we always have the get() in case the BIOS enabled the bit.
v2: - Rebase
Tested-by: Chris Wilson ch...@chris-wilson.co.uk (v1)
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
Patch 1/4 pushed to -fixes.
BR,
Jani.
---
drivers/gpu/drm/i915/intel_dp.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 80e5598..44df493 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3785,7 +3785,8 @@ static bool intel_edp_init_connector(struct intel_dp
*intel_dp,
{
struct drm_connector *connector = intel_connector-base;
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
- struct drm_device *dev = intel_dig_port-base.base.dev;
+ struct intel_encoder *intel_encoder = intel_dig_port-base;
+ struct drm_device *dev = intel_encoder-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_display_mode *fixed_mode = NULL;
struct drm_display_mode *downclock_mode = NULL;
@@ -3798,6 +3799,14 @@ static bool intel_edp_init_connector(struct intel_dp
*intel_dp,
if (!is_edp(intel_dp))
return true;
+ /* The VDD bit needs a power domain reference, so if the bit is already
+ * enabled when we boot, grab this reference. */
+ if (edp_have_panel_vdd(intel_dp)) {
+ enum intel_display_power_domain power_domain;
+ power_domain = intel_display_port_power_domain(intel_encoder);
+ intel_display_power_get(dev_priv, power_domain);
+ }
+
/* Cache DPCD and EDID for edp. */
intel_edp_panel_vdd_on(intel_dp);
has_dpcd = intel_dp_get_dpcd(intel_dp);
--
1.9.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Discard BIOS framebuffers too small to accommodate chosen mode
On Wed, 23 Apr 2014, Ville Syrjälä ville.syrj...@linux.intel.com wrote:
On Wed, Apr 23, 2014 at 08:54:31AM +0100, Chris Wilson wrote:
If the inherited BIOS framebuffer is smaller than the mode selected for
fbdev, then if we continue to use it then we cause display corruption as
we do not setup the panel fitter to upscale.
Regression from commit d978ef14456a38034f6c0e94a794129501f89200
Author: Jesse Barnes jbar...@virtuousgeek.org
Date: Fri Mar 7 08:57:51 2014 -0800
drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS
fbcon v12
v2: Add a debug message to track the discard of the BIOS fb.
v3: Ville pointed out the difference between ref/unref
Reported-by: Knut Petersen knut_peter...@t-online.de
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77767
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Jesse Barnes jbar...@virtuousgeek.org
Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Pushed to -fixes, thanks for the patch and review.
BR,
Jani.
---
drivers/gpu/drm/i915/intel_fbdev.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index b16116db6c37..fbe7941f88c8 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -133,6 +133,16 @@ static int intelfb_create(struct drm_fb_helper *helper,
mutex_lock(dev-struct_mutex);
+if (intel_fb
+(sizes-fb_width intel_fb-base.width ||
+ sizes-fb_height intel_fb-base.height)) {
+DRM_DEBUG_KMS(BIOS fb too small (%dx%d), we require (%dx%d),
+ releasing it\n,
+ intel_fb-base.width, intel_fb-base.height,
+ sizes-fb_width, sizes-fb_height);
+drm_framebuffer_unreference(intel_fb-base);
+intel_fb = ifbdev-fb = NULL;
+}
if (!intel_fb || WARN_ON(!intel_fb-obj)) {
DRM_DEBUG_KMS(no BIOS fb, allocating a new one\n);
ret = intelfb_alloc(helper, sizes);
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't check gmch state on inherited configs
On Sun, 13 Apr 2014, Daniel Vetter daniel.vet...@ffwll.ch wrote:
... our current modeset code isn't good enough yet to handle this. The
scenario is:
1. BIOS sets up a cloned config with lvds+external screen on the same
pipe, e.g. pipe B.
2. We read out that state for pipe B and assign the gmch_pfit state to
it.
3. The initial modeset switches the lvds to pipe A but due to lack of
atomic modeset we don't recompute the config of pipe B.
- both pipes now claim (in the sw pipe config structure) to use the
gmch_pfit, which just won't work.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74081
Cc: Alan Stern st...@rowland.harvard.edu
Cc: sta...@vger.kernel.org
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
Pushed to -fixes, thanks for the patch and review.
BR,
Jani.
---
drivers/gpu/drm/i915/intel_display.c | 23 ++-
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 1390ab5e00dc..7b7987dc65ba 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9655,11 +9655,22 @@ intel_pipe_config_compare(struct drm_device *dev,
PIPE_CONF_CHECK_I(pipe_src_w);
PIPE_CONF_CHECK_I(pipe_src_h);
- PIPE_CONF_CHECK_I(gmch_pfit.control);
- /* pfit ratios are autocomputed by the hw on gen4+ */
- if (INTEL_INFO(dev)-gen 4)
- PIPE_CONF_CHECK_I(gmch_pfit.pgm_ratios);
- PIPE_CONF_CHECK_I(gmch_pfit.lvds_border_bits);
+ /*
+ * FIXME: BIOS likes to set up a cloned config with lvds+external
+ * screen. Since we don't yet re-compute the pipe config when moving
+ * just the lvds port away to another pipe the sw tracking won't match.
+ *
+ * Proper atomic modesets with recomputed global state will fix this.
+ * Until then just don't check gmch state for inherited modes.
+ */
+ if (!PIPE_CONF_QUIRK(PIPE_CONFIG_QUIRK_INHERITED_MODE)) {
+ PIPE_CONF_CHECK_I(gmch_pfit.control);
+ /* pfit ratios are autocomputed by the hw on gen4+ */
+ if (INTEL_INFO(dev)-gen 4)
+ PIPE_CONF_CHECK_I(gmch_pfit.pgm_ratios);
+ PIPE_CONF_CHECK_I(gmch_pfit.lvds_border_bits);
+ }
+
PIPE_CONF_CHECK_I(pch_pfit.enabled);
if (current_config-pch_pfit.enabled) {
PIPE_CONF_CHECK_I(pch_pfit.pos);
@@ -11618,6 +11629,8 @@ static void intel_modeset_readout_hw_state(struct
drm_device *dev)
base.head) {
memset(crtc-config, 0, sizeof(crtc-config));
+ crtc-config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
+
crtc-active = dev_priv-display.get_pipe_config(crtc,
crtc-config);
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index c551472b892e..b885df150910 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -236,7 +236,8 @@ struct intel_crtc_config {
* tracked with quirk flags so that fastboot and state checker can act
* accordingly.
*/
-#define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS (10) /* unreliable sync
mode.flags */
+#define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS(10) /* unreliable sync
mode.flags */
+#define PIPE_CONFIG_QUIRK_INHERITED_MODE (11) /* mode inherited from
firmware */
unsigned long quirks;
/* User requested mode, only valid as a starting point to
--
1.8.4.rc3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2 7/8] kms_cursor_crc: Add random cursor placement test
On Thu, Apr 10, 2014 at 03:08:11PM +0300, Antti Koskipaa wrote:
Signed-off-by: Antti Koskipaa antti.koski...@linux.intel.com
---
tests/kms_cursor_crc.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index b2498a1..e00abf5 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -214,6 +214,18 @@ static void test_crc_sliding(test_data_t *test_data)
}
}
+static void test_crc_random(test_data_t *test_data)
+{
+ int i;
+
+ /* Random cursor placement */
+ for (i = 0; i 50; i++) {
+ int x = rand() % (test_data-screenw + test_data-curw * 2) -
test_data-curw;
+ int y = rand() % (test_data-screenh + test_data-curh * 2) -
test_data-curh;
+ do_single_test(test_data, x, y);
+ }
+}
As this is not deterministic it would be nice if the test would print
out some of the test parameters on failure (cursor coordinates and size
at least). Otherwise there's no good way to analyze failures.
+
static bool prepare_crtc(test_data_t *test_data, igt_output_t *output,
int cursor_w, int cursor_h)
{
@@ -359,6 +371,8 @@ static void run_test_generic(data_t *data, int
cursor_max_size)
run_test(data, test_crc_offscreen, cursor_size,
cursor_size);
igt_subtest_f(cursor-%s-sliding, c_size)
run_test(data, test_crc_sliding, cursor_size,
cursor_size);
+ igt_subtest_f(cursor-%s-random, c_size)
+ run_test(data, test_crc_random, cursor_size,
cursor_size);
}
}
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] xf86-video-intel: enable hw-generated binding tables
On Tuesday, April 22, 2014 07:20:58 PM Daniel Vetter wrote: On Tue, Apr 22, 2014 at 04:23:12PM +0100, Chris Wilson wrote: On Tue, Apr 22, 2014 at 06:16:34PM +0300, Abdiel Janulgue wrote: This patch series enables resource streamer for xf86-video-intel UXA. Fixes i965 Mesa driver that makes use of resource streamer optimization to survive a full piglit run without bricking the machine. Previously, I get random hungs when doing a full piglit round when enabling RS. After months of head-scratching, I noticed I didn't quite pay attention to this small detail in bspec: The binding table generator feature has a simple all or nothing model. If HW generated binding tables are enabled, the driver must enable the pool and use 3D_HW_BINDING_TABLE_POINTER_* commands. I realized that our xf86-video-intel driver is piping out 3D commands as well that used the older manual-generated binding table format that caused a conflict when Mesa is using the hw-generated binding table format. This has to be per-context right? Otherwise how do you intend to coordinate multiple clients submitting to the kernel using incompatible command sets? Even in the restricted X/DRI sense, how do you intend to negotiate which to use? Hm, I've thought that the MI_BB_START should synchronize with the asynchronous RS parsing/processing? Is there no way to disable RS again for the next batch in a different context? I've already tried disabling RS at the end of every batch so that next batch in different context can continue to use older non-RS format. That does not work either and still causes hangs. What I've seen so far, it seems GPU does not like switching the format of commands from RS-format to non-RS format. It's either one way or the other. If switched on, it affects subsequent contexes henceforth expecting RS-format commands until the GPU gets reset. That's probably the note in bspec: the binding table generator feature has a simple all or nothing model. If there's no way to solve this with contexts or some manual reset trick using LRIs then we're pretty decently screwed up. Worst case we need to stop the gpu and reset it to keep backwards compat :( ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] tests/gem_userptr_benchmark: Benchmarking userptr surfaces and impact
Hi Brad, On 04/18/2014 12:18 AM, Volkin, Bradley D wrote: On Wed, Mar 19, 2014 at 04:13:06AM -0700, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com This adds a small benchmark for the new userptr functionality. Apart from basic surface creation and destruction, also tested is the impact of having userptr surfaces in the process address space. Reason for that is the impact of MMU notifiers on common address space operations like munmap() which is per process. v2: * Moved to benchmarks. * Added pointer read/write tests. * Changed output to say iterations per second instead of operations per second. * Multiply result by batch size for multi-create* tests for a more comparable number with create-destroy test. v3: * Fixed ioctl detection on kernels without MMU_NOTIFIERs. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com Cc: Chris Wilson ch...@chris-wilson.co.uk --- Android.mk | 3 +- benchmarks/.gitignore | 1 + benchmarks/Android.mk | 36 +++ benchmarks/Makefile.am | 7 +- benchmarks/Makefile.sources| 6 + benchmarks/gem_userptr_benchmark.c | 513 + 6 files changed, 558 insertions(+), 8 deletions(-) create mode 100644 benchmarks/Android.mk create mode 100644 benchmarks/Makefile.sources create mode 100644 benchmarks/gem_userptr_benchmark.c diff --git a/Android.mk b/Android.mk index 8aeb2d4..0c969b8 100644 --- a/Android.mk +++ b/Android.mk @@ -1,2 +1 @@ -include $(call all-named-subdir-makefiles, lib tests tools) - +include $(call all-named-subdir-makefiles, lib tests tools benchmarks) diff --git a/benchmarks/.gitignore b/benchmarks/.gitignore index ddea6f7..09e5bd8 100644 --- a/benchmarks/.gitignore +++ b/benchmarks/.gitignore @@ -1,3 +1,4 @@ +gem_userptr_benchmark intel_upload_blit_large intel_upload_blit_large_gtt intel_upload_blit_large_map diff --git a/benchmarks/Android.mk b/benchmarks/Android.mk new file mode 100644 index 000..5bb8ef5 --- /dev/null +++ b/benchmarks/Android.mk @@ -0,0 +1,36 @@ +LOCAL_PATH := $(call my-dir) + +include $(LOCAL_PATH)/Makefile.sources + +## + +define add_benchmark +include $(CLEAR_VARS) + +LOCAL_SRC_FILES := $1.c + +LOCAL_CFLAGS += -DHAVE_STRUCT_SYSINFO_TOTALRAM +LOCAL_CFLAGS += -DANDROID -UNDEBUG -include check-ndebug.h +LOCAL_CFLAGS += -std=c99 +# FIXME: drop once Bionic correctly annotates noreturn on pthread_exit +LOCAL_CFLAGS += -Wno-error=return-type +# Excessive complaining for established cases. Rely on the Linux version warnings. +LOCAL_CFLAGS += -Wno-sign-compare + +LOCAL_MODULE := $1 +LOCAL_MODULE_TAGS := optional + +LOCAL_STATIC_LIBRARIES := libintel_gpu_tools + +LOCAL_SHARED_LIBRARIES := libpciaccess \ + libdrm\ + libdrm_intel + +include $(BUILD_EXECUTABLE) +endef + +## + +benchmark_list := $(bin_PROGRAMS) + +$(foreach item,$(benchmark_list),$(eval $(call add_benchmark,$(item diff --git a/benchmarks/Makefile.am b/benchmarks/Makefile.am index e2ad784..d173bf4 100644 --- a/benchmarks/Makefile.am +++ b/benchmarks/Makefile.am @@ -1,9 +1,4 @@ - -bin_PROGRAMS = \ - intel_upload_blit_large \ - intel_upload_blit_large_gtt \ - intel_upload_blit_large_map \ - intel_upload_blit_small +include Makefile.sources AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources new file mode 100644 index 000..fd6c107 --- /dev/null +++ b/benchmarks/Makefile.sources @@ -0,0 +1,6 @@ +bin_PROGRAMS = \ +intel_upload_blit_large \ +intel_upload_blit_large_gtt \ +intel_upload_blit_large_map \ +intel_upload_blit_small \ +gem_userptr_benchmark You might split the makefile cleanup aspect of this into a separate patch, but I'm fine either way. Good idea, will try to squeeze that in. diff --git a/benchmarks/gem_userptr_benchmark.c b/benchmarks/gem_userptr_benchmark.c new file mode 100644 index 000..218f6f1 --- /dev/null +++ b/benchmarks/gem_userptr_benchmark.c @@ -0,0 +1,513 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next
Re: [Intel-gfx] [PATCH 1/3] tests/gem_userptr_blits: Expanded userptr test cases
On 04/18/2014 06:10 PM, Volkin, Bradley D wrote: On Wed, Mar 19, 2014 at 04:13:04AM -0700, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com A set of userptr test cases to support the new feature. For the eviction and swapping stress testing I have extracted some common behaviour from gem_evict_everything and made both test cases use it to avoid duplicating the code. Both unsynchronized and synchronized userptr objects are tested but the latter set of tests will be skipped if kernel is compiled without MMU_NOTIFIERS. Also, with 32-bit userspace swapping tests are skipped if the system has a lot more RAM than process address space. Forking swapping tests are not skipped since they can still trigger swapping by cumulative effect. v2: * Fixed dmabuf test. * Added test for rejecting read-only. * Fixed ioctl detection for latest kernel patch. v3: * Updated copy() for Gen8+. * Fixed ioctl detection on kernels without MMU_NOTIFIERs. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com A number of the comments I made on patch 3 apply here as well. The sizeof(linear) thing is more prevalent in this test, though it looks like linear is at least used. Other than those comments this looks good to me. Believe it or not that sizeof(linear) idiom I inherited from other blitter tests. Personally I don't care one way or another. But since it makes sense to get rid of it for the benchmark part, perhaps I should change it here as well to be consistent. How strongly do you feel strongly about this? Will see what you reply on the static initializer comment it 3/3, not sure what you meant there. Thanks, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm: Introduce drm_fb_helper_prepare()
On Wed, Apr 23, 2014 at 09:35:48AM +0200, Daniel Vetter wrote: On Wed, Apr 23, 2014 at 09:14:41AM +0200, Thierry Reding wrote: On Tue, Apr 22, 2014 at 05:54:06PM +0200, Daniel Vetter wrote: On Tue, Apr 22, 2014 at 04:42:20PM +0200, Thierry Reding wrote: [...] diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c [...] @@ -502,6 +503,33 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) } /** + * drm_fb_helper_prepare - setup a drm_fb_helper structure + * @dev: DRM device + * @helper: driver-allocated fbdev helper structure to set up + * @funcs: pointer to structure of functions associate with this helper + * + * Sets up the bare minimum to make the framebuffer helper usable. This is + * useful to implement race-free initialization of the polling helpers. In + * that case a typical sequence would be: + * + * - call drm_fb_helper_prepare() + * - set dev-mode_config.funcs This step is done in drm_fb_helper_prepare already. drm_fb_helper_prepare() sets fb_helper-funcs. dev-mode_config.funcs needs to be set because it gets called by drm_kms_helper_hotplug_event() which in turn is called by output_poll_execute(), which can be called at any point after drm_kms_helper_poll_init(). It could be scheduled immediately by drm_kms_helper_poll_enable(). I wonder if perhaps we should be wrapping that into a function as well. Currently this is only documented in code by the drivers that call this. But since it's only a single step it doesn't seem worth it. Perhaps if we rolled the min/max_width/height into that function as well it would be more worth it? That could be difficult to do since a couple of drivers need to set those depending on the chipset generation. Oh I've misread this step for the fb_helper-funcs assignment. Makes sense and I don't think we need to augment your kerneldoc more to explain this. + * - perform driver-specific setup Hm, maybe clarify this as initialize modeset objects like crtcs, encoders and connectors? Since that's the important part we need to get done here. + * - call drm_kms_helper_poll_init() Maybe mention that after this you can start to handle hpd events using the probe helpers? Isn't that somewhat implied already? The poll helpers call directly the dev-mode_config.funcs-output_poll_changed() function, which has already been set up earlier. I've more meant that after this it's save for drivers to enable hpd interrupts and start to process them. I.e. - enable interrupts and start processing hpd events as an additional step to make it really cleear how it all fits together. Otherwise driver authors are left wondering wtf this isn't just one function call to do it all for them ;-) + * - call drm_fb_helper_init() + * - call drm_fb_helper_single_add_all_connectors() + * - call drm_fb_helper_initial_config() Does this list look sane in the generated DocBook? Afaik kerneldoc unfortunately lacks any form of markup, at least afaik :( I must admit I didn't check. I'll make sure I do that before sending out the next version. If it looks ugly I'm ok as-is, just wondered. Kerneldoc is a bit simplistic ime. In the second version I just sent out I ended up moving the description of this sequence into the fbdev helper section rather than keeping it in the description of the drm_fb_helper_prepare() function, since the new function is really just a part of the whole sequence, so it seemed to fit more nicely. And I've dropped the list formatting and turned it into prose instead. Thierry pgp194LSyswFd.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] tests: Skip building kms_flip_tiling on Android
From: Tvrtko Ursulin tvrtko.ursu...@intel.com Dependencies are not available at the moment so it does not build. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com --- tests/Android.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Android.mk b/tests/Android.mk index 9233b2b..6e77e45 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -40,6 +40,7 @@ skip_tests_list := \ kms_addfb \ kms_cursor_crc \ kms_flip \ +kms_flip_tiling \ kms_pipe_crc_basic \ kms_fbc_crc \ kms_render \ -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] tests: Extract ALIGN macro into a common header
From: Tvrtko Ursulin tvrtko.ursu...@intel.com
Makes for a little bit less code duplication, especially since
it will be used from more callers in the future.
Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
---
lib/drmtest.h | 9 +
lib/media_fill_gen7.c | 2 +-
lib/media_fill_gen8.c | 2 +-
lib/rendercopy_gen6.c | 1 -
lib/rendercopy_gen7.c | 1 -
lib/rendercopy_gen8.c | 1 -
6 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/lib/drmtest.h b/lib/drmtest.h
index f3afbaa..84f80dc 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -60,6 +60,15 @@ static inline void *igt_mmap64(void *addr, size_t length,
int prot, int flags,
*/
#define ARRAY_SIZE(arr) (sizeof(arr)/sizeof(arr[0]))
+/**
+ * ALIGN:
+ * @v: value to be aligned
+ * @a: alignment unit in bytes
+ *
+ * Macro to align a value @v to a specified unit @a.
+ */
+#define ALIGN(v, a) (((v) + (a)-1) ~((a)-1))
+
int drm_get_card(void);
int drm_open_any(void);
int drm_open_any_render(void);
diff --git a/lib/media_fill_gen7.c b/lib/media_fill_gen7.c
index cdf4b60..82c3469 100644
--- a/lib/media_fill_gen7.c
+++ b/lib/media_fill_gen7.c
@@ -4,10 +4,10 @@
#include media_fill.h
#include gen7_media.h
#include intel_reg.h
+#include drmtest.h
#include assert.h
-#define ALIGN(x, y) (((x) + (y)-1) ~((y)-1))
static const uint32_t media_kernel[][4] = {
{ 0x0041, 0x20200231, 0x0020, 0x },
diff --git a/lib/media_fill_gen8.c b/lib/media_fill_gen8.c
index 996d4d0..54309d5 100644
--- a/lib/media_fill_gen8.c
+++ b/lib/media_fill_gen8.c
@@ -4,10 +4,10 @@
#include media_fill.h
#include gen8_media.h
#include intel_reg.h
+#include drmtest.h
#include assert.h
-#define ALIGN(x, y) (((x) + (y)-1) ~((y)-1))
static const uint32_t media_kernel[][4] = {
{ 0x0041, 0x20202288, 0x0020, 0x },
diff --git a/lib/rendercopy_gen6.c b/lib/rendercopy_gen6.c
index d806cef..7b3104c 100644
--- a/lib/rendercopy_gen6.c
+++ b/lib/rendercopy_gen6.c
@@ -20,7 +20,6 @@
#include gen6_render.h
#include intel_reg.h
-#define ALIGN(x, y) (((x) + (y)-1) ~((y)-1))
#define VERTEX_SIZE (3*4)
static const uint32_t ps_kernel_nomask_affine[][4] = {
diff --git a/lib/rendercopy_gen7.c b/lib/rendercopy_gen7.c
index cdbc70c..5131d8f 100644
--- a/lib/rendercopy_gen7.c
+++ b/lib/rendercopy_gen7.c
@@ -21,7 +21,6 @@
#include gen7_render.h
#include intel_reg.h
-#define ALIGN(x, y) (((x) + (y)-1) ~((y)-1))
static const uint32_t ps_kernel[][4] = {
{ 0x0080005a, 0x2e2077bd, 0x00c0, 0x008d0040 },
diff --git a/lib/rendercopy_gen8.c b/lib/rendercopy_gen8.c
index e846376..6f5a698 100644
--- a/lib/rendercopy_gen8.c
+++ b/lib/rendercopy_gen8.c
@@ -25,7 +25,6 @@
#include intel_aub.h
-#define ALIGN(x, y) (((x) + (y)-1) ~((y)-1))
#define VERTEX_SIZE (3*4)
#if DEBUG_RENDERCPY
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] tests/gem_userptr_benchmark: Benchmarking userptr surfaces and impact
[snip]
On Wed, Apr 23, 2014 at 06:28:54AM -0700, Tvrtko Ursulin wrote:
On 04/18/2014 12:18 AM, Volkin, Bradley D wrote:
On Wed, Mar 19, 2014 at 04:13:06AM -0700, Tvrtko Ursulin wrote:
+static void **handle_ptr_map;
+static unsigned int num_handle_ptr_map;
I'd prefer that we explicitly initialize at least num_handle_ptr_map.
To zero, why?
Partly because I just like explicitly initialized variables and
partly because I forgot that static variables are *guaranteed* to
be initialized to zero vs just-happen-to-be-zero :)
You can leave it as is or change it, I'm fine either way.
Thanks,
Brad
+
+static void add_handle_ptr(uint32_t handle, void *ptr)
+{
+ if (handle = num_handle_ptr_map) {
+ handle_ptr_map = realloc(handle_ptr_map,
+ (handle + 1000) * sizeof(void*));
+ num_handle_ptr_map = handle + 1000;
+ }
+
+ handle_ptr_map[handle] = ptr;
+}
+
+static void *get_handle_ptr(uint32_t handle)
+{
+ return handle_ptr_map[handle];
+}
+
+static void free_handle_ptr(uint32_t handle)
+{
+ igt_assert(handle num_handle_ptr_map);
+ igt_assert(handle_ptr_map[handle]);
+
+ free(handle_ptr_map[handle]);
+ handle_ptr_map[handle] = NULL;
+}
+
+static uint32_t create_userptr_bo(int fd, int size)
+{
+ void *ptr;
+ uint32_t handle;
+ int ret;
+
+ ret = posix_memalign(ptr, PAGE_SIZE, size);
+ igt_assert(ret == 0);
+
+ ret = gem_userptr(fd, (uint32_t *)ptr, size, 0, handle);
+ igt_assert(ret == 0);
+ add_handle_ptr(handle, ptr);
+
+ return handle;
+}
+
+static void free_userptr_bo(int fd, uint32_t handle)
+{
+ gem_close(fd, handle);
+ free_handle_ptr(handle);
+}
+
+static int has_userptr(int fd)
+{
+ uint32_t handle = 0;
+ void *ptr;
+ uint32_t oldflags;
+ int ret;
+
+ assert(posix_memalign(ptr, PAGE_SIZE, PAGE_SIZE) == 0);
+ oldflags = userptr_flags;
+ gem_userptr_test_unsynchronized();
+ ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, handle);
+ userptr_flags = oldflags;
+ if (ret != 0) {
+ free(ptr);
+ return 0;
+ }
+
+ gem_close(fd, handle);
+ free(ptr);
+
+ return handle != 0;
+}
+
+static const unsigned int nr_bos[] = {0, 1, 10, 100, 1000};
+static const unsigned int test_duration_sec = 3;
+
+static volatile unsigned int run_test;
+
+static void alarm_handler(int sig)
+{
+ assert(run_test == 1);
+ run_test = 0;
+}
+
+static void start_test(unsigned int duration)
+{
+ run_test = 1;
+ if (duration == 0)
+ duration = test_duration_sec;
+ signal(SIGALRM, alarm_handler);
+ alarm(duration);
+}
+
+static void exchange_ptr(void *array, unsigned i, unsigned j)
+{
+ void **arr, *tmp;
+ arr = (void **)array;
+
+ tmp = arr[i];
+ arr[i] = arr[j];
+ arr[j] = tmp;
+}
+
+static void test_malloc_free(int random)
+{
+ unsigned long iter = 0;
+ unsigned int i, tot = 1000;
+ void *ptr[tot];
+
+ start_test(test_duration_sec);
+
+ while (run_test) {
+ for (i = 0; i tot; i++) {
+ ptr[i] = malloc(1000);
+ assert(ptr[i]);
+ }
+ if (random)
+ igt_permute_array(ptr, tot, exchange_ptr);
+ for (i = 0; i tot; i++)
+ free(ptr[i]);
+ iter++;
+ }
+
+ printf(%8lu iter/s\n, iter / test_duration_sec);
+}
+
+static void test_malloc_realloc_free(int random)
+{
+ unsigned long iter = 0;
+ unsigned int i, tot = 1000;
+ void *ptr[tot];
+
+ start_test(test_duration_sec);
+
+ while (run_test) {
+ for (i = 0; i tot; i++) {
+ ptr[i] = malloc(1000);
+ assert(ptr[i]);
+ }
+ if (random)
+ igt_permute_array(ptr, tot, exchange_ptr);
+ for (i = 0; i tot; i++) {
+ ptr[i] = realloc(ptr[i], 2000);
+ assert(ptr[i]);
+ }
+ if (random)
+ igt_permute_array(ptr, tot, exchange_ptr);
+ for (i = 0; i tot; i++)
+ free(ptr[i]);
+ iter++;
+ }
+
+ printf(%8lu iter/s\n, iter / test_duration_sec);
+}
+
+static void test_mmap_unmap(int random)
+{
+ unsigned long iter = 0;
+ unsigned int i, tot = 1000;
+ void *ptr[tot];
+
+ start_test(test_duration_sec);
+
+ while (run_test) {
+ for (i = 0; i tot; i++) {
+ ptr[i] = mmap(NULL, 1000, PROT_READ | PROT_WRITE,
+ MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+ assert(ptr[i] != MAP_FAILED);
+ }
+ if (random)
+ igt_permute_array(ptr, tot, exchange_ptr);
+ for (i = 0; i tot; i++)
+ munmap(ptr[i], 1000);
+ iter++;
+ }
+
Re: [Intel-gfx] [PATCH 1/3] tests/gem_userptr_blits: Expanded userptr test cases
On Wed, Apr 23, 2014 at 06:33:40AM -0700, Tvrtko Ursulin wrote: On 04/18/2014 06:10 PM, Volkin, Bradley D wrote: On Wed, Mar 19, 2014 at 04:13:04AM -0700, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com A set of userptr test cases to support the new feature. For the eviction and swapping stress testing I have extracted some common behaviour from gem_evict_everything and made both test cases use it to avoid duplicating the code. Both unsynchronized and synchronized userptr objects are tested but the latter set of tests will be skipped if kernel is compiled without MMU_NOTIFIERS. Also, with 32-bit userspace swapping tests are skipped if the system has a lot more RAM than process address space. Forking swapping tests are not skipped since they can still trigger swapping by cumulative effect. v2: * Fixed dmabuf test. * Added test for rejecting read-only. * Fixed ioctl detection for latest kernel patch. v3: * Updated copy() for Gen8+. * Fixed ioctl detection on kernels without MMU_NOTIFIERs. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com A number of the comments I made on patch 3 apply here as well. The sizeof(linear) thing is more prevalent in this test, though it looks like linear is at least used. Other than those comments this looks good to me. Believe it or not that sizeof(linear) idiom I inherited from other blitter tests. Personally I don't care one way or another. But since it makes sense to get rid of it for the benchmark part, perhaps I should change it here as well to be consistent. How strongly do you feel strongly about this? I think changing it would be slightly more readable, but if it's consistent with other blit tests then I don't feel too strongly about it. In fact, consistency with the other tests might be the better approach. I'm fine with whichever approach you prefer. Thanks, Brad Will see what you reply on the static initializer comment it 3/3, not sure what you meant there. Thanks, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] tests/gem_userptr_benchmark: Benchmarking userptr surfaces and impact
From: Tvrtko Ursulin tvrtko.ursu...@intel.com
This adds a small benchmark for the new userptr functionality.
Apart from basic surface creation and destruction, also tested is the
impact of having userptr surfaces in the process address space. Reason
for that is the impact of MMU notifiers on common address space
operations like munmap() which is per process.
v2:
* Moved to benchmarks.
* Added pointer read/write tests.
* Changed output to say iterations per second instead of
operations per second.
* Multiply result by batch size for multi-create* tests
for a more comparable number with create-destroy test.
v3:
* Use ALIGN macro.
* Catchup with big lib/ reorganization.
* Removed unused code and one global variable.
* Fixed up some warnings.
Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
---
benchmarks/.gitignore | 1 +
benchmarks/Makefile.sources| 3 +-
benchmarks/gem_userptr_benchmark.c | 497 +
3 files changed, 500 insertions(+), 1 deletion(-)
create mode 100644 benchmarks/gem_userptr_benchmark.c
diff --git a/benchmarks/.gitignore b/benchmarks/.gitignore
index ddea6f7..09e5bd8 100644
--- a/benchmarks/.gitignore
+++ b/benchmarks/.gitignore
@@ -1,3 +1,4 @@
+gem_userptr_benchmark
intel_upload_blit_large
intel_upload_blit_large_gtt
intel_upload_blit_large_map
diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources
index f9da579..60bdae2 100644
--- a/benchmarks/Makefile.sources
+++ b/benchmarks/Makefile.sources
@@ -2,4 +2,5 @@ bin_PROGRAMS = \
intel_upload_blit_large \
intel_upload_blit_large_gtt \
intel_upload_blit_large_map \
- intel_upload_blit_small
+ intel_upload_blit_small \
+ gem_userptr_benchmark
diff --git a/benchmarks/gem_userptr_benchmark.c
b/benchmarks/gem_userptr_benchmark.c
new file mode 100644
index 000..a51201c
--- /dev/null
+++ b/benchmarks/gem_userptr_benchmark.c
@@ -0,0 +1,497 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Tvrtko Ursulin tvrtko.ursu...@intel.com
+ *
+ */
+
+/** @file gem_userptr_benchmark.c
+ *
+ * Benchmark the userptr code and impact of having userptr surfaces
+ * in process address space on some normal operations.
+ *
+ */
+
+#include stdlib.h
+#include stdio.h
+#include string.h
+#include fcntl.h
+#include inttypes.h
+#include errno.h
+#include assert.h
+#include sys/stat.h
+#include sys/time.h
+#include sys/mman.h
+#include drm.h
+#include i915_drm.h
+#include drmtest.h
+#include intel_bufmgr.h
+#include intel_batchbuffer.h
+#include intel_chipset.h
+#include ioctl_wrappers.h
+#include igt_aux.h
+
+#ifndef PAGE_SIZE
+ #define PAGE_SIZE 4096
+#endif
+
+#define LOCAL_I915_GEM_USERPTR 0x34
+#define LOCAL_IOCTL_I915_GEM_USERPTR DRM_IOWR (DRM_COMMAND_BASE +
LOCAL_I915_GEM_USERPTR, struct local_i915_gem_userptr)
+struct local_i915_gem_userptr {
+ uint64_t user_ptr;
+ uint64_t user_size;
+ uint32_t flags;
+#define LOCAL_I915_USERPTR_READ_ONLY (10)
+#define LOCAL_I915_USERPTR_UNSYNCHRONIZED (131)
+ uint32_t handle;
+};
+
+static uint32_t userptr_flags = LOCAL_I915_USERPTR_UNSYNCHRONIZED;
+
+#define BO_SIZE (65536)
+
+static void gem_userptr_test_unsynchronized(void)
+{
+ userptr_flags = LOCAL_I915_USERPTR_UNSYNCHRONIZED;
+}
+
+static void gem_userptr_test_synchronized(void)
+{
+ userptr_flags = 0;
+}
+
+static int gem_userptr(int fd, void *ptr, int size, int read_only, uint32_t
*handle)
+{
+ struct local_i915_gem_userptr userptr;
+ int ret;
+
+ userptr.user_ptr = (uintptr_t)ptr;
+ userptr.user_size = size;
+ userptr.flags = userptr_flags;
+ if (read_only)
+ userptr.flags |= LOCAL_I915_USERPTR_READ_ONLY;
+
+
[Intel-gfx] [PATCH 2/3] tests/gem_vmap_blits: Remove obsolete test case
From: Tvrtko Ursulin tvrtko.ursu...@intel.com
No need for the old test case once the new one was added.
v2:
* Just rebase for lib/ reorganization.
Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
---
tests/.gitignore | 1 -
tests/Makefile.sources | 1 -
tests/gem_vmap_blits.c | 345 -
3 files changed, 347 deletions(-)
delete mode 100644 tests/gem_vmap_blits.c
diff --git a/tests/.gitignore b/tests/.gitignore
index ff193bd..d192bb9 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -94,7 +94,6 @@ gem_tiled_swapping
gem_tiling_max_stride
gem_unfence_active_buffers
gem_unref_active_buffers
-gem_vmap_blits
gem_userptr_blits
gem_wait_render_timeout
gem_write_read_ring_switch
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 09d6aa3..57a0da2 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -120,7 +120,6 @@ TESTS_progs = \
gem_tiling_max_stride \
gem_unfence_active_buffers \
gem_unref_active_buffers \
- gem_vmap_blits \
gem_userptr_blits \
gem_wait_render_timeout \
gen3_mixed_blits \
diff --git a/tests/gem_vmap_blits.c b/tests/gem_vmap_blits.c
deleted file mode 100644
index 430338b..000
--- a/tests/gem_vmap_blits.c
+++ /dev/null
@@ -1,345 +0,0 @@
-/*
- * Copyright © 2009,2011 Intel Corporation
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the Software),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- *
- * Authors:
- *Eric Anholt e...@anholt.net
- *Chris Wilson ch...@chris-wilson.co.uk
- *
- */
-
-/** @file gem_vmap_blits.c
- *
- * This is a test of doing many blits using a mixture of normal system pages
- * and uncached linear buffers, with a working set larger than the
- * aperture size.
- *
- * The goal is to simply ensure the basics work.
- */
-
-#include stdlib.h
-#include stdio.h
-#include string.h
-#include fcntl.h
-#include inttypes.h
-#include errno.h
-#include sys/stat.h
-#include sys/time.h
-
-#include drm.h
-#include ioctl_wrappers.h
-#include drmtest.h
-#include intel_bufmgr.h
-#include intel_batchbuffer.h
-#include intel_io.h
-
-#if !defined(I915_PARAM_HAS_VMAP)
-#pragma message(No vmap support in drm, skipping)
-int main(int argc, char **argv)
-{
- fprintf(stderr, No vmap support in drm.\n);
- return 77;
-}
-#else
-
-#define WIDTH 512
-#define HEIGHT 512
-
-static uint32_t linear[WIDTH*HEIGHT];
-
-static uint32_t gem_vmap(int fd, void *ptr, int size, int read_only)
-{
- struct drm_i915_gem_vmap vmap;
-
- vmap.user_ptr = (uintptr_t)ptr;
- vmap.user_size = size;
- vmap.flags = 0;
- if (read_only)
- vmap.flags |= I915_VMAP_READ_ONLY;
-
- if (drmIoctl(fd, DRM_IOCTL_I915_GEM_VMAP, vmap))
- return 0;
-
- return vmap.handle;
-}
-
-
-static void gem_vmap_sync(int fd, uint32_t handle)
-{
- gem_set_domain(fd, handle, I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
-}
-
-static void
-copy(int fd, uint32_t dst, uint32_t src)
-{
- uint32_t batch[10];
- struct drm_i915_gem_relocation_entry reloc[2];
- struct drm_i915_gem_exec_object2 obj[3];
- struct drm_i915_gem_execbuffer2 exec;
- uint32_t handle;
- int ret;
-
- batch[0] = XY_SRC_COPY_BLT_CMD |
- XY_SRC_COPY_BLT_WRITE_ALPHA |
- XY_SRC_COPY_BLT_WRITE_RGB | 6;
- batch[1] = (3 24) | /* 32 bits */
- (0xcc 16) | /* copy ROP */
- WIDTH*4;
- batch[2] = 0; /* dst x1,y1 */
- batch[3] = (HEIGHT 16) | WIDTH; /* dst x2,y2 */
- batch[4] = 0; /* dst reloc */
- batch[5] = 0; /* src x1,y1 */
- batch[6] = WIDTH*4;
- batch[7] = 0; /* src reloc */
- batch[8] = MI_BATCH_BUFFER_END;
- batch[9] = MI_NOOP;
-
- handle = gem_create(fd, 4096);
- gem_write(fd, handle, 0, batch, sizeof(batch));
-
- reloc[0].target_handle = dst;
-
[Intel-gfx] [PATCH 1/3] tests/gem_userptr_blits: Expanded userptr test cases
From: Tvrtko Ursulin tvrtko.ursu...@intel.com
A set of userptr test cases to support the new feature.
For the eviction and swapping stress testing I have extracted
some common behaviour from gem_evict_everything and made both
test cases use it to avoid duplicating the code.
Both unsynchronized and synchronized userptr objects are
tested but the latter set of tests will be skipped if kernel
is compiled without MMU_NOTIFIERS.
Also, with 32-bit userspace swapping tests are skipped if
the system has a lot more RAM than process address space.
Forking swapping tests are not skipped since they can still
trigger swapping by cumulative effect.
v2:
* Fixed dmabuf test.
* Added test for rejecting read-only.
* Fixed ioctl detection for latest kernel patch.
v3:
* Use ALIGN macro.
* Catchup with big lib/ reorganization.
* Fixed up some warnings.
Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
---
tests/.gitignore |1 +
tests/Makefile.sources|1 +
tests/gem_userptr_blits.c | 1247 +
3 files changed, 1249 insertions(+)
create mode 100644 tests/gem_userptr_blits.c
diff --git a/tests/.gitignore b/tests/.gitignore
index 146bab0..ff193bd 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -95,6 +95,7 @@ gem_tiling_max_stride
gem_unfence_active_buffers
gem_unref_active_buffers
gem_vmap_blits
+gem_userptr_blits
gem_wait_render_timeout
gem_write_read_ring_switch
gen3_mixed_blits
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c957ace..09d6aa3 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -121,6 +121,7 @@ TESTS_progs = \
gem_unfence_active_buffers \
gem_unref_active_buffers \
gem_vmap_blits \
+ gem_userptr_blits \
gem_wait_render_timeout \
gen3_mixed_blits \
gen3_render_linear_blits \
diff --git a/tests/gem_userptr_blits.c b/tests/gem_userptr_blits.c
new file mode 100644
index 000..03af58e
--- /dev/null
+++ b/tests/gem_userptr_blits.c
@@ -0,0 +1,1247 @@
+/*
+ * Copyright © 2009-2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Eric Anholt e...@anholt.net
+ *Chris Wilson ch...@chris-wilson.co.uk
+ *Tvrtko Ursulin tvrtko.ursu...@intel.com
+ *
+ */
+
+/** @file gem_userptr_blits.c
+ *
+ * This is a test of doing many blits using a mixture of normal system pages
+ * and uncached linear buffers, with a working set larger than the
+ * aperture size.
+ *
+ * The goal is to simply ensure the basics work.
+ */
+
+#include stdlib.h
+#include stdio.h
+#include string.h
+#include fcntl.h
+#include inttypes.h
+#include errno.h
+#include assert.h
+#include sys/stat.h
+#include sys/time.h
+#include sys/mman.h
+#include drm.h
+#include i915_drm.h
+#include drmtest.h
+#include intel_bufmgr.h
+#include intel_batchbuffer.h
+#include intel_chipset.h
+#include ioctl_wrappers.h
+
+#include eviction_common.c
+
+#ifndef PAGE_SIZE
+#define PAGE_SIZE 4096
+#endif
+
+#define LOCAL_I915_GEM_USERPTR 0x34
+#define LOCAL_IOCTL_I915_GEM_USERPTR DRM_IOWR (DRM_COMMAND_BASE +
LOCAL_I915_GEM_USERPTR, struct local_i915_gem_userptr)
+struct local_i915_gem_userptr {
+ uint64_t user_ptr;
+ uint64_t user_size;
+ uint32_t flags;
+#define LOCAL_I915_USERPTR_READ_ONLY (10)
+#define LOCAL_I915_USERPTR_UNSYNCHRONIZED (131)
+ uint32_t handle;
+};
+
+static uint32_t userptr_flags = LOCAL_I915_USERPTR_UNSYNCHRONIZED;
+
+#define WIDTH 512
+#define HEIGHT 512
+
+static uint32_t linear[WIDTH*HEIGHT];
+
+static void gem_userptr_test_unsynchronized(void)
+{
+ userptr_flags = LOCAL_I915_USERPTR_UNSYNCHRONIZED;
+}
+
+static void gem_userptr_test_synchronized(void)
+{
+ userptr_flags = 0;
+}
+
+static int gem_userptr(int fd, void *ptr, int size, int read_only, uint32_t
*handle)
+{
+ struct local_i915_gem_userptr userptr;
+
Re: [Intel-gfx] [RFC] xf86-video-intel: enable hw-generated binding tables
On Wed, Apr 23, 2014 at 1:21 PM, Abdiel Janulgue abdiel.janul...@linux.intel.com wrote: I've already tried disabling RS at the end of every batch so that next batch in different context can continue to use older non-RS format. That does not work either and still causes hangs. What I've seen so far, it seems GPU does not like switching the format of commands from RS-format to non-RS format. It's either one way or the other. If switched on, it affects subsequent contexes henceforth expecting RS-format commands until the GPU gets reset. That's probably the note in bspec: the binding table generator feature has a simple all or nothing model. Oh hooray, that's just awesome :( So we indeed need to stop the gpu and reset it if there's a non-RS render batch after any RS render batch. Which also means that we need to enable this for _all_ userspace to avoid completely disastrous performance. So uxa, sna, libva, maybe opencl ... I guess before we engage in this endeavor we need to track this down with the hardware people. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] tests: Extract ALIGN macro into a common header
Reviewed-by: Brad Volkin bradley.d.vol...@intel.com
On Wed, Apr 23, 2014 at 08:07:55AM -0700, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin tvrtko.ursu...@intel.com
Makes for a little bit less code duplication, especially since
it will be used from more callers in the future.
Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
---
lib/drmtest.h | 9 +
lib/media_fill_gen7.c | 2 +-
lib/media_fill_gen8.c | 2 +-
lib/rendercopy_gen6.c | 1 -
lib/rendercopy_gen7.c | 1 -
lib/rendercopy_gen8.c | 1 -
6 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/lib/drmtest.h b/lib/drmtest.h
index f3afbaa..84f80dc 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -60,6 +60,15 @@ static inline void *igt_mmap64(void *addr, size_t length,
int prot, int flags,
*/
#define ARRAY_SIZE(arr) (sizeof(arr)/sizeof(arr[0]))
+/**
+ * ALIGN:
+ * @v: value to be aligned
+ * @a: alignment unit in bytes
+ *
+ * Macro to align a value @v to a specified unit @a.
+ */
+#define ALIGN(v, a) (((v) + (a)-1) ~((a)-1))
+
int drm_get_card(void);
int drm_open_any(void);
int drm_open_any_render(void);
diff --git a/lib/media_fill_gen7.c b/lib/media_fill_gen7.c
index cdf4b60..82c3469 100644
--- a/lib/media_fill_gen7.c
+++ b/lib/media_fill_gen7.c
@@ -4,10 +4,10 @@
#include media_fill.h
#include gen7_media.h
#include intel_reg.h
+#include drmtest.h
#include assert.h
-#define ALIGN(x, y) (((x) + (y)-1) ~((y)-1))
static const uint32_t media_kernel[][4] = {
{ 0x0041, 0x20200231, 0x0020, 0x },
diff --git a/lib/media_fill_gen8.c b/lib/media_fill_gen8.c
index 996d4d0..54309d5 100644
--- a/lib/media_fill_gen8.c
+++ b/lib/media_fill_gen8.c
@@ -4,10 +4,10 @@
#include media_fill.h
#include gen8_media.h
#include intel_reg.h
+#include drmtest.h
#include assert.h
-#define ALIGN(x, y) (((x) + (y)-1) ~((y)-1))
static const uint32_t media_kernel[][4] = {
{ 0x0041, 0x20202288, 0x0020, 0x },
diff --git a/lib/rendercopy_gen6.c b/lib/rendercopy_gen6.c
index d806cef..7b3104c 100644
--- a/lib/rendercopy_gen6.c
+++ b/lib/rendercopy_gen6.c
@@ -20,7 +20,6 @@
#include gen6_render.h
#include intel_reg.h
-#define ALIGN(x, y) (((x) + (y)-1) ~((y)-1))
#define VERTEX_SIZE (3*4)
static const uint32_t ps_kernel_nomask_affine[][4] = {
diff --git a/lib/rendercopy_gen7.c b/lib/rendercopy_gen7.c
index cdbc70c..5131d8f 100644
--- a/lib/rendercopy_gen7.c
+++ b/lib/rendercopy_gen7.c
@@ -21,7 +21,6 @@
#include gen7_render.h
#include intel_reg.h
-#define ALIGN(x, y) (((x) + (y)-1) ~((y)-1))
static const uint32_t ps_kernel[][4] = {
{ 0x0080005a, 0x2e2077bd, 0x00c0, 0x008d0040 },
diff --git a/lib/rendercopy_gen8.c b/lib/rendercopy_gen8.c
index e846376..6f5a698 100644
--- a/lib/rendercopy_gen8.c
+++ b/lib/rendercopy_gen8.c
@@ -25,7 +25,6 @@
#include intel_aub.h
-#define ALIGN(x, y) (((x) + (y)-1) ~((y)-1))
#define VERTEX_SIZE (3*4)
#if DEBUG_RENDERCPY
--
1.9.1
___
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
[Intel-gfx] [PATCH 1/3] drm: Check CRTC compatibility in setplane
The DRM core setplane code should check that the plane is usable on the
specified CRTC before calling into the driver.
Prior to this patch, a plane's possible_crtcs field was purely
informational for userspace and was never actually verified at the
kernel level (aside from the primary plane helper).
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
drivers/gpu/drm/drm_crtc.c | 7 +++
drivers/gpu/drm/drm_plane_helper.c | 6 --
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 461d19b..b6d6c04 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2140,6 +2140,13 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
}
crtc = obj_to_crtc(obj);
+ /* Check whether this plane is usable on this CRTC */
+ if (!(plane-possible_crtcs drm_crtc_mask(crtc))) {
+ DRM_DEBUG_KMS(Invalid crtc for plane\n);
+ ret = -EINVAL;
+ goto out;
+ }
+
fb = drm_framebuffer_lookup(dev, plane_req-fb_id);
if (!fb) {
DRM_DEBUG_KMS(Unknown framebuffer ID %d\n,
diff --git a/drivers/gpu/drm/drm_plane_helper.c
b/drivers/gpu/drm/drm_plane_helper.c
index b72736d..65c4a00 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -137,12 +137,6 @@ int drm_primary_helper_update(struct drm_plane *plane,
struct drm_crtc *crtc,
return -EINVAL;
}
- /* Primary planes are locked to their owning CRTC */
- if (plane-possible_crtcs != drm_crtc_mask(crtc)) {
- DRM_DEBUG_KMS(Cannot change primary plane CRTC\n);
- return -EINVAL;
- }
-
/* Disallow scaling */
src_w = 16;
src_h = 16;
--
1.8.5.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Intel-specific primary plane handling (v5)
Intel hardware allows the primary plane to be disabled independently of
the CRTC. Provide custom primary plane handling to allow this.
v5:
- Use new drm_primary_helper_check_update() helper function to check
setplane parameter validity.
- Swap primary plane's pipe for pre-gen4 FBC (caught by Ville Syrjälä)
- Cleanup primary plane properly on crtc init failure
v4:
- Don't add a primary_plane field to intel_crtc; that was left over
from a much earlier iteration of this patch series, but is no longer
needed/used now that the DRM core primary plane support has been
merged.
v3:
- Provide gen-specific primary plane format lists (suggested by Daniel
Vetter).
- If the primary plane is already enabled, go ahead and just call the
primary plane helper to do the update (suggested by Daniel Vetter).
- Don't try to disable the primary plane on destruction; the DRM layer
should have already taken care of this for us.
v2:
- Unpin fb properly on primary plane disable
- Provide an Intel-specific set of primary plane formats
- Additional sanity checks on setplane (in line with the checks
currently being done by the DRM core primary plane helper)
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 161 ++-
1 file changed, 159 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b57210c..ca83970 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -39,8 +39,35 @@
#include i915_trace.h
#include drm/drm_dp_helper.h
#include drm/drm_crtc_helper.h
+#include drm/drm_plane_helper.h
+#include drm/drm_rect.h
#include linux/dma_remapping.h
+/* Primary plane formats supported by all gen */
+#define COMMON_PRIMARY_FORMATS \
+ DRM_FORMAT_C8, \
+ DRM_FORMAT_RGB565, \
+ DRM_FORMAT_XRGB, \
+ DRM_FORMAT_ARGB
+
+/* Primary plane formats for gen = 3 */
+static const uint32_t intel_primary_formats_gen3[] = {
+ COMMON_PRIMARY_FORMATS,
+ DRM_FORMAT_XRGB1555,
+ DRM_FORMAT_ARGB1555,
+};
+
+/* Primary plane formats for gen = 4 */
+static const uint32_t intel_primary_formats_gen4[] = {
+ COMMON_PRIMARY_FORMATS, \
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_XRGB2101010,
+ DRM_FORMAT_ARGB2101010,
+ DRM_FORMAT_XBGR2101010,
+ DRM_FORMAT_ABGR2101010,
+};
+
static void intel_increase_pllclock(struct drm_crtc *crtc);
static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
@@ -10543,17 +10570,147 @@ static void intel_shared_dpll_init(struct drm_device
*dev)
BUG_ON(dev_priv-num_shared_dpll I915_NUM_PLLS);
}
+static int
+intel_primary_plane_disable(struct drm_plane *plane)
+{
+ struct drm_device *dev = plane-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ struct intel_crtc *intel_crtc;
+
+ if (WARN_ON(!plane-crtc))
+ return -EINVAL;
+
+ intel_crtc = to_intel_crtc(plane-crtc);
+ if (!intel_crtc-primary_enabled)
+ return 0;
+
+ intel_disable_primary_hw_plane(dev_priv, intel_plane-plane,
+ intel_plane-pipe);
+
+ /*
+* N.B. The DRM setplane code will update the plane-fb pointer after
+* we finish here.
+*/
+ intel_unpin_fb_obj(to_intel_framebuffer(plane-fb)-obj);
+
+ return 0;
+}
+
+static int
+intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
+struct drm_framebuffer *fb, int crtc_x, int crtc_y,
+unsigned int crtc_w, unsigned int crtc_h,
+uint32_t src_x, uint32_t src_y,
+uint32_t src_w, uint32_t src_h)
+{
+ struct drm_device *dev = crtc-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_rect dest = {
+ .x1 = crtc_x,
+ .y1 = crtc_y,
+ .x2 = crtc_x + crtc_w,
+ .y2 = crtc_y + crtc_h,
+ };
+ struct drm_rect src = {
+ .x1 = src_x 16,
+ .y1 = src_y 16,
+ .x2 = (src_x + src_w) 16,
+ .y2 = (src_y + src_h) 16,
+ };
+ const struct drm_rect clip = {
+ .x2 = intel_crtc-config.pipe_src_w,
+ .y2 = intel_crtc-config.pipe_src_h,
+ };
+ int visible;
+ int ret;
+
+ ret = drm_primary_helper_check_update(plane, crtc, fb,
+ crtc_x, crtc_y, crtc_w, crtc_h,
+ src_x, src_y, src_w, src_h,
+ false, false);
+ if (ret)
+ return
[Intel-gfx] [PATCH 2/3] drm/plane-helper: Add drm_primary_helper_check_update()
Pull the parameter checking from drm_primary_helper_update() out into
its own function; drivers that provide their own setplane()
implementations rather than using the helper may still want to share
this parameter checking logic.
A few of the checks here were also updated based on suggestions by
Ville Syrjälä.
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
drivers/gpu/drm/drm_plane_helper.c | 148 +
include/drm/drm_plane_helper.h | 9 +++
2 files changed, 128 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/drm_plane_helper.c
b/drivers/gpu/drm/drm_plane_helper.c
index 65c4a00..9bbbdf2 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -66,6 +66,102 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
}
/**
+ * drm_primary_helper_check_update() - Check primary plane update for validity
+ * @plane: plane object to update
+ * @crtc: owning CRTC of owning plane
+ * @fb: framebuffer to flip onto plane
+ * @crtc_x: x offset of primary plane on crtc
+ * @crtc_y: y offset of primary plane on crtc
+ * @crtc_w: width of primary plane rectangle on crtc
+ * @crtc_h: height of primary plane rectangle on crtc
+ * @src_x: x offset of @fb for panning
+ * @src_y: y offset of @fb for panning
+ * @src_w: width of source rectangle in @fb
+ * @src_h: height of source rectangle in @fb
+ * @can_scale: is primary plane scaling legal?
+ * @can_position: is it legal to position the primary plane such that it
+ *doesn't cover the entire crtc?
+ *
+ * Checks that a desired primary plane update is valid. Drivers that provide
+ * their own primary plane handling may still wish to call this function to
+ * avoid duplication of error checking code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_primary_helper_check_update(struct drm_plane *plane,
+ struct drm_crtc *crtc,
+ struct drm_framebuffer *fb,
+ int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y,
+ uint32_t src_w, uint32_t src_h,
+ bool can_scale,
+ bool can_position)
+{
+ struct drm_rect dest = {
+ .x1 = crtc_x,
+ .y1 = crtc_y,
+ .x2 = crtc_x + crtc_w,
+ .y2 = crtc_y + crtc_h,
+ };
+ struct drm_rect src = {
+ .x1 = src_x 16,
+ .y1 = src_y 16,
+ .x2 = (src_x + src_w) 16,
+ .y2 = (src_y + src_h) 16,
+ };
+ const struct drm_rect clip = {
+ .x2 = crtc-mode.hdisplay,
+ .y2 = crtc-mode.vdisplay,
+ };
+ int hscale, vscale;
+ int visible;
+
+ if (!crtc-enabled) {
+ DRM_DEBUG_KMS(Cannot update primary plane of a disabled
CRTC.\n);
+ return -EINVAL;
+ }
+
+ /* setplane API takes shifted source rectangle values; unshift them */
+ src_x = 16;
+ src_y = 16;
+ src_w = 16;
+ src_h = 16;
+
+ /* check scaling */
+ if (!can_scale (crtc_w != src_w || crtc_h != src_h)) {
+ DRM_DEBUG_KMS(Can't scale primary plane\n);
+ return -EINVAL;
+ }
+
+ /*
+* Drivers that can scale should perform their own min/max scale
+* factor checks.
+*/
+ hscale = drm_rect_calc_hscale(src, dest, 0, INT_MAX);
+ vscale = drm_rect_calc_vscale(src, dest, 0, INT_MAX);
+ visible = drm_rect_clip_scaled(src, dest, clip, hscale, vscale);
+ if (!visible)
+ /*
+* Primary plane isn't visible; some drivers can handle this
+* so we just return success here. Drivers that can't
+* (including those that use the primary plane helper's
+* update function) will return an error from their
+* update_plane handler.
+*/
+ return 0;
+
+ if (!can_position !drm_rect_equals(dest, clip)) {
+ DRM_DEBUG_KMS(Primary plane must cover entire CRTC\n);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_primary_helper_check_update);
+
+/**
* drm_primary_helper_update() - Helper for primary plane update
* @plane: plane object to update
* @crtc: owning CRTC of owning plane
@@ -113,6 +209,12 @@ int drm_primary_helper_update(struct drm_plane *plane,
struct drm_crtc *crtc,
.x = src_x 16,
.y = src_y 16,
};
+ struct drm_rect src = {
+ .x1 = src_x 16,
+ .y1 = src_y 16,
+
Re: [Intel-gfx] [PATCH] benchmarks: Build them on Android.
Reviewed-by: Brad Volkin bradley.d.vol...@intel.com On Wed, Apr 23, 2014 at 09:03:23AM -0700, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com They build fine so give them some exposure. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com --- Android.mk | 2 +- benchmarks/Android.mk | 36 benchmarks/Makefile.am | 6 +- benchmarks/Makefile.sources | 5 + 4 files changed, 43 insertions(+), 6 deletions(-) create mode 100644 benchmarks/Android.mk create mode 100644 benchmarks/Makefile.sources diff --git a/Android.mk b/Android.mk index 8aeb2d4..681d114 100644 --- a/Android.mk +++ b/Android.mk @@ -1,2 +1,2 @@ -include $(call all-named-subdir-makefiles, lib tests tools) +include $(call all-named-subdir-makefiles, lib tests tools benchmarks) diff --git a/benchmarks/Android.mk b/benchmarks/Android.mk new file mode 100644 index 000..5bb8ef5 --- /dev/null +++ b/benchmarks/Android.mk @@ -0,0 +1,36 @@ +LOCAL_PATH := $(call my-dir) + +include $(LOCAL_PATH)/Makefile.sources + +## + +define add_benchmark +include $(CLEAR_VARS) + +LOCAL_SRC_FILES := $1.c + +LOCAL_CFLAGS += -DHAVE_STRUCT_SYSINFO_TOTALRAM +LOCAL_CFLAGS += -DANDROID -UNDEBUG -include check-ndebug.h +LOCAL_CFLAGS += -std=c99 +# FIXME: drop once Bionic correctly annotates noreturn on pthread_exit +LOCAL_CFLAGS += -Wno-error=return-type +# Excessive complaining for established cases. Rely on the Linux version warnings. +LOCAL_CFLAGS += -Wno-sign-compare + +LOCAL_MODULE := $1 +LOCAL_MODULE_TAGS := optional + +LOCAL_STATIC_LIBRARIES := libintel_gpu_tools + +LOCAL_SHARED_LIBRARIES := libpciaccess \ + libdrm\ + libdrm_intel + +include $(BUILD_EXECUTABLE) +endef + +## + +benchmark_list := $(bin_PROGRAMS) + +$(foreach item,$(benchmark_list),$(eval $(call add_benchmark,$(item diff --git a/benchmarks/Makefile.am b/benchmarks/Makefile.am index e2ad784..86f755a 100644 --- a/benchmarks/Makefile.am +++ b/benchmarks/Makefile.am @@ -1,9 +1,5 @@ -bin_PROGRAMS = \ - intel_upload_blit_large \ - intel_upload_blit_large_gtt \ - intel_upload_blit_large_map \ - intel_upload_blit_small +include Makefile.sources AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources new file mode 100644 index 000..f9da579 --- /dev/null +++ b/benchmarks/Makefile.sources @@ -0,0 +1,5 @@ +bin_PROGRAMS = \ + intel_upload_blit_large \ + intel_upload_blit_large_gtt \ + intel_upload_blit_large_map \ + intel_upload_blit_small -- 1.9.1 ___ 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 2/3] tests/gem_vmap_blits: Remove obsolete test case
Reviewed-by: Brad Volkin bradley.d.vol...@intel.com
On Wed, Apr 23, 2014 at 05:38:34PM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin tvrtko.ursu...@intel.com
No need for the old test case once the new one was added.
v2:
* Just rebase for lib/ reorganization.
Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
---
tests/.gitignore | 1 -
tests/Makefile.sources | 1 -
tests/gem_vmap_blits.c | 345
-
3 files changed, 347 deletions(-)
delete mode 100644 tests/gem_vmap_blits.c
diff --git a/tests/.gitignore b/tests/.gitignore
index ff193bd..d192bb9 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -94,7 +94,6 @@ gem_tiled_swapping
gem_tiling_max_stride
gem_unfence_active_buffers
gem_unref_active_buffers
-gem_vmap_blits
gem_userptr_blits
gem_wait_render_timeout
gem_write_read_ring_switch
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 09d6aa3..57a0da2 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -120,7 +120,6 @@ TESTS_progs = \
gem_tiling_max_stride \
gem_unfence_active_buffers \
gem_unref_active_buffers \
- gem_vmap_blits \
gem_userptr_blits \
gem_wait_render_timeout \
gen3_mixed_blits \
diff --git a/tests/gem_vmap_blits.c b/tests/gem_vmap_blits.c
deleted file mode 100644
index 430338b..000
--- a/tests/gem_vmap_blits.c
+++ /dev/null
@@ -1,345 +0,0 @@
-/*
- * Copyright © 2009,2011 Intel Corporation
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the Software),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
DEALINGS
- * IN THE SOFTWARE.
- *
- * Authors:
- *Eric Anholt e...@anholt.net
- *Chris Wilson ch...@chris-wilson.co.uk
- *
- */
-
-/** @file gem_vmap_blits.c
- *
- * This is a test of doing many blits using a mixture of normal system pages
- * and uncached linear buffers, with a working set larger than the
- * aperture size.
- *
- * The goal is to simply ensure the basics work.
- */
-
-#include stdlib.h
-#include stdio.h
-#include string.h
-#include fcntl.h
-#include inttypes.h
-#include errno.h
-#include sys/stat.h
-#include sys/time.h
-
-#include drm.h
-#include ioctl_wrappers.h
-#include drmtest.h
-#include intel_bufmgr.h
-#include intel_batchbuffer.h
-#include intel_io.h
-
-#if !defined(I915_PARAM_HAS_VMAP)
-#pragma message(No vmap support in drm, skipping)
-int main(int argc, char **argv)
-{
- fprintf(stderr, No vmap support in drm.\n);
- return 77;
-}
-#else
-
-#define WIDTH 512
-#define HEIGHT 512
-
-static uint32_t linear[WIDTH*HEIGHT];
-
-static uint32_t gem_vmap(int fd, void *ptr, int size, int read_only)
-{
- struct drm_i915_gem_vmap vmap;
-
- vmap.user_ptr = (uintptr_t)ptr;
- vmap.user_size = size;
- vmap.flags = 0;
- if (read_only)
- vmap.flags |= I915_VMAP_READ_ONLY;
-
- if (drmIoctl(fd, DRM_IOCTL_I915_GEM_VMAP, vmap))
- return 0;
-
- return vmap.handle;
-}
-
-
-static void gem_vmap_sync(int fd, uint32_t handle)
-{
- gem_set_domain(fd, handle, I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
-}
-
-static void
-copy(int fd, uint32_t dst, uint32_t src)
-{
- uint32_t batch[10];
- struct drm_i915_gem_relocation_entry reloc[2];
- struct drm_i915_gem_exec_object2 obj[3];
- struct drm_i915_gem_execbuffer2 exec;
- uint32_t handle;
- int ret;
-
- batch[0] = XY_SRC_COPY_BLT_CMD |
- XY_SRC_COPY_BLT_WRITE_ALPHA |
- XY_SRC_COPY_BLT_WRITE_RGB | 6;
- batch[1] = (3 24) | /* 32 bits */
- (0xcc 16) | /* copy ROP */
- WIDTH*4;
- batch[2] = 0; /* dst x1,y1 */
- batch[3] = (HEIGHT 16) | WIDTH; /* dst x2,y2 */
- batch[4] = 0; /* dst reloc */
- batch[5] = 0; /* src x1,y1 */
- batch[6] = WIDTH*4;
- batch[7] = 0; /* src reloc */
- batch[8] =
Re: [Intel-gfx] [PATCH 3/3] tests/gem_userptr_benchmark: Benchmarking userptr surfaces and impact
On Wed, Apr 23, 2014 at 05:38:35PM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin tvrtko.ursu...@intel.com
This adds a small benchmark for the new userptr functionality.
Apart from basic surface creation and destruction, also tested is the
impact of having userptr surfaces in the process address space. Reason
for that is the impact of MMU notifiers on common address space
operations like munmap() which is per process.
v2:
* Moved to benchmarks.
* Added pointer read/write tests.
* Changed output to say iterations per second instead of
operations per second.
* Multiply result by batch size for multi-create* tests
for a more comparable number with create-destroy test.
v3:
* Use ALIGN macro.
* Catchup with big lib/ reorganization.
* Removed unused code and one global variable.
* Fixed up some warnings.
Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
---
benchmarks/.gitignore | 1 +
benchmarks/Makefile.sources| 3 +-
benchmarks/gem_userptr_benchmark.c | 497
+
3 files changed, 500 insertions(+), 1 deletion(-)
create mode 100644 benchmarks/gem_userptr_benchmark.c
diff --git a/benchmarks/.gitignore b/benchmarks/.gitignore
index ddea6f7..09e5bd8 100644
--- a/benchmarks/.gitignore
+++ b/benchmarks/.gitignore
@@ -1,3 +1,4 @@
+gem_userptr_benchmark
intel_upload_blit_large
intel_upload_blit_large_gtt
intel_upload_blit_large_map
diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources
index f9da579..60bdae2 100644
--- a/benchmarks/Makefile.sources
+++ b/benchmarks/Makefile.sources
@@ -2,4 +2,5 @@ bin_PROGRAMS = \
intel_upload_blit_large \
intel_upload_blit_large_gtt \
intel_upload_blit_large_map \
- intel_upload_blit_small
+ intel_upload_blit_small \
+ gem_userptr_benchmark
diff --git a/benchmarks/gem_userptr_benchmark.c
b/benchmarks/gem_userptr_benchmark.c
new file mode 100644
index 000..a51201c
--- /dev/null
+++ b/benchmarks/gem_userptr_benchmark.c
@@ -0,0 +1,497 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Tvrtko Ursulin tvrtko.ursu...@intel.com
+ *
+ */
+
+/** @file gem_userptr_benchmark.c
+ *
+ * Benchmark the userptr code and impact of having userptr surfaces
+ * in process address space on some normal operations.
+ *
+ */
+
+#include stdlib.h
+#include stdio.h
+#include string.h
+#include fcntl.h
+#include inttypes.h
+#include errno.h
+#include assert.h
+#include sys/stat.h
+#include sys/time.h
+#include sys/mman.h
+#include drm.h
+#include i915_drm.h
+#include drmtest.h
+#include intel_bufmgr.h
+#include intel_batchbuffer.h
+#include intel_chipset.h
+#include ioctl_wrappers.h
+#include igt_aux.h
+
+#ifndef PAGE_SIZE
+ #define PAGE_SIZE 4096
+#endif
+
+#define LOCAL_I915_GEM_USERPTR 0x34
+#define LOCAL_IOCTL_I915_GEM_USERPTR DRM_IOWR (DRM_COMMAND_BASE +
LOCAL_I915_GEM_USERPTR, struct local_i915_gem_userptr)
+struct local_i915_gem_userptr {
+ uint64_t user_ptr;
+ uint64_t user_size;
+ uint32_t flags;
+#define LOCAL_I915_USERPTR_READ_ONLY (10)
+#define LOCAL_I915_USERPTR_UNSYNCHRONIZED (131)
+ uint32_t handle;
+};
+
+static uint32_t userptr_flags = LOCAL_I915_USERPTR_UNSYNCHRONIZED;
+
+#define BO_SIZE (65536)
+
+static void gem_userptr_test_unsynchronized(void)
+{
+ userptr_flags = LOCAL_I915_USERPTR_UNSYNCHRONIZED;
+}
+
+static void gem_userptr_test_synchronized(void)
+{
+ userptr_flags = 0;
+}
+
+static int gem_userptr(int fd, void *ptr, int size, int read_only, uint32_t
*handle)
+{
+ struct local_i915_gem_userptr userptr;
+ int ret;
+
+
Re: [Intel-gfx] [PATCH 1/3] tests/gem_userptr_blits: Expanded userptr test cases
Reviewed-by: Brad Volkin bradley.d.vol...@intel.com
On Wed, Apr 23, 2014 at 05:38:33PM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin tvrtko.ursu...@intel.com
A set of userptr test cases to support the new feature.
For the eviction and swapping stress testing I have extracted
some common behaviour from gem_evict_everything and made both
test cases use it to avoid duplicating the code.
Both unsynchronized and synchronized userptr objects are
tested but the latter set of tests will be skipped if kernel
is compiled without MMU_NOTIFIERS.
Also, with 32-bit userspace swapping tests are skipped if
the system has a lot more RAM than process address space.
Forking swapping tests are not skipped since they can still
trigger swapping by cumulative effect.
v2:
* Fixed dmabuf test.
* Added test for rejecting read-only.
* Fixed ioctl detection for latest kernel patch.
v3:
* Use ALIGN macro.
* Catchup with big lib/ reorganization.
* Fixed up some warnings.
Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
---
tests/.gitignore |1 +
tests/Makefile.sources|1 +
tests/gem_userptr_blits.c | 1247
+
3 files changed, 1249 insertions(+)
create mode 100644 tests/gem_userptr_blits.c
diff --git a/tests/.gitignore b/tests/.gitignore
index 146bab0..ff193bd 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -95,6 +95,7 @@ gem_tiling_max_stride
gem_unfence_active_buffers
gem_unref_active_buffers
gem_vmap_blits
+gem_userptr_blits
gem_wait_render_timeout
gem_write_read_ring_switch
gen3_mixed_blits
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c957ace..09d6aa3 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -121,6 +121,7 @@ TESTS_progs = \
gem_unfence_active_buffers \
gem_unref_active_buffers \
gem_vmap_blits \
+ gem_userptr_blits \
gem_wait_render_timeout \
gen3_mixed_blits \
gen3_render_linear_blits \
diff --git a/tests/gem_userptr_blits.c b/tests/gem_userptr_blits.c
new file mode 100644
index 000..03af58e
--- /dev/null
+++ b/tests/gem_userptr_blits.c
@@ -0,0 +1,1247 @@
+/*
+ * Copyright © 2009-2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Eric Anholt e...@anholt.net
+ *Chris Wilson ch...@chris-wilson.co.uk
+ *Tvrtko Ursulin tvrtko.ursu...@intel.com
+ *
+ */
+
+/** @file gem_userptr_blits.c
+ *
+ * This is a test of doing many blits using a mixture of normal system pages
+ * and uncached linear buffers, with a working set larger than the
+ * aperture size.
+ *
+ * The goal is to simply ensure the basics work.
+ */
+
+#include stdlib.h
+#include stdio.h
+#include string.h
+#include fcntl.h
+#include inttypes.h
+#include errno.h
+#include assert.h
+#include sys/stat.h
+#include sys/time.h
+#include sys/mman.h
+#include drm.h
+#include i915_drm.h
+#include drmtest.h
+#include intel_bufmgr.h
+#include intel_batchbuffer.h
+#include intel_chipset.h
+#include ioctl_wrappers.h
+
+#include eviction_common.c
+
+#ifndef PAGE_SIZE
+#define PAGE_SIZE 4096
+#endif
+
+#define LOCAL_I915_GEM_USERPTR 0x34
+#define LOCAL_IOCTL_I915_GEM_USERPTR DRM_IOWR (DRM_COMMAND_BASE +
LOCAL_I915_GEM_USERPTR, struct local_i915_gem_userptr)
+struct local_i915_gem_userptr {
+ uint64_t user_ptr;
+ uint64_t user_size;
+ uint32_t flags;
+#define LOCAL_I915_USERPTR_READ_ONLY (10)
+#define LOCAL_I915_USERPTR_UNSYNCHRONIZED (131)
+ uint32_t handle;
+};
+
+static uint32_t userptr_flags = LOCAL_I915_USERPTR_UNSYNCHRONIZED;
+
+#define WIDTH 512
+#define HEIGHT 512
+
+static uint32_t linear[WIDTH*HEIGHT];
+
+static void gem_userptr_test_unsynchronized(void)
+{
+ userptr_flags =
Re: [Intel-gfx] [PATCH] drm/i915: Sanitize the enable_ppgtt module option once
On Wed, Apr 23, 2014 at 09:18:03AM +0200, Daniel Vetter wrote:
On Tue, Apr 22, 2014 at 10:22:15PM +0100, Chris Wilson wrote:
On Tue, Apr 22, 2014 at 10:17:50PM +0200, Daniel Vetter wrote:
Otherwise we'll end up spamming dmesg on every context creation on snb
with vt-d enabled. This regression was introduced in
commit 246cbfb5fb9a1ca0997fbb135464c1ff5bb9c549
Author: Ben Widawsky benjamin.widaw...@intel.com
Date: Fri Dec 6 14:11:14 2013 -0800
drm/i915: Reorganize intel_enable_ppgtt
I started to consider what would happen if i915.enable_ppgtt changed on
the fly, but then saw that it is 0400 and this pre-initialisation makes
a lot of sense. So maybe we could mention that here:
As the i915.enable_ppgtt is read-only it cannot be changed after the
module is loaded and so we can perform an early sanitization of the
values.
References: https://lkml.org/lkml/2014/4/17/599
Cc: Alessandro Suardi alessandro.sua...@gmail.com
Cc: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
If you care to add in some of the comments,
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
Fully agreed on both since the first thing I've checked is that the
enable_ppgtt option is indeed 0400 ;-)
Jani, can you please apply Chris' commit message text and comment when
merging?
-Daniel
Here as well:
Reviewed-by: Ben Widawsky b...@bwidawsk.net
drivers/gpu/drm/i915/i915_gem_gtt.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0d514ff9b94c..47491c4a1181 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -34,25 +34,35 @@ static void gen8_setup_private_ppat(struct
drm_i915_private *dev_priv);
bool intel_enable_ppgtt(struct drm_device *dev, bool full)
{
- if (i915.enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
+ if (i915.enable_ppgtt == 0)
return false;
if (i915.enable_ppgtt == 1 full)
return false;
+ return true;
+}
+
/* i915.enable_ppgtt is read-only, so do an early pass to validate
* the user's requested state against the hardware/driver capabilities.
* We do this now so that we can print out any log messages once rather
* than every time we check intel_enable_ppgtt().
*/
+static int sanitize_enable_ppgtt(struct drm_device *dev, int
enable_ppgtt)
+{
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
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] [RFC] xf86-video-intel: enable hw-generated binding tables
On Wed, Apr 23, 2014 at 06:52:09PM +0200, Daniel Vetter wrote: On Wed, Apr 23, 2014 at 1:21 PM, Abdiel Janulgue abdiel.janul...@linux.intel.com wrote: I've already tried disabling RS at the end of every batch so that next batch in different context can continue to use older non-RS format. That does not work either and still causes hangs. What I've seen so far, it seems GPU does not like switching the format of commands from RS-format to non-RS format. It's either one way or the other. If switched on, it affects subsequent contexes henceforth expecting RS-format commands until the GPU gets reset. That's probably the note in bspec: the binding table generator feature has a simple all or nothing model. Oh hooray, that's just awesome :( So we indeed need to stop the gpu and reset it if there's a non-RS render batch after any RS render batch. I'm not sure I buy it. There's an example in the spec showing how to disable the RS around eg. GPGPU workloads and re-enable it for 3D workloads even within one batch. I guess it's possible that the GPGPU pipeline mode is somehow special here, but since the RS state is supposed to be saved to the context I'm thinking you should be able to disable it whenever you want. What's really confusing with that example is the fact that it first re-enables the binding tables and then the RS, but then there's a note after that saying you have to those steps in the opposite order. Anyhoo, I'm wondering if the problems are simply due to disabling RS but leaving the binding tables enabled? So one idea might be that when we switch from executing an RS batch to a non-RS batch, we should disable the RS and binding tables manually. We would then have to demand that any user batch which needs the binding tables enabled must enable them, even if if we just executed a batch that already used the binding tables. And when we need to disable the RS and binding tables we could either have the kernel do that autmagically when it notices a RS-non-RS transition, or we could also demand that all user batches must disable the binding tables at the end. But maybe demanding that from every batch would incur some extra overhead? In any case it should be fairly easy to try that and see if it cures the hangs. Or are you already doing things that way? -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] tests/gem_userptr_blits: Expanded userptr test cases
On Wed, Apr 23, 2014 at 08:32:27AM -0700, Volkin, Bradley D wrote: On Wed, Apr 23, 2014 at 06:33:40AM -0700, Tvrtko Ursulin wrote: On 04/18/2014 06:10 PM, Volkin, Bradley D wrote: On Wed, Mar 19, 2014 at 04:13:04AM -0700, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com A set of userptr test cases to support the new feature. For the eviction and swapping stress testing I have extracted some common behaviour from gem_evict_everything and made both test cases use it to avoid duplicating the code. Both unsynchronized and synchronized userptr objects are tested but the latter set of tests will be skipped if kernel is compiled without MMU_NOTIFIERS. Also, with 32-bit userspace swapping tests are skipped if the system has a lot more RAM than process address space. Forking swapping tests are not skipped since they can still trigger swapping by cumulative effect. v2: * Fixed dmabuf test. * Added test for rejecting read-only. * Fixed ioctl detection for latest kernel patch. v3: * Updated copy() for Gen8+. * Fixed ioctl detection on kernels without MMU_NOTIFIERs. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com A number of the comments I made on patch 3 apply here as well. The sizeof(linear) thing is more prevalent in this test, though it looks like linear is at least used. Other than those comments this looks good to me. Believe it or not that sizeof(linear) idiom I inherited from other blitter tests. Personally I don't care one way or another. But since it makes sense to get rid of it for the benchmark part, perhaps I should change it here as well to be consistent. How strongly do you feel strongly about this? I think changing it would be slightly more readable, but if it's consistent with other blit tests then I don't feel too strongly about it. In fact, consistency with the other tests might be the better approach. I'm fine with whichever approach you prefer. Some of the igt tests are so Gross Hacks that justifying ugliness in new tests with consistency is ill-advised ;-) If you find some spare cycles to clean up existing tests that would be awesome, but I don't mind if they keep being ugly. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Sanitize the enable_ppgtt module option once
Otherwise we'll end up spamming dmesg on every context creation on snb
with vt-d enabled. This regression was introduced in
commit 246cbfb5fb9a1ca0997fbb135464c1ff5bb9c549
Author: Ben Widawsky benjamin.widaw...@intel.com
Date: Fri Dec 6 14:11:14 2013 -0800
drm/i915: Reorganize intel_enable_ppgtt
As the i915.enable_ppgtt is read-only it cannot be changed after the
module is loaded and so we can perform an early sanitization of the
values.
v2:
- Add comment and pimp commit message (Chris)
- Use the param consistently (Jani)
References: https://lkml.org/lkml/2014/4/17/599
Cc: Alessandro Suardi alessandro.sua...@gmail.com
Cc: Ben Widawsky b...@bwidawsk.net
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
Reviewed-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 32 +---
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0d514ff9b94c..70e8892f81a6 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -34,25 +34,35 @@ static void gen8_setup_private_ppat(struct drm_i915_private
*dev_priv);
bool intel_enable_ppgtt(struct drm_device *dev, bool full)
{
- if (i915.enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
+ if (i915.enable_ppgtt == 0)
return false;
if (i915.enable_ppgtt == 1 full)
return false;
+ return true;
+}
+
+static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
+{
+ if (enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
+ return 0;
+
+ if (enable_ppgtt == 1)
+ return 1;
+
+ if (enable_ppgtt == 2 HAS_PPGTT(dev))
+ return 2;
+
#ifdef CONFIG_INTEL_IOMMU
/* Disable ppgtt on SNB if VT-d is on. */
if (INTEL_INFO(dev)-gen == 6 intel_iommu_gfx_mapped) {
DRM_INFO(Disabling PPGTT because VT-d is on\n);
- return false;
+ return 0;
}
#endif
- /* Full ppgtt disabled by default for now due to issues. */
- if (full)
- return HAS_PPGTT(dev) (i915.enable_ppgtt == 2);
- else
- return HAS_ALIASING_PPGTT(dev);
+ return HAS_ALIASING_PPGTT(dev) ? 1 : 0;
}
@@ -1157,6 +1167,14 @@ int i915_gem_init_ppgtt(struct drm_device *dev, struct
i915_hw_ppgtt *ppgtt)
struct drm_i915_private *dev_priv = dev-dev_private;
int ret = 0;
+ /*
+* i915.enable_ppgtt is read-only, so do an early pass to validate the
+* user's requested state against the hardware/driver capabilities. We
+* do this now so that we can print out any log messages once rather
+* than every time we check intel_enable_ppgtt().
+*/
+ i915.enable_ppgtt = sanitize_enable_ppgtt(dev, i915.enable_ppgtt);
+
ppgtt-base.dev = dev;
ppgtt-base.scratch = dev_priv-gtt.base.scratch;
--
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: Sanitize the enable_ppgtt module option once
On Wed, Apr 23, 2014 at 11:37:46AM +0300, Jani Nikula wrote:
On Tue, 22 Apr 2014, Daniel Vetter daniel.vet...@ffwll.ch wrote:
Otherwise we'll end up spamming dmesg on every context creation on snb
with vt-d enabled. This regression was introduced in
commit 246cbfb5fb9a1ca0997fbb135464c1ff5bb9c549
Author: Ben Widawsky benjamin.widaw...@intel.com
Date: Fri Dec 6 14:11:14 2013 -0800
drm/i915: Reorganize intel_enable_ppgtt
References: https://lkml.org/lkml/2014/4/17/599
Cc: Alessandro Suardi alessandro.sua...@gmail.com
Cc: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0d514ff9b94c..47491c4a1181 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -34,25 +34,35 @@ static void gen8_setup_private_ppat(struct
drm_i915_private *dev_priv);
bool intel_enable_ppgtt(struct drm_device *dev, bool full)
{
- if (i915.enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
+ if (i915.enable_ppgtt == 0)
return false;
if (i915.enable_ppgtt == 1 full)
return false;
+ return true;
+}
+
+static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
+{
+ if (enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
+ return 0;
+
+ if (i915.enable_ppgtt == 1)
+ return 1;
+
+ if (i915.enable_ppgtt == 2 HAS_PPGTT(dev))
+ return 2;
You should probably either pass enable_ppgtt as parameter and use that
exclusively, or not pass it and refer to i915.enable_ppgtt directly, but
not mix them.
New patch on its way with Chris' comments also applied.
+
#ifdef CONFIG_INTEL_IOMMU
/* Disable ppgtt on SNB if VT-d is on. */
if (INTEL_INFO(dev)-gen == 6 intel_iommu_gfx_mapped) {
DRM_INFO(Disabling PPGTT because VT-d is on\n);
- return false;
+ return 0;
}
#endif
- /* Full ppgtt disabled by default for now due to issues. */
- if (full)
- return HAS_PPGTT(dev) (i915.enable_ppgtt == 2);
- else
- return HAS_ALIASING_PPGTT(dev);
+ return HAS_ALIASING_PPGTT(dev) ? 1 : 0;
This conflicts in -fixes due to
commit 8d214b7d9c45f4af23ce41b2bc74f79c44f760de
Author: Ben Widawsky benjamin.widaw...@linux.intel.com
Date: Mon Mar 24 18:06:00 2014 -0700
drm/i915: Allow full PPGTT with param override
Should I incorporate that in the conflict resolution for simplicity,
letting 3.15 users also play with full ppgtt with the module param?
Yeah I guess it's easiest to just cherry-pick that one to -fixes, too.
-Daniel
BR,
Jani.
}
@@ -1157,6 +1167,8 @@ int i915_gem_init_ppgtt(struct drm_device *dev,
struct i915_hw_ppgtt *ppgtt)
struct drm_i915_private *dev_priv = dev-dev_private;
int ret = 0;
+ i915.enable_ppgtt = sanitize_enable_ppgtt(dev, i915.enable_ppgtt);
+
ppgtt-base.dev = dev;
ppgtt-base.scratch = dev_priv-gtt.base.scratch;
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 0/3] render state initialization (bdw rc6)
On Tue, Apr 22, 2014 at 09:51:56PM +0100, Chris Wilson wrote: On Tue, Apr 22, 2014 at 08:19:41PM +0300, Mika Kuoppala wrote: Hi, Here are patches to initialize first render context to a hopefully, valid state. If pipeline/context is not initialized and we enter rc6 on bdw, the render ring can hung on the first batch submitted. That is atleast the hypothesis this work is based on. The states are stripped from rendercopy_genX's from i-g-t/lib and the state generators are part of i-g-t also. The states are propably overshoot from what can be consider the minimal valid (null) state on the pipeline. I just initialized everything rendercopy does and haven't really put effort on optimizing the state until I get some test results that this indeed solves anything. The state generators can be found here but they are not needed for testing. http://cgit.freedesktop.org/~miku/intel-gpu-tools/log/?h=null_state_gen Gen7 and Gen8 seems to atleast survive the boot but Gen6 is totally untested. Here is the branch for testing: http://cgit.freedesktop.org/~miku/drm-intel/log/?h=render_state I am interested to know if these patches make matters better/worse for those who have issues with first batch hanging on bdw, but as always, any feedback is greatly appreciated. Mika Kuoppala (3): drm/i915: export vmap_batch from command parser It's only a single page, it does not need to be vmapped. drm/i915: add render state initialization drm/i915: add null render states for gen6, gen7 and gen8 I still don't buy that this is anything more than papering over a problem. The state you load into the context is invalid as soon as it is executed, which may lead to problems, we don't know since the problem is not being discussed, and it will certainly be more explicit if the right bits are poked into the context directly to keep the hw from falling over. -Chris Paper is better than no paper. Anyway there are a couple of units where we know NULL is better than not NULL (VFE is one). I have been unable to get an exact reason why this is needed so that we know exactly what to fix. It has been a very frustrating experience. We could try to get info on what Windows does, but we may not be able to get a, why. -- 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 1/3] drm: Check CRTC compatibility in setplane
On Wed, Apr 23, 2014 at 10:03:59AM -0700, Matt Roper wrote:
The DRM core setplane code should check that the plane is usable on the
specified CRTC before calling into the driver.
Prior to this patch, a plane's possible_crtcs field was purely
informational for userspace and was never actually verified at the
kernel level (aside from the primary plane helper).
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
Do you have a nasty igt somewhere which tries to use a plane on the wrong
crtc? Especially useful since our i915 code and hw relies on this.
-Daniel
---
drivers/gpu/drm/drm_crtc.c | 7 +++
drivers/gpu/drm/drm_plane_helper.c | 6 --
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 461d19b..b6d6c04 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2140,6 +2140,13 @@ int drm_mode_setplane(struct drm_device *dev, void
*data,
}
crtc = obj_to_crtc(obj);
+ /* Check whether this plane is usable on this CRTC */
+ if (!(plane-possible_crtcs drm_crtc_mask(crtc))) {
+ DRM_DEBUG_KMS(Invalid crtc for plane\n);
+ ret = -EINVAL;
+ goto out;
+ }
+
fb = drm_framebuffer_lookup(dev, plane_req-fb_id);
if (!fb) {
DRM_DEBUG_KMS(Unknown framebuffer ID %d\n,
diff --git a/drivers/gpu/drm/drm_plane_helper.c
b/drivers/gpu/drm/drm_plane_helper.c
index b72736d..65c4a00 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -137,12 +137,6 @@ int drm_primary_helper_update(struct drm_plane *plane,
struct drm_crtc *crtc,
return -EINVAL;
}
- /* Primary planes are locked to their owning CRTC */
- if (plane-possible_crtcs != drm_crtc_mask(crtc)) {
- DRM_DEBUG_KMS(Cannot change primary plane CRTC\n);
- return -EINVAL;
- }
-
/* Disallow scaling */
src_w = 16;
src_h = 16;
--
1.8.5.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: Check CRTC compatibility in setplane
On Wed, Apr 23, 2014 at 08:03:50PM +0200, Daniel Vetter wrote:
On Wed, Apr 23, 2014 at 10:03:59AM -0700, Matt Roper wrote:
The DRM core setplane code should check that the plane is usable on the
specified CRTC before calling into the driver.
Prior to this patch, a plane's possible_crtcs field was purely
informational for userspace and was never actually verified at the
kernel level (aside from the primary plane helper).
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
Do you have a nasty igt somewhere which tries to use a plane on the wrong
crtc? Especially useful since our i915 code and hw relies on this.
-Daniel
Not yet; I'll add/modify a test to include that. I'm still working on
some other i-g-t test updates for the primary plane stuff based on your
previous feedback.
Speaking of i-g-t testing, what is the expected behavior if someone
issues a pageflip ioctl while the primary plane is disabled? I'd expect
it to return an error, but the -EBUSY currently returned by the DRM core
seems a bit confusing/misleading in my opinion. The comments indicate
that the existing assumption is that crtc-primary-fb == NULL indicates
a hotplug event that userspace hasn't noticed yet, although now we
clearly have other ways to hit that error, so I'm not sure EBUSY is
really the right response.
Matt
---
drivers/gpu/drm/drm_crtc.c | 7 +++
drivers/gpu/drm/drm_plane_helper.c | 6 --
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 461d19b..b6d6c04 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2140,6 +2140,13 @@ int drm_mode_setplane(struct drm_device *dev, void
*data,
}
crtc = obj_to_crtc(obj);
+ /* Check whether this plane is usable on this CRTC */
+ if (!(plane-possible_crtcs drm_crtc_mask(crtc))) {
+ DRM_DEBUG_KMS(Invalid crtc for plane\n);
+ ret = -EINVAL;
+ goto out;
+ }
+
fb = drm_framebuffer_lookup(dev, plane_req-fb_id);
if (!fb) {
DRM_DEBUG_KMS(Unknown framebuffer ID %d\n,
diff --git a/drivers/gpu/drm/drm_plane_helper.c
b/drivers/gpu/drm/drm_plane_helper.c
index b72736d..65c4a00 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -137,12 +137,6 @@ int drm_primary_helper_update(struct drm_plane *plane,
struct drm_crtc *crtc,
return -EINVAL;
}
- /* Primary planes are locked to their owning CRTC */
- if (plane-possible_crtcs != drm_crtc_mask(crtc)) {
- DRM_DEBUG_KMS(Cannot change primary plane CRTC\n);
- return -EINVAL;
- }
-
/* Disallow scaling */
src_w = 16;
src_h = 16;
--
1.8.5.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: Check CRTC compatibility in setplane
On Wed, Apr 23, 2014 at 11:26:04AM -0700, Matt Roper wrote: On Wed, Apr 23, 2014 at 08:03:50PM +0200, Daniel Vetter wrote: On Wed, Apr 23, 2014 at 10:03:59AM -0700, Matt Roper wrote: The DRM core setplane code should check that the plane is usable on the specified CRTC before calling into the driver. Prior to this patch, a plane's possible_crtcs field was purely informational for userspace and was never actually verified at the kernel level (aside from the primary plane helper). Cc: dri-de...@lists.freedesktop.org Signed-off-by: Matt Roper matthew.d.ro...@intel.com Do you have a nasty igt somewhere which tries to use a plane on the wrong crtc? Especially useful since our i915 code and hw relies on this. -Daniel Not yet; I'll add/modify a test to include that. I'm still working on some other i-g-t test updates for the primary plane stuff based on your previous feedback. Speaking of i-g-t testing, what is the expected behavior if someone issues a pageflip ioctl while the primary plane is disabled? I'd expect it to return an error, but the -EBUSY currently returned by the DRM core seems a bit confusing/misleading in my opinion. The comments indicate that the existing assumption is that crtc-primary-fb == NULL indicates a hotplug event that userspace hasn't noticed yet, although now we clearly have other ways to hit that error, so I'm not sure EBUSY is really the right response. That comment is outdated since nowadays the kernel doesn't randomly kill a crtc if its outputs gets unplugged. I think a simple -EINVAL should be good. We'd need to update kms_flip with that new value though. A quick grep through the intel ddx shows that we don't really depend on this either way. -EBUSY for a disabled primary plane (whether the crtc is on or not doesn't matter imo) really makes no sense. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/plane-helper: Add drm_primary_helper_check_update()
On Wed, Apr 23, 2014 at 10:04:00AM -0700, Matt Roper wrote:
Pull the parameter checking from drm_primary_helper_update() out into
its own function; drivers that provide their own setplane()
implementations rather than using the helper may still want to share
this parameter checking logic.
A few of the checks here were also updated based on suggestions by
Ville Syrjälä.
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
drivers/gpu/drm/drm_plane_helper.c | 148
+
include/drm/drm_plane_helper.h | 9 +++
2 files changed, 128 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/drm_plane_helper.c
b/drivers/gpu/drm/drm_plane_helper.c
index 65c4a00..9bbbdf2 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -66,6 +66,102 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
}
/**
+ * drm_primary_helper_check_update() - Check primary plane update for
validity
+ * @plane: plane object to update
+ * @crtc: owning CRTC of owning plane
+ * @fb: framebuffer to flip onto plane
+ * @crtc_x: x offset of primary plane on crtc
+ * @crtc_y: y offset of primary plane on crtc
+ * @crtc_w: width of primary plane rectangle on crtc
+ * @crtc_h: height of primary plane rectangle on crtc
+ * @src_x: x offset of @fb for panning
+ * @src_y: y offset of @fb for panning
+ * @src_w: width of source rectangle in @fb
+ * @src_h: height of source rectangle in @fb
+ * @can_scale: is primary plane scaling legal?
+ * @can_position: is it legal to position the primary plane such that it
+ *doesn't cover the entire crtc?
+ *
+ * Checks that a desired primary plane update is valid. Drivers that provide
+ * their own primary plane handling may still wish to call this function to
+ * avoid duplication of error checking code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_primary_helper_check_update(struct drm_plane *plane,
+ struct drm_crtc *crtc,
+ struct drm_framebuffer *fb,
+ int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y,
+ uint32_t src_w, uint32_t src_h,
+ bool can_scale,
+ bool can_position)
+{
+ struct drm_rect dest = {
+ .x1 = crtc_x,
+ .y1 = crtc_y,
+ .x2 = crtc_x + crtc_w,
+ .y2 = crtc_y + crtc_h,
+ };
+ struct drm_rect src = {
+ .x1 = src_x 16,
+ .y1 = src_y 16,
+ .x2 = (src_x + src_w) 16,
+ .y2 = (src_y + src_h) 16,
I think you want '(x16) + (y16)' instead. Otherwise you may end up
increasing the source width/height.
+ };
+ const struct drm_rect clip = {
+ .x2 = crtc-mode.hdisplay,
+ .y2 = crtc-mode.vdisplay,
+ };
+ int hscale, vscale;
+ int visible;
+
+ if (!crtc-enabled) {
+ DRM_DEBUG_KMS(Cannot update primary plane of a disabled
CRTC.\n);
+ return -EINVAL;
+ }
We allow this for sprites, so I'd allow it for everything. I'd be fine
with leaving this restriction in drm_primary_helper_update() simply
because I have no interst in auditing every other driver.
Although I think we still overwrite the primary plane configure during
setcrtc. We should really change that so that the user can pre-configure
all the planes and then watch them pop into view during a modeset as
previously configured.
+
+ /* setplane API takes shifted source rectangle values; unshift them */
+ src_x = 16;
+ src_y = 16;
+ src_w = 16;
+ src_h = 16;
+
+ /* check scaling */
+ if (!can_scale (crtc_w != src_w || crtc_h != src_h)) {
+ DRM_DEBUG_KMS(Can't scale primary plane\n);
+ return -EINVAL;
+ }
+
+ /*
+ * Drivers that can scale should perform their own min/max scale
+ * factor checks.
+ */
+ hscale = drm_rect_calc_hscale(src, dest, 0, INT_MAX);
+ vscale = drm_rect_calc_vscale(src, dest, 0, INT_MAX);
+ visible = drm_rect_clip_scaled(src, dest, clip, hscale, vscale);
w/o sub-pixel coordinates the scaling factors will be truncated
to integers, which makes the clipping rather bogus if the plane can
actually scale. I think I'd just make this code assume that scaling
isn't supported, and if the driver supports scaling it can just
implent the appropriate code itself. We can try to refactor some of
the scaling aware code from intel_sprite later if warranted.
But I'm starting to question the usefulness of this function. We
anyway have to
Re: [Intel-gfx] [PATCH 05/24] drm/i915: Merge LP1+ watermarks in safer way
2014-03-07 13:32 GMT-03:00 ville.syrj...@linux.intel.com:
From: Ville Syrjälä ville.syrj...@linux.intel.com
On ILK when we disable a particular watermark level, we must
Just to be clear: do you mean ILK or ILK+ here?
maintain the actual watermark values for that level for some time
(until the next vblank possibly). Otherwise we risk underruns.
In order to achieve that result we must merge the LP1+ watermarks a
bit differently since we must also merge levels that are to be
disabled. We must also make sure we don't overflow the fields in the
watermark registers in case the calculated watermarks come out too
big to fit.
As early as possbile we mark all computed watermark levels as
disabled if they would exceed the register maximums. We make sure
to leave the actual watermarks for such levels zeroed out. The during
merging, we take the maxium values for every level, regardless if
they're disabled or not. That may seem a bit pointless since at the
moment all the watermark levels we merge should have their values
zeroed if the level is already disabled. However soon we will be
dealing with intermediate watermarks that, in addition to the new
watermark values, also contain the previous watermark values, and so
levels that are disabled may no longer be zeroed out.
I am having a hard time here. Watermarks code is extremely complex
these days, and my brain does not have enough memory to think about
all the implications and side effects of all the changes you did here.
I have stared a this patch for a long time, and I don't think I fully
understand it. I think you should probably try to break this change
into some smaller steps, with nice commit messages. Some stupid
questions to help me clarify:
As far as I understood, the biggest change of this patch is that when
some WM level is disabled, we won't write 0x0 to the WM register, but
we will write actual values, but with bit 31 set to zero, right? On
the first sentence of the commit message, you say we must maintain the
old values for that level for some time, but on the current code we
won't keep the old values: we will generate new values, and if a level
that was previously enabled needs to be disabled, there's no guarantee
that the new value we will generate will be exactly the same as the
old-value-but-with-bit-31-disabled. Why does that work? Since it
doesn't make sense to me, I probably understood something very wrong
here...
A little more below...
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 81
-
1 file changed, 64 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f061ef1..ba4b23e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1921,6 +1921,16 @@ static void ilk_compute_wm_maximums(const struct
drm_device *dev,
max-fbc = ilk_fbc_wm_reg_max(dev);
}
+static void ilk_compute_wm_reg_maximums(struct drm_device *dev,
+ int level,
+ struct ilk_wm_maximums *max)
+{
+ max-pri = ilk_plane_wm_reg_max(dev, level, false);
+ max-spr = ilk_plane_wm_reg_max(dev, level, true);
+ max-cur = ilk_cursor_wm_reg_max(dev, level);
+ max-fbc = ilk_fbc_wm_reg_max(dev);
+}
+
static bool ilk_validate_wm_level(int level,
const struct ilk_wm_maximums *max,
struct intel_wm_level *result)
@@ -2178,9 +2188,6 @@ static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
};
struct ilk_wm_maximums max;
- /* LP0 watermarks always use 1/2 DDB partitioning */
- ilk_compute_wm_maximums(dev, 0, config, INTEL_DDB_PART_1_2, max);
-
pipe_wm-pipe_enabled = params-active;
pipe_wm-sprites_enabled = params-spr.enabled;
pipe_wm-sprites_scaled = params-spr.scaled;
@@ -2193,15 +2200,37 @@ static bool intel_compute_pipe_wm(struct drm_crtc
*crtc,
if (params-spr.scaled)
max_level = 0;
- for (level = 0; level = max_level; level++)
- ilk_compute_wm_level(dev_priv, level, params,
-pipe_wm-wm[level]);
+ ilk_compute_wm_level(dev_priv, 0, params, pipe_wm-wm[0]);
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
pipe_wm-linetime = hsw_compute_linetime_wm(dev, crtc);
+ /* LP0 watermarks always use 1/2 DDB partitioning */
+ ilk_compute_wm_maximums(dev, 0, config, INTEL_DDB_PART_1_2, max);
+
/* At least LP0 must be valid */
- return ilk_validate_wm_level(0, max, pipe_wm-wm[0]);
+ if (!ilk_validate_wm_level(0, max, pipe_wm-wm[0]))
+ return false;
+
For example, this chunk introduces an early return to the function. I
really think this should be on its own patch, with a
[Intel-gfx] [PATCH] tests: Add gem_exec_params
This fills all the gaps we've had in our execbuf testing. Overflow
testing of the various arrays is already done by gem_reloc_overflow.
Also add kms_flip_tiling to .gitignore.
This will cause a bunch of failures since current kernels don't catch
all fallout.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
tests/.gitignore| 2 +
tests/Makefile.sources | 1 +
tests/gem_exec_params.c | 212
3 files changed, 215 insertions(+)
create mode 100644 tests/gem_exec_params.c
diff --git a/tests/.gitignore b/tests/.gitignore
index 146bab06b565..4c50bae93aa3 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -35,6 +35,7 @@ gem_exec_blt
gem_exec_faulting_reloc
gem_exec_lut_handle
gem_exec_nop
+gem_exec_params
gem_exec_parse
gem_fd_exhaustion
gem_fenced_exec_thrash
@@ -113,6 +114,7 @@ kms_addfb
kms_cursor_crc
kms_fbc_crc
kms_flip
+kms_flip_tiling
kms_pipe_crc_basic
kms_plane
kms_render
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c957ace2ace0..9b2d7cff1113 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -29,6 +29,7 @@ TESTS_progs_M = \
gem_exec_bad_domains \
gem_exec_faulting_reloc \
gem_exec_nop \
+ gem_exec_params \
gem_exec_parse \
gem_fenced_exec_thrash \
gem_fence_thrash \
diff --git a/tests/gem_exec_params.c b/tests/gem_exec_params.c
new file mode 100644
index ..b1d996c530f5
--- /dev/null
+++ b/tests/gem_exec_params.c
@@ -0,0 +1,212 @@
+/*
+ * Copyright (c) 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Daniel Vetter
+ *
+ */
+
+#include unistd.h
+#include stdlib.h
+#include stdint.h
+#include stdio.h
+#include string.h
+#include fcntl.h
+#include inttypes.h
+#include errno.h
+#include sys/stat.h
+#include sys/ioctl.h
+#include sys/time.h
+#include drm.h
+
+#include ioctl_wrappers.h
+#include drmtest.h
+#include intel_io.h
+#include intel_chipset.h
+#include igt_aux.h
+
+#define LOCAL_I915_EXEC_VEBOX (40)
+
+struct drm_i915_gem_execbuffer2 execbuf;
+struct drm_i915_gem_exec_object2 gem_exec[1];
+uint32_t batch[2] = {MI_BATCH_BUFFER_END};
+uint32_t handle, devid;
+int fd;
+
+igt_main
+{
+ igt_fixture {
+ fd = drm_open_any();
+
+ devid = intel_get_drm_devid(fd);
+
+ handle = gem_create(fd, 4096);
+ gem_write(fd, handle, 0, batch, sizeof(batch));
+
+ gem_exec[0].handle = handle;
+ gem_exec[0].relocation_count = 0;
+ gem_exec[0].relocs_ptr = 0;
+ gem_exec[0].alignment = 0;
+ gem_exec[0].offset = 0;
+ gem_exec[0].flags = 0;
+ gem_exec[0].rsvd1 = 0;
+ gem_exec[0].rsvd2 = 0;
+
+ execbuf.buffers_ptr = (uintptr_t)gem_exec;
+ execbuf.buffer_count = 1;
+ execbuf.batch_start_offset = 0;
+ execbuf.batch_len = 8;
+ execbuf.cliprects_ptr = 0;
+ execbuf.num_cliprects = 0;
+ execbuf.DR1 = 0;
+ execbuf.DR4 = 0;
+ execbuf.flags = 0;
+ i915_execbuffer2_set_context_id(execbuf, 0);
+ execbuf.rsvd2 = 0;
+ }
+
+ igt_subtest(control) {
+ igt_assert(drmIoctl(fd,
+ DRM_IOCTL_I915_GEM_EXECBUFFER2,
+ execbuf) == 0);
+ execbuf.flags = I915_EXEC_RENDER;
+ igt_assert(drmIoctl(fd,
+ DRM_IOCTL_I915_GEM_EXECBUFFER2,
+ execbuf) == 0);
+ }
+
+#define RUN_FAIL(expected_errno) do { \
+ igt_assert(drmIoctl(fd, \
+ DRM_IOCTL_I915_GEM_EXECBUFFER2, \
+ execbuf)
[Intel-gfx] [PATCH 3/3] drm/i915: Catch dirt in unused execbuffer fields
We need to make sure that userspace keeps on following the contract,
otherwise we won't be able to use the reserved fields at all.
Testcase: igt/gem_exec_params/*-dirt
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index c2e5d39a1df8..0f0aebdd8dbd 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1115,6 +1115,9 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
ret = -EFAULT;
goto pre_mutex_err;
}
+ } else {
+ if (args-DR1 || args-DR4 || args-cliprects_ptr)
+ return -EINVAL;
}
intel_runtime_pm_get(dev_priv);
@@ -1392,6 +1395,9 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
return -EINVAL;
}
+ if (args-rsvd2 != 0)
+ return -EINVAL;
+
exec2_list = kmalloc(sizeof(*exec2_list)*args-buffer_count,
GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY);
if (exec2_list == NULL)
--
1.8.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: Catch abuse of I915_EXEC_GEN7_SOL_RESET
Currently we catch it, but silently succeed. Our userspace is better than this. Testcase: igt/gem_exec_params/sol-reset-* Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 0ec8621eb4f8..40ae5ff0a031 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -982,7 +982,7 @@ i915_reset_gen7_sol_offsets(struct drm_device *dev, int ret, i; if (!IS_GEN7(dev) || ring != dev_priv-ring[RCS]) - return 0; + return -EINVAL; ret = intel_ring_begin(ring, 4 * 3); if (ret) -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Catch abuse of I915_EXEC_CONSTANTS_*
A bit tricky since 0 is also a valid constant ...
Testcase: igt/gem_exec_params/rel-constants-*
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 40ae5ff0a031..c2e5d39a1df8 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1058,8 +1058,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
case I915_EXEC_CONSTANTS_REL_GENERAL:
case I915_EXEC_CONSTANTS_ABSOLUTE:
case I915_EXEC_CONSTANTS_REL_SURFACE:
- if (ring == dev_priv-ring[RCS]
- mode != dev_priv-relative_constants_mode) {
+ if (mode != 0 ring != dev_priv-ring[RCS])
+ return -EINVAL;
+
+ if (mode != dev_priv-relative_constants_mode) {
if (INTEL_INFO(dev)-gen 4)
return -EINVAL;
--
1.8.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/24] drm/i915: Merge LP1+ watermarks in safer way
On Wed, Apr 23, 2014 at 04:13:25PM -0300, Paulo Zanoni wrote:
2014-03-07 13:32 GMT-03:00 ville.syrj...@linux.intel.com:
From: Ville Syrjälä ville.syrj...@linux.intel.com
On ILK when we disable a particular watermark level, we must
Just to be clear: do you mean ILK or ILK+ here?
IIRC ILK was where I saw it really easily. IVB didn't seem quite as
sensitive to it.
maintain the actual watermark values for that level for some time
(until the next vblank possibly). Otherwise we risk underruns.
In order to achieve that result we must merge the LP1+ watermarks a
bit differently since we must also merge levels that are to be
disabled. We must also make sure we don't overflow the fields in the
watermark registers in case the calculated watermarks come out too
big to fit.
As early as possbile we mark all computed watermark levels as
disabled if they would exceed the register maximums. We make sure
to leave the actual watermarks for such levels zeroed out. The during
merging, we take the maxium values for every level, regardless if
they're disabled or not. That may seem a bit pointless since at the
moment all the watermark levels we merge should have their values
zeroed if the level is already disabled. However soon we will be
dealing with intermediate watermarks that, in addition to the new
watermark values, also contain the previous watermark values, and so
levels that are disabled may no longer be zeroed out.
I am having a hard time here. Watermarks code is extremely complex
these days, and my brain does not have enough memory to think about
all the implications and side effects of all the changes you did here.
I have stared a this patch for a long time, and I don't think I fully
understand it. I think you should probably try to break this change
into some smaller steps, with nice commit messages. Some stupid
questions to help me clarify:
As far as I understood, the biggest change of this patch is that when
some WM level is disabled, we won't write 0x0 to the WM register, but
we will write actual values, but with bit 31 set to zero, right?
That's the idea.
On
the first sentence of the commit message, you say we must maintain the
old values for that level for some time, but on the current code we
won't keep the old values: we will generate new values, and if a level
that was previously enabled needs to be disabled, there's no guarantee
that the new value we will generate will be exactly the same as the
old-value-but-with-bit-31-disabled. Why does that work?
It's not really hooked up yet. There's going to be another patch down
the line that calculates the intermediate watermarks for an individual
pipe as max(old,new). Those intermediate watermarks get applied to the
affected pipe, and then as we merge the LP1+ watermarks from all pipes
we must use similar logic. Ie. if one pipe has/had LP2=100 but
another pipe has/had LP2 disabled we must write 100 to the register
(w/o the enable bit set of course).
So this patch just modifies the LP1+ merge logic to make that happen.
Another patch will deal with the old vs. new situation.
Since it
doesn't make sense to me, I probably understood something very wrong
here...
A little more below...
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 81
-
1 file changed, 64 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index f061ef1..ba4b23e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1921,6 +1921,16 @@ static void ilk_compute_wm_maximums(const struct
drm_device *dev,
max-fbc = ilk_fbc_wm_reg_max(dev);
}
+static void ilk_compute_wm_reg_maximums(struct drm_device *dev,
+ int level,
+ struct ilk_wm_maximums *max)
+{
+ max-pri = ilk_plane_wm_reg_max(dev, level, false);
+ max-spr = ilk_plane_wm_reg_max(dev, level, true);
+ max-cur = ilk_cursor_wm_reg_max(dev, level);
+ max-fbc = ilk_fbc_wm_reg_max(dev);
+}
+
static bool ilk_validate_wm_level(int level,
const struct ilk_wm_maximums *max,
struct intel_wm_level *result)
@@ -2178,9 +2188,6 @@ static bool intel_compute_pipe_wm(struct drm_crtc
*crtc,
};
struct ilk_wm_maximums max;
- /* LP0 watermarks always use 1/2 DDB partitioning */
- ilk_compute_wm_maximums(dev, 0, config, INTEL_DDB_PART_1_2, max);
-
pipe_wm-pipe_enabled = params-active;
pipe_wm-sprites_enabled = params-spr.enabled;
pipe_wm-sprites_scaled = params-spr.scaled;
@@ -2193,15 +2200,37 @@ static bool intel_compute_pipe_wm(struct drm_crtc
*crtc,
if (params-spr.scaled)
[Intel-gfx] 3.15-rc2: i915 regression: only top 20% of screen works in X
Hi! After update to 3.15-rc2, only top 20% of screen works on X. 00:02.0 VGA compatible controller: Intel Corporation 4 Series Chipset Integrated Graphics Controller (rev 03) 00:02.1 Display controller: Intel Corporation 4 Series Chipset Integrated Graphics Controller (rev 03) Subsystem: Intel Corporation Device d614 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast TAbort- TAbort- MAbort- SERR- PERR- INTx- Latency: 0 Region 0: Memory at d040 (64-bit, non-prefetchable) [size=1M] Capabilities: access denied This worked before. I believe it worked in 3.14. It definitely works in 3.11-rc2. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] 3.15-rc2: i915 regression: only top 20% of screen works in X
On Wed, Apr 23, 2014 at 10:22 PM, Pavel Machek pa...@ucw.cz wrote: After update to 3.15-rc2, only top 20% of screen works on X. 00:02.0 VGA compatible controller: Intel Corporation 4 Series Chipset Integrated Graphics Controller (rev 03) 00:02.1 Display controller: Intel Corporation 4 Series Chipset Integrated Graphics Controller (rev 03) Subsystem: Intel Corporation Device d614 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast TAbort- TAbort- MAbort- SERR- PERR- INTx- Latency: 0 Region 0: Memory at d040 (64-bit, non-prefetchable) [size=1M] Capabilities: access denied This worked before. I believe it worked in 3.14. It definitely works in 3.11-rc2. Screenshot or more detailed description of what only top 20% of screen works in X means? Anything in dmesg? bisect result presuming that it reproduces reliably? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] mm: Throttle shrinkers harder
On 04/22/2014 12:30 PM, Daniel Vetter wrote: During testing of i915.ko with working texture sets larger than RAM, we encounter OOM with plenty of memory still trapped within writeback, e.g: [ 42.386039] active_anon:10134 inactive_anon:1900781 isolated_anon:32 active_file:33 inactive_file:39 isolated_file:0 unevictable:0 dirty:0 writeback:337627 unstable:0 free:11985 slab_reclaimable:9458 slab_unreclaimable:23614 mapped:41 shmem:1560769 pagetables:1276 bounce:0 If we throttle for writeback following shrink_slab, this gives us time to wait upon the writeback generated by the i915.ko shinker: [ 4756.750808] active_anon:24386 inactive_anon:900793 isolated_anon:0 active_file:23 inactive_file:20 isolated_file:0 unevictable:0 dirty:0 writeback:0 unstable:0 free:5550 slab_reclaimable:5184 slab_unreclaimable:4888 mapped:3 shmem:472393 pagetables:1249 bounce:0 Could you get some dumps of the entire set of OOM information? These are only tiny snippets. Also, the vmstat output from the bug: https://bugs.freedesktop.org/show_bug.cgi?id=72742 shows there being an *AWFUL* lot of swap I/O going on here. From the looks of it, we stuck ~2GB in swap and evicted another 1.5GB of page cache (although I guess that could be double-counting tmpfs getting swapped out too). Hmmm, was this one of the cases where you actually ran _out_ of swap? 2 0 19472 33952296 36103240 19472 0 19472 1474 151 3 27 71 0 4 0 484964 66468296 31758640 465492 0 465516 2597 1395 0 32 66 2 0 2 751940 23692980 30228840 266976 688 266976 3681 636 0 27 66 6 procs ---memory-- ---swap-- -io -system-- cpu r b swpd free buff cache si sobibo in cs us sy id wa 2 1 1244580 295336988 26069840 492896 0 492908 1237 311 1 9 50 41 0 2 2047996 28760988 20371440 803160 0 803160 1221 1291 1 15 69 14 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/24] drm/i915: Check hw vs. sw watermark state after programming
2014-03-07 13:32 GMT-03:00 ville.syrj...@linux.intel.com:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Make sure we programmed the watermarks correctly, by reading out the
hardware state again after programming and comparing it with the
state we supposedly programmed into hardware. Dump the watermark
registers after a mismatch, very much like we for the pipe config.
The only difference is that we don't dump the entire watermark
software tracking state since that's spread around a bit.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
This one could also have been split into more than one patch: first
you extract the functions, then later you add the new callers.
My only comment is: do we really want to check HW/SW state right after
we write the register values? Shouldn't this be done at some other
time, like the end of the modeset sequence?
Still, the patch seems correct, so: Reviewed-by: Paulo Zanoni
paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 117
1 file changed, 83 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ba4b23e..e519578a1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2524,15 +2524,84 @@ static bool _ilk_disable_lp_wm(struct
drm_i915_private *dev_priv,
return changed;
}
+static void _ilk_pipe_wm_get_hw_state(struct drm_device *dev,
+ enum pipe pipe,
+ struct ilk_wm_values *hw)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ static const unsigned int wm0_pipe_reg[] = {
+ [PIPE_A] = WM0_PIPEA_ILK,
+ [PIPE_B] = WM0_PIPEB_ILK,
+ [PIPE_C] = WM0_PIPEC_IVB,
+ };
+
+ hw-wm_pipe[pipe] = I915_READ(wm0_pipe_reg[pipe]);
+
+ if (IS_HASWELL(dev) || IS_BROADWELL(dev))
+ hw-wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));
+}
+
+static void _ilk_wm_get_hw_state(struct drm_device *dev,
+struct ilk_wm_values *hw)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ enum pipe pipe;
+
+ for_each_pipe(pipe)
+ _ilk_pipe_wm_get_hw_state(dev, pipe, hw);
+
+ hw-wm_lp[0] = I915_READ(WM1_LP_ILK);
+ hw-wm_lp[1] = I915_READ(WM2_LP_ILK);
+ hw-wm_lp[2] = I915_READ(WM3_LP_ILK);
+
+ hw-wm_lp_spr[0] = I915_READ(WM1S_LP_ILK);
+ if (INTEL_INFO(dev)-gen = 7) {
+ hw-wm_lp_spr[1] = I915_READ(WM2S_LP_IVB);
+ hw-wm_lp_spr[2] = I915_READ(WM3S_LP_IVB);
+ }
+
+ if (IS_HASWELL(dev) || IS_BROADWELL(dev))
+ hw-partitioning = (I915_READ(WM_MISC)
WM_MISC_DATA_PARTITION_5_6) ?
+ INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
+ else if (IS_IVYBRIDGE(dev))
+ hw-partitioning = (I915_READ(DISP_ARB_CTL2)
DISP_DATA_PARTITION_5_6) ?
+ INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
+
+ hw-enable_fbc_wm =
+ !(I915_READ(DISP_ARB_CTL) DISP_FBC_WM_DIS);
+}
+
+static void ilk_dump_wm_values(const struct ilk_wm_values *hw,
+ const char *context)
+{
+ DRM_DEBUG_KMS(%s watermark values\n, context);
+ DRM_DEBUG_KMS(WM_PIPE_A = 0x%08x\n, hw-wm_pipe[PIPE_A]);
+ DRM_DEBUG_KMS(WM_PIPE_B = 0x%08x\n, hw-wm_pipe[PIPE_B]);
+ DRM_DEBUG_KMS(WM_PIPE_C = 0x%08x\n, hw-wm_pipe[PIPE_C]);
+ DRM_DEBUG_KMS(WM_LP_1 = 0x%08x\n, hw-wm_lp[0]);
+ DRM_DEBUG_KMS(WM_LP_2 = 0x%08x\n, hw-wm_lp[1]);
+ DRM_DEBUG_KMS(WM_LP_3 = 0x%08x\n, hw-wm_lp[2]);
+ DRM_DEBUG_KMS(WM_LP_SPR_1 = 0x%08x\n, hw-wm_lp_spr[0]);
+ DRM_DEBUG_KMS(WM_LP_SPR_2 = 0x%08x\n, hw-wm_lp_spr[1]);
+ DRM_DEBUG_KMS(WM_LP_SPR_3 = 0x%08x\n, hw-wm_lp_spr[2]);
+ DRM_DEBUG_KMS(WM_LINETIME_A = 0x%08x\n, hw-wm_linetime[PIPE_A]);
+ DRM_DEBUG_KMS(WM_LINETIME_B = 0x%08x\n, hw-wm_linetime[PIPE_B]);
+ DRM_DEBUG_KMS(WM_LINETIME_C = 0x%08x\n, hw-wm_linetime[PIPE_C]);
+ DRM_DEBUG_KMS(enable FBC watermark = %d\n, hw-enable_fbc_wm);
+ DRM_DEBUG_KMS(DDB partitioning = %s\n,
+ hw-partitioning == INTEL_DDB_PART_1_2 ? 1/2 : 5/6);
+}
+
/*
* The spec says we shouldn't write when we don't need, because every write
* causes WMs to be re-evaluated, expending some power.
*/
static void ilk_write_wm_values(struct drm_i915_private *dev_priv,
- struct ilk_wm_values *results)
+ const struct ilk_wm_values *results)
{
struct drm_device *dev = dev_priv-dev;
struct ilk_wm_values *previous = dev_priv-wm.hw;
+ struct ilk_wm_values hw = {};
unsigned int dirty;
uint32_t val;
@@ -2602,6 +2671,14 @@ static void
Re: [Intel-gfx] [PATCH 12/24] drm/i915: Refactor ilk_validate_pipe_wm()
2014-03-07 13:32 GMT-03:00 ville.syrj...@linux.intel.com:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Pull the LP0 validate out from intel_compute_pipe_wm() into a separate
function. We will have further need for such a function to validate
both the final watermarks and the intermediate watermarks we will be
using while the plane(s) transition between different configurations.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 30 +++---
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e519578a1..e142095 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2172,6 +2172,24 @@ static void ilk_compute_wm_config(struct drm_device
*dev,
}
}
+static bool ilk_validate_pipe_wm(struct drm_device *dev,
+struct intel_pipe_wm *pipe_wm)
+{
+ /* LP0 watermark maximums depend on this pipe alone */
+ const struct intel_wm_config config = {
+ .num_pipes_active = 1,
+ .sprites_enabled = pipe_wm-sprites_enabled,
+ .sprites_scaled = pipe_wm-sprites_scaled,
+ };
+ struct ilk_wm_maximums max;
+
+ /* LP0 watermarks always use 1/2 DDB partitioning */
+ ilk_compute_wm_maximums(dev, 0, config, INTEL_DDB_PART_1_2, max);
+
+ /* At least LP0 must be valid */
I would remove this comment, and keep the original one on the caller.
IMHO it makes more sense there.
With or without that change:
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
+ return ilk_validate_wm_level(0, max, pipe_wm-wm[0]);
+}
+
/* Compute new watermarks for the pipe */
static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
const struct ilk_pipe_wm_parameters *params,
@@ -2180,12 +2198,6 @@ static bool intel_compute_pipe_wm(struct drm_crtc
*crtc,
struct drm_device *dev = crtc-dev;
const struct drm_i915_private *dev_priv = dev-dev_private;
int level, max_level = ilk_wm_max_level(dev);
- /* LP0 watermark maximums depend on this pipe alone */
- struct intel_wm_config config = {
- .num_pipes_active = 1,
- .sprites_enabled = params-spr.enabled,
- .sprites_scaled = params-spr.scaled,
- };
struct ilk_wm_maximums max;
pipe_wm-pipe_enabled = params-active;
@@ -2205,11 +2217,7 @@ static bool intel_compute_pipe_wm(struct drm_crtc
*crtc,
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
pipe_wm-linetime = hsw_compute_linetime_wm(dev, crtc);
- /* LP0 watermarks always use 1/2 DDB partitioning */
- ilk_compute_wm_maximums(dev, 0, config, INTEL_DDB_PART_1_2, max);
-
- /* At least LP0 must be valid */
- if (!ilk_validate_wm_level(0, max, pipe_wm-wm[0]))
+ if (!ilk_validate_pipe_wm(dev, pipe_wm))
return false;
ilk_compute_wm_reg_maximums(dev, 1, max);
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/24] drm/i915: Refactor ilk_update_wm
2014-03-07 13:32 GMT-03:00 ville.syrj...@linux.intel.com:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Split ilk_update_wm() into two parts; one doing the progragramming
and the other the calculations.
There are 1.060 google results for the word progragramming, which
you used above :)
Sounds like one of those words that have some special funny meaning.
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 38 ++
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e142095..3f5c1dc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2696,27 +2696,14 @@ static bool ilk_disable_lp_wm(struct drm_device *dev)
return _ilk_disable_lp_wm(dev_priv, WM_DIRTY_LP_ALL);
}
-static void ilk_update_wm(struct drm_crtc *crtc)
+static void ilk_program_watermarks(struct drm_device *dev)
{
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct drm_device *dev = crtc-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
struct ilk_wm_maximums max;
- struct ilk_pipe_wm_parameters params = {};
+ struct intel_wm_config config = {};
struct ilk_wm_values results = {};
enum intel_ddb_partitioning partitioning;
- struct intel_pipe_wm pipe_wm = {};
- struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
- struct intel_wm_config config = {};
-
- ilk_compute_wm_parameters(crtc, params);
-
- intel_compute_pipe_wm(crtc, params, pipe_wm);
-
- if (!memcmp(intel_crtc-wm.active, pipe_wm, sizeof(pipe_wm)))
- return;
-
- intel_crtc-wm.active = pipe_wm;
ilk_compute_wm_config(dev, config);
@@ -2742,6 +2729,25 @@ static void ilk_update_wm(struct drm_crtc *crtc)
ilk_write_wm_values(dev_priv, results);
}
+static void ilk_update_wm(struct drm_crtc *crtc)
+{
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_device *dev = crtc-dev;
+ struct ilk_pipe_wm_parameters params = {};
+ struct intel_pipe_wm pipe_wm = {};
+
+ ilk_compute_wm_parameters(crtc, params);
+
+ intel_compute_pipe_wm(crtc, params, pipe_wm);
+
+ if (!memcmp(intel_crtc-wm.active, pipe_wm, sizeof(pipe_wm)))
+ return;
+
+ intel_crtc-wm.active = pipe_wm;
+
+ ilk_program_watermarks(dev);
+}
+
static void ilk_update_sprite_wm(struct drm_plane *plane,
struct drm_crtc *crtc,
uint32_t sprite_width, int pixel_size,
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 14/24] drm/i915: Add dev_priv-wm.mutex
2014-03-07 13:32 GMT-03:00 ville.syrj...@linux.intel.com:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Add a mutex to protect most of the watermark state. Will be useful when
we start to update watermarks asynchronously from plane updates, or
when we get finer grained locking for planes.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 11 ++-
drivers/gpu/drm/i915/intel_drv.h | 5 -
drivers/gpu/drm/i915/intel_pm.c | 21 +++--
3 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 29da39f..0b19723 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1595,8 +1595,17 @@ typedef struct drm_i915_private {
/* cursor */
uint16_t cur_latency[5];
- /* current hardware state */
+ /*
+* current hardware state
+* protected by dev_priv-wm.mutex
+*/
struct ilk_wm_values hw;
+
+ /*
+* protects some dev_priv-wm and intel_crtc-wm
+* state as well as the actual hardware registers
+*/
+ struct mutex mutex;
} wm;
struct i915_package_c8 pc8;
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index f022a78..8e32d69 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -384,7 +384,10 @@ struct intel_crtc {
/* per-pipe watermark state */
struct {
- /* watermarks currently being used */
+ /*
+* watermarks currently being used
+* protected by dev_priv-wm.mutex
+*/
struct intel_pipe_wm active;
} wm;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3f5c1dc..d8adcb3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2692,8 +2692,13 @@ static void ilk_write_wm_values(struct
drm_i915_private *dev_priv,
static bool ilk_disable_lp_wm(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
+ bool changed;
- return _ilk_disable_lp_wm(dev_priv, WM_DIRTY_LP_ALL);
+ mutex_lock(dev_priv-wm.mutex);
+ changed = _ilk_disable_lp_wm(dev_priv, WM_DIRTY_LP_ALL);
+ mutex_unlock(dev_priv-wm.mutex);
+
+ return changed;
}
static void ilk_program_watermarks(struct drm_device *dev)
@@ -2733,6 +2738,7 @@ static void ilk_update_wm(struct drm_crtc *crtc)
{
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_device *dev = crtc-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
struct ilk_pipe_wm_parameters params = {};
struct intel_pipe_wm pipe_wm = {};
@@ -2740,12 +2746,17 @@ static void ilk_update_wm(struct drm_crtc *crtc)
intel_compute_pipe_wm(crtc, params, pipe_wm);
+ mutex_lock(dev_priv-wm.mutex);
+
if (!memcmp(intel_crtc-wm.active, pipe_wm, sizeof(pipe_wm)))
- return;
+ goto unlock;
intel_crtc-wm.active = pipe_wm;
ilk_program_watermarks(dev);
+
+ unlock:
+ mutex_unlock(dev_priv-wm.mutex);
}
static void ilk_update_sprite_wm(struct drm_plane *plane,
@@ -2817,10 +2828,14 @@ void ilk_wm_get_hw_state(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_crtc *crtc;
+ mutex_lock(dev_priv-wm.mutex);
+
_ilk_wm_get_hw_state(dev, dev_priv-wm.hw);
list_for_each_entry(crtc, dev-mode_config.crtc_list, head)
_ilk_pipe_wm_hw_to_sw(crtc);
+
+ mutex_unlock(dev_priv-wm.mutex);
}
/**
@@ -5760,6 +5775,8 @@ void intel_init_pm(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
+ mutex_init(dev_priv-wm.mutex);
+
I think you should probably init this at intel_pm_setup(), which is
called way earlier. Just to be safe.
With that fixed: Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com.
if (HAS_FBC(dev)) {
if (INTEL_INFO(dev)-gen = 7) {
dev_priv-display.fbc_enabled = ironlake_fbc_enabled;
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] 3.15-rc2: i915 regression: only top 20% of screen works in X
On Wed 2014-04-23 23:09:45, Daniel Vetter wrote: On Wed, Apr 23, 2014 at 10:22 PM, Pavel Machek pa...@ucw.cz wrote: After update to 3.15-rc2, only top 20% of screen works on X. 00:02.0 VGA compatible controller: Intel Corporation 4 Series Chipset Integrated Graphics Controller (rev 03) 00:02.1 Display controller: Intel Corporation 4 Series Chipset Integrated Graphics Controller (rev 03) Subsystem: Intel Corporation Device d614 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast TAbort- TAbort- MAbort- SERR- PERR- INTx- Latency: 0 Region 0: Memory at d040 (64-bit, non-prefetchable) [size=1M] Capabilities: access denied This worked before. I believe it worked in 3.14. It definitely works in 3.11-rc2. Screenshot or more detailed description of what only top 20% of screen works in X means? Well, top cca 20% is ok, then I got repeated pattern of some part of screen. Anything in dmesg? That will take a look. bisect result presuming that it reproduces reliably? If there's no other chance, I guess I could do bisect. But first Do you have similar hardware? Does it work for you? Are there any experimental changes that went in recently and I should try reverting first? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] 3.15-rc2: i915 regression: only top 20% of screen works in X
Hi! After update to 3.15-rc2, only top 20% of screen works on X. 00:02.0 VGA compatible controller: Intel Corporation 4 Series Chipset Integrated Graphics Controller (rev 03) 00:02.1 Display controller: Intel Corporation 4 Series Chipset Integrated Graphics Controller (rev 03) Subsystem: Intel Corporation Device d614 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast TAbort- TAbort- MAbort- SERR- PERR- INTx- Latency: 0 Region 0: Memory at d040 (64-bit, non-prefetchable) [size=1M] Capabilities: access denied This worked before. I believe it worked in 3.14. It definitely works in 3.11-rc2. Screenshot or more detailed description of what only top 20% of screen works in X means? Anything in dmesg? Actually yes, dmesg suggests it is quite sick. drivers/gpu/drm/drm_mm.c:767 warning triggered repeatedly. Also.. initial framebuffer does not work ; I don't seem to see anything before X start up. (This is Debian 6.0.9) Best regards, Pavel Initializing cgroup subsys cpu Linux version 3.15.0-rc1+ (pavel@amd) (gcc version 4.4.5 (Debian 4.4.5-8) ) #335 SMP Sat Apr 19 17:58:01 CEST 2014 e820: BIOS-provided physical RAM map: BIOS-e820: [mem 0x-0x0009ebff] usable BIOS-e820: [mem 0x0009ec00-0x0009] reserved BIOS-e820: [mem 0x000e-0x000f] reserved BIOS-e820: [mem 0x0010-0xbd87dfff] usable BIOS-e820: [mem 0xbd87e000-0xbd900fff] ACPI NVS BIOS-e820: [mem 0xbd901000-0xbda42fff] reserved BIOS-e820: [mem 0xbda43000-0xbda56fff] ACPI NVS BIOS-e820: [mem 0xbda57000-0xbdb54fff] reserved BIOS-e820: [mem 0xbdb55000-0xbdb5dfff] ACPI data BIOS-e820: [mem 0xbdb5e000-0xbdb67fff] ACPI NVS BIOS-e820: [mem 0xbdb68000-0xbdb88fff] reserved BIOS-e820: [mem 0xbdb89000-0xbdb8efff] ACPI NVS BIOS-e820: [mem 0xbdb8f000-0xbdcf] usable BIOS-e820: [mem 0xfec0-0xfec00fff] reserved BIOS-e820: [mem 0xfed0-0xfed00fff] reserved BIOS-e820: [mem 0xfed1c000-0xfed8] reserved BIOS-e820: [mem 0xfff0-0x] reserved BIOS-e820: [mem 0x0001-0x00013fff] usable Notice: NX (Execute Disable) protection cannot be enabled: non-PAE kernel! SMBIOS 2.4 present. DMI: /DG41MJ, BIOS MJG4110H.86A.0006.2009.1223.1155 12/23/2009 e820: update [mem 0x-0x0fff] usable == reserved e820: remove [mem 0x000a-0x000f] usable e820: last_pfn = 0xbdd00 max_arch_pfn = 0x10 MTRR default type: uncachable MTRR fixed ranges enabled: 0-9 write-back A-E7FFF uncachable E8000-F write-protect MTRR variable ranges enabled: 0 base 0 mask F write-back 1 base 1 mask FC000 write-back 2 base 0BDD0 mask 0 write-through 3 base 0BDE0 mask FFFE0 uncachable 4 base 0BE00 mask FFE00 uncachable 5 base 0C000 mask FC000 uncachable 6 disabled x86 PAT enabled: cpu 0, old 0x7040600070406, new 0x7010600070106 initial memory mapped: [mem 0x-0x053f] Base memory trampoline at [c009a000] 9a000 size 16384 init_memory_mapping: [mem 0x-0x000f] [mem 0x-0x000f] page 4k init_memory_mapping: [mem 0x3700-0x373f] [mem 0x3700-0x373f] page 4k BRK [0x05109000, 0x05109fff] PGTABLE init_memory_mapping: [mem 0x3000-0x36ff] [mem 0x3000-0x36ff] page 4k BRK [0x0510a000, 0x0510afff] PGTABLE BRK [0x0510b000, 0x0510bfff] PGTABLE BRK [0x0510c000, 0x0510cfff] PGTABLE BRK [0x0510d000, 0x0510dfff] PGTABLE BRK [0x0510e000, 0x0510efff] PGTABLE init_memory_mapping: [mem 0x0010-0x2fff] [mem 0x0010-0x2fff] page 4k init_memory_mapping: [mem 0x3740-0x377fdfff] [mem 0x3740-0x377fdfff] page 4k ACPI: RSDP 0x000F03C0 24 (v02 INTEL ) ACPI: XSDT 0xBDB5CE18 44 (v01 INTEL DG41MJ 0006 MSFT 00010013) ACPI: FACP 0xBDB5BD98 F4 (v04 INTEL DG41RQ 0006 MSFT 00010013) ACPI BIOS Warning (bug): 32/64X FACS address mismatch in FADT: 0xBDB5FF40/0xBDB64F40, using 64-bit address (20140214/tbfadt-271) ACPI: DSDT 0xBDB55018 005983 (v01 INTEL DG41MJ 0006 INTL 20051117) ACPI: FACS 0xBDB64F40 40 ACPI: APIC 0xBDB5BF18 6C (v02 INTEL DG41MJ 0006 MSFT 00010013) ACPI: MCFG 0xBDB66E18 3C (v01 INTEL DG41MJ 0006 MSFT 0097) ACPI: HPET 0xBDB66D98 38 (v01 INTEL DG41MJ 0006 AMI. 0003) ACPI: Local APIC address 0xfee0 2149MB HIGHMEM available. 887MB LOWMEM available. mapped low ram: 0 - 377fe000 low ram: 0 - 377fe000 Zone
[Intel-gfx] [PATCH I-g-t V4 0/2] Tests: Add test cases based on multi drm_fd to test sync
This follows Daniel's advice to add the two test cases based on multi drm_fd to test the ring sync and CPU-GPU sync. The Broadwell GT3 machine has two independent BSD rings that can be used to process the video commands. This is implemented in kernel driver and transparent to the user-space. But we still need to check the ring sync and CPU-GPU sync for the second BSD ring. Two tests are created based on the multi drm_fds to test the sync. Multi drm_fd can assure that the second BSD ring has the opportunity to dispatch the GPU command. V1-V2: Follow Daniel's comment to add one subtext instead of one individual test case, which is used to test the CPU-GPU sync under multi BSD rings/ V2-V3: Follow Imre's comment to remove the unnecessary initialization and use igt_assert_f instead of igt_assert Zhao Yakui (2): tests: Add one ring sync case based on multi drm_fd to test ring semaphore sync under multi BSD rings tests/gem_dummy_reloc_loop: Add one subtest based on multi drm_fd to test CPU-GPU sync under multi BSD rings tests/.gitignore|1 + tests/Makefile.sources |1 + tests/gem_dummy_reloc_loop.c| 107 +++- tests/gem_multi_bsd_sync_loop.c | 175 +++ 4 files changed, 283 insertions(+), 1 deletion(-) create mode 100644 tests/gem_multi_bsd_sync_loop.c -- 1.7.10.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH I-g-t 2/2] tests/gem_dummy_reloc_loop: Add one subtest based on multi drm_fd to test CPU-GPU sync under multi BSD rings
The Broadwell GT3 machine has two independent BSD rings in kernel driver while
it is transparent to the user-space driver. In such case it needs to check
the CPU-GPU sync for the second BSD ring.
V1-V2: Follow Daniel's comment to add one subtext instead of one individual
test case, which is used to test the CPU-GPU sync under multi BSD rings.
V2-V3: Follow Imre's comment to remove the unnecessary initialization and
use igt_assert_f instead of igt_assert
Reviewed-by: Imre Deak imre.d...@intel.com
Signed-off-by: Zhao Yakui yakui.z...@intel.com
---
tests/gem_dummy_reloc_loop.c | 107 +-
1 file changed, 106 insertions(+), 1 deletion(-)
diff --git a/tests/gem_dummy_reloc_loop.c b/tests/gem_dummy_reloc_loop.c
index a61b59b..4e4dd49 100644
--- a/tests/gem_dummy_reloc_loop.c
+++ b/tests/gem_dummy_reloc_loop.c
@@ -48,6 +48,13 @@ static drm_intel_bufmgr *bufmgr;
struct intel_batchbuffer *batch;
static drm_intel_bo *target_buffer;
+#define NUM_FD 50
+
+static int mfd[NUM_FD];
+static drm_intel_bufmgr *mbufmgr[NUM_FD];
+static struct intel_batchbuffer *mbatch[NUM_FD];
+static drm_intel_bo *mbuffer[NUM_FD];
+
/*
* Testcase: Basic check of ring-cpu sync using a dummy reloc
*
@@ -124,6 +131,50 @@ dummy_reloc_loop_random_ring(int num_rings)
}
}
+static void
+dummy_reloc_loop_random_ring_multi_fd(int num_rings)
+{
+ int i;
+ struct intel_batchbuffer *saved_batch;
+
+ saved_batch = batch;
+
+ srandom(0xdeadbeef);
+
+ for (i = 0; i 0x10; i++) {
+ int mindex;
+ int ring = random() % num_rings + 1;
+
+ mindex = random() % NUM_FD;
+ batch = mbatch[mindex];
+
+ if (ring == I915_EXEC_RENDER) {
+ BEGIN_BATCH(4);
+ OUT_BATCH(MI_COND_BATCH_BUFFER_END | MI_DO_COMPARE);
+ OUT_BATCH(0x); /* compare dword */
+ OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
+ I915_GEM_DOMAIN_RENDER, 0);
+ OUT_BATCH(MI_NOOP);
+ ADVANCE_BATCH();
+ } else {
+ BEGIN_BATCH(4);
+ OUT_BATCH(MI_FLUSH_DW | 1);
+ OUT_BATCH(0); /* reserved */
+ OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
+ I915_GEM_DOMAIN_RENDER, 0);
+ OUT_BATCH(MI_NOOP | (122) | (0xf));
+ ADVANCE_BATCH();
+ }
+ intel_batchbuffer_flush_on_ring(batch, ring);
+
+ drm_intel_bo_map(target_buffer, 0);
+ // map to force waiting on rendering
+ drm_intel_bo_unmap(target_buffer);
+ }
+
+ batch = saved_batch;
+}
+
int fd;
int devid;
int num_rings;
@@ -133,6 +184,7 @@ igt_main
igt_skip_on_simulation();
igt_fixture {
+ int i;
fd = drm_open_any();
devid = intel_get_drm_devid(fd);
num_rings = gem_get_num_rings(fd);
@@ -148,6 +200,40 @@ igt_main
target_buffer = drm_intel_bo_alloc(bufmgr, target bo, 4096,
4096);
igt_assert(target_buffer);
+
+ /* Create multi drm_fd and map one gem object to multi
gem_contexts */
+ {
+ unsigned int target_flink;
+ char buffer_name[32];
+ if (dri_bo_flink(target_buffer, target_flink)) {
+ printf(fail to get flink for target buffer\n);
+ igt_assert_f(0, fail to create global
+gem_handle for target buffer\n);
+ }
+ for (i = 0; i NUM_FD; i++) {
+ sprintf(buffer_name, Target buffer %d\n, i);
+ mfd[i] = drm_open_any();
+ mbufmgr[i] = drm_intel_bufmgr_gem_init(mfd[i],
4096);
+ igt_assert_f(mbufmgr[i],
+fail to initialize buf manager
+for drm_fd %d\n,
+mfd[i]);
+ drm_intel_bufmgr_gem_enable_reuse(mbufmgr[i]);
+ mbatch[i] = intel_batchbuffer_alloc(mbufmgr[i],
devid);
+ igt_assert_f(mbatch[i],
+fail to create batchbuffer
+for drm_fd %d\n,
+mfd[i]);
+ mbuffer[i] = intel_bo_gem_create_from_name(
+ mbufmgr[i],
+
Re: [Intel-gfx] [PATCH I-g-t V4 0/2] Tests: Add test cases based on multi drm_fd to test sync
On Wed, 2014-04-23 at 20:02 -0600, Zhao, Yakui wrote: It seems that the patch 01 is filter out. So I will try to resend it again. Thanks. Yakui This follows Daniel's advice to add the two test cases based on multi drm_fd to test the ring sync and CPU-GPU sync. The Broadwell GT3 machine has two independent BSD rings that can be used to process the video commands. This is implemented in kernel driver and transparent to the user-space. But we still need to check the ring sync and CPU-GPU sync for the second BSD ring. Two tests are created based on the multi drm_fds to test the sync. Multi drm_fd can assure that the second BSD ring has the opportunity to dispatch the GPU command. V1-V2: Follow Daniel's comment to add one subtext instead of one individual test case, which is used to test the CPU-GPU sync under multi BSD rings/ V2-V3: Follow Imre's comment to remove the unnecessary initialization and use igt_assert_f instead of igt_assert Zhao Yakui (2): tests: Add one ring sync case based on multi drm_fd to test ring semaphore sync under multi BSD rings tests/gem_dummy_reloc_loop: Add one subtest based on multi drm_fd to test CPU-GPU sync under multi BSD rings tests/.gitignore|1 + tests/Makefile.sources |1 + tests/gem_dummy_reloc_loop.c| 107 +++- tests/gem_multi_bsd_sync_loop.c | 175 +++ 4 files changed, 283 insertions(+), 1 deletion(-) create mode 100644 tests/gem_multi_bsd_sync_loop.c ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH I-g-t V4 0/2] Tests: Add test cases based on multi drm_fd to test sync
This follows Daniel's advice to add the two test cases based on multi drm_fd to test the ring sync and CPU-GPU sync. The Broadwell GT3 machine has two independent BSD rings that can be used to process the video commands. This is implemented in kernel driver and transparent to the user-space. But we still need to check the ring sync and CPU-GPU sync for the second BSD ring. Two tests are created based on the multi drm_fds to test the sync. Multi drm_fd can assure that the second BSD ring has the opportunity to dispatch the GPU command. V1-V2: Follow Daniel's comment to add one subtext instead of one individual test case, which is used to test the CPU-GPU sync under multi BSD rings/ V2-V3: Follow Imre's comment to remove the unnecessary initialization and use igt_assert_f instead of igt_assert V3-V4: Add gem_multi_bsd_sync_loop.c into the tests/.gitignore Zhao Yakui (2): tests: Add one ring sync case based on multi drm_fd to test ring semaphore sync under multi BSD rings tests/gem_dummy_reloc_loop: Add one subtest based on multi drm_fd to test CPU-GPU sync under multi BSD rings tests/.gitignore|1 + tests/Makefile.sources |1 + tests/gem_dummy_reloc_loop.c| 107 +++- tests/gem_multi_bsd_sync_loop.c | 175 +++ 4 files changed, 283 insertions(+), 1 deletion(-) create mode 100644 tests/gem_multi_bsd_sync_loop.c -- 1.7.10.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH I-g-t V4 1/2] tests: Add one ring sync case based on multi drm_fd to test ring semaphore sync under multi BSD rings
The Broadwell GT3 machine has two independent BSD rings in kernel driver while
it is transparent to the user-space driver. In such case it needs to check
the ring sync between the two BSD rings. At the same time it also needs to
check the sync among the second BSD ring and the other rings.
V2-V3: Follow Imre's comment to remove the unnecessary initialization and
use igt_assert_f instead of igt_assert.
V3-V4: Add gem_multi_bsd_sync_loop.c into the tests/.gitignore
Reviewed-by: Imre Deak imre.d...@intel.com
Signed-off-by: Zhao Yakui yakui.z...@intel.com
---
tests/.gitignore|1 +
tests/Makefile.sources |1 +
tests/gem_multi_bsd_sync_loop.c | 175 +++
3 files changed, 177 insertions(+)
create mode 100644 tests/gem_multi_bsd_sync_loop.c
diff --git a/tests/.gitignore b/tests/.gitignore
index 146bab0..42690dd 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -54,6 +54,7 @@ gem_media_fill
gem_mmap
gem_mmap_gtt
gem_mmap_offset_exhaustion
+gem_multi_bsd_sync_loop
gem_non_secure_batch
gem_partial_pwrite_pread
gem_persistent_relocs
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c957ace..7cd9ca8 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -105,6 +105,7 @@ TESTS_progs = \
gem_render_tiled_blits \
gem_ring_sync_copy \
gem_ring_sync_loop \
+ gem_multi_bsd_sync_loop \
gem_seqno_wrap \
gem_set_tiling_vs_gtt \
gem_set_tiling_vs_pwrite \
diff --git a/tests/gem_multi_bsd_sync_loop.c b/tests/gem_multi_bsd_sync_loop.c
new file mode 100644
index 000..b01764a
--- /dev/null
+++ b/tests/gem_multi_bsd_sync_loop.c
@@ -0,0 +1,175 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Daniel Vetter daniel.vet...@ffwll.ch (based on gem_ring_sync_loop_*.c)
+ *Zhao Yakui yakui.z...@intel.com
+ *
+ */
+
+#include stdlib.h
+#include stdio.h
+#include string.h
+#include fcntl.h
+#include inttypes.h
+#include errno.h
+#include sys/stat.h
+#include sys/time.h
+#include drm.h
+#include ioctl_wrappers.h
+#include drmtest.h
+#include intel_bufmgr.h
+#include intel_batchbuffer.h
+#include intel_io.h
+#include i830_reg.h
+#include intel_chipset.h
+
+static drm_intel_bufmgr *bufmgr;
+struct intel_batchbuffer *batch;
+static drm_intel_bo *target_buffer;
+
+#define NUM_FD 50
+
+static int mfd[NUM_FD];
+static drm_intel_bufmgr *mbufmgr[NUM_FD];
+static struct intel_batchbuffer *mbatch[NUM_FD];
+static drm_intel_bo *mbuffer[NUM_FD];
+
+
+/*
+ * Testcase: Basic check of ring-ring sync using a dummy reloc
+ *
+ * Extremely efficient at catching missed irqs with semaphores=0 ...
+ */
+
+#define MI_COND_BATCH_BUFFER_END (0x3623 | 1)
+#define MI_DO_COMPARE (121)
+
+static void
+store_dword_loop(int fd)
+{
+ int i;
+ int num_rings = gem_get_num_rings(fd);
+
+ srandom(0xdeadbeef);
+
+ for (i = 0; i SLOW_QUICK(0x10, 10); i++) {
+ int ring, mindex;
+ ring = random() % num_rings + 1;
+ mindex = random() % NUM_FD;
+ batch = mbatch[mindex];
+ if (ring == I915_EXEC_RENDER) {
+ BEGIN_BATCH(4);
+ OUT_BATCH(MI_COND_BATCH_BUFFER_END | MI_DO_COMPARE);
+ OUT_BATCH(0x); /* compare dword */
+ OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
+ I915_GEM_DOMAIN_RENDER, 0);
+ OUT_BATCH(MI_NOOP);
+ ADVANCE_BATCH();
+ } else {
+ BEGIN_BATCH(4);
+ OUT_BATCH(MI_FLUSH_DW | 1);
+ OUT_BATCH(0); /* reserved */
+ OUT_RELOC(mbuffer[mindex], I915_GEM_DOMAIN_RENDER,
+
Re: [Intel-gfx] 3.15-rc2: i915 regression: only top 20% of screen works in X
On Thu, Apr 24, 2014 at 12:09:52AM +0200, Pavel Machek wrote: Hi! After update to 3.15-rc2, only top 20% of screen works on X. 00:02.0 VGA compatible controller: Intel Corporation 4 Series Chipset Integrated Graphics Controller (rev 03) 00:02.1 Display controller: Intel Corporation 4 Series Chipset Integrated Graphics Controller (rev 03) Subsystem: Intel Corporation Device d614 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast TAbort- TAbort- MAbort- SERR- PERR- INTx- Latency: 0 Region 0: Memory at d040 (64-bit, non-prefetchable) [size=1M] Capabilities: access denied This worked before. I believe it worked in 3.14. It definitely works in 3.11-rc2. Screenshot or more detailed description of what only top 20% of screen works in X means? Anything in dmesg? Actually yes, dmesg suggests it is quite sick. drivers/gpu/drm/drm_mm.c:767 warning triggered repeatedly. Also.. initial framebuffer does not work ; I don't seem to see anything before X start up. (This is Debian 6.0.9) That says that i915.ko failed to initialise the GPU (or rather the GPU wasn't responding) and bailed during module load. The key line here is [drm:init_ring_common] *ERROR* render ring initialization failed ctl 0001f001 head 2034 tail start 0012f000 Jiri has been seeing a similar issue creep in during resume, but it is not reliable enough to bisect. Is your boot failure reliable enough to bisect? Also drm-intel-nightly should mitigate this failure and allow i915.ko to continue to load and run X, which would be worth testing to make sure that works as intended. -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] mm: Throttle shrinkers harder
On Wed, Apr 23, 2014 at 02:14:36PM -0700, Dave Hansen wrote: On 04/22/2014 12:30 PM, Daniel Vetter wrote: During testing of i915.ko with working texture sets larger than RAM, we encounter OOM with plenty of memory still trapped within writeback, e.g: [ 42.386039] active_anon:10134 inactive_anon:1900781 isolated_anon:32 active_file:33 inactive_file:39 isolated_file:0 unevictable:0 dirty:0 writeback:337627 unstable:0 free:11985 slab_reclaimable:9458 slab_unreclaimable:23614 mapped:41 shmem:1560769 pagetables:1276 bounce:0 If we throttle for writeback following shrink_slab, this gives us time to wait upon the writeback generated by the i915.ko shinker: [ 4756.750808] active_anon:24386 inactive_anon:900793 isolated_anon:0 active_file:23 inactive_file:20 isolated_file:0 unevictable:0 dirty:0 writeback:0 unstable:0 free:5550 slab_reclaimable:5184 slab_unreclaimable:4888 mapped:3 shmem:472393 pagetables:1249 bounce:0 Could you get some dumps of the entire set of OOM information? These are only tiny snippets. For reference the last oom report after flushing all the writeback: [ 4756.749554] crond invoked oom-killer: gfp_mask=0x201da, order=0, oom_score_adj=0 [ 4756.749603] crond cpuset=/ mems_allowed=0 [ 4756.749628] CPU: 0 PID: 3574 Comm: crond Tainted: GW 3.14.0_prts_de579f_20140410 #2 [ 4756.749676] Hardware name: Gigabyte Technology Co., Ltd. H55M-UD2H/H55M-UD2H, BIOS F4 12/02/2009 [ 4756.749723] 000201da 81717273 8800d235dc40 [ 4756.749762] 81714541 0400 8800cb6f3b10 880117ff8000 [ 4756.749800] 81072266 0206 812d6ebe 880112f25c40 [ 4756.749838] Call Trace: [ 4756.749856] [81717273] ? dump_stack+0x41/0x51 [ 4756.749881] [81714541] ? dump_header.isra.8+0x69/0x191 [ 4756.749911] [81072266] ? ktime_get_ts+0x49/0xab [ 4756.749938] [812d6ebe] ? ___ratelimit+0xae/0xc8 [ 4756.749965] [810a72a8] ? oom_kill_process+0x76/0x32c [ 4756.749992] [810a706d] ? find_lock_task_mm+0x22/0x6e [ 4756.750018] [810a7add] ? out_of_memory+0x41c/0x44f [ 4756.750045] [810ab31d] ? __alloc_pages_nodemask+0x680/0x78d [ 4756.750076] [810d4b7f] ? alloc_pages_current+0xbf/0xdc [ 4756.750103] [810a61f8] ? filemap_fault+0x266/0x38b [ 4756.750130] [810bc3f5] ? __do_fault+0xac/0x3bf [ 4756.750155] [810bfb85] ? handle_mm_fault+0x1e7/0x7e2 [ 4756.750181] [810bc960] ? tlb_flush_mmu+0x4b/0x64 [ 4756.750219] [812d8ed5] ? timerqueue_add+0x79/0x98 [ 4756.750254] [8104d283] ? enqueue_hrtimer+0x15/0x37 [ 4756.750287] [8171f63d] ? __do_page_fault+0x42e/0x47b [ 4756.750319] [8104d580] ? hrtimer_try_to_cancel+0x67/0x70 [ 4756.750353] [8104d595] ? hrtimer_cancel+0xc/0x16 [ 4756.750385] [81719807] ? do_nanosleep+0xb3/0xf1 [ 4756.750415] [8104def9] ? hrtimer_nanosleep+0x89/0x10b [ 4756.750447] [8171cbf2] ? page_fault+0x22/0x30 [ 4756.750476] Mem-Info: [ 4756.750490] Node 0 DMA per-cpu: [ 4756.750510] CPU0: hi:0, btch: 1 usd: 0 [ 4756.750533] CPU1: hi:0, btch: 1 usd: 0 [ 4756.750555] CPU2: hi:0, btch: 1 usd: 0 [ 4756.750576] CPU3: hi:0, btch: 1 usd: 0 [ 4756.750598] Node 0 DMA32 per-cpu: [ 4756.750615] CPU0: hi: 186, btch: 31 usd: 0 [ 4756.750637] CPU1: hi: 186, btch: 31 usd: 0 [ 4756.750660] CPU2: hi: 186, btch: 31 usd: 0 [ 4756.750681] CPU3: hi: 186, btch: 31 usd: 0 [ 4756.750702] Node 0 Normal per-cpu: [ 4756.750720] CPU0: hi: 90, btch: 15 usd: 0 [ 4756.750742] CPU1: hi: 90, btch: 15 usd: 0 [ 4756.750763] CPU2: hi: 90, btch: 15 usd: 0 [ 4756.750785] CPU3: hi: 90, btch: 15 usd: 0 [ 4756.750808] active_anon:24386 inactive_anon:900793 isolated_anon:0 active_file:23 inactive_file:20 isolated_file:0 unevictable:0 dirty:0 writeback:0 unstable:0 free:5550 slab_reclaimable:5184 slab_unreclaimable:4888 mapped:3 shmem:472393 pagetables:1249 bounce:0 free_cma:0 [ 4756.750938] Node 0 DMA free:14664kB min:32kB low:40kB high:48kB active_anon:0kB inactive_anon:1024kB active_file:0kB inactive_file:4kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:15992kB managed:15908kB mlocked:0kB dirty:0kB writeback:0kB mapped:0kB shmem:412kB slab_reclaimable:80kB slab_unreclaimable:24kB kernel_stack:0kB pagetables:48kB unstable:0kB bounce:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:76 all_unreclaimable? yes [ 4756.751103] lowmem_reserve[]: 0 3337 3660 3660 [ 4756.751133] Node 0 DMA32 free:7208kB min:7044kB low:8804kB high:10564kB active_anon:36172kB inactive_anon:3351408kB active_file:92kB inactive_file:72kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:3518336kB managed:3440548kB mlocked:0kB dirty:0kB writeback:0kB mapped:12kB
