[Intel-gfx] [PATCH v2] drm/i915/opregion: ignore firmware requests for backlight change
Some Thinkpad laptops' firmware will initiate a backlight level change
request through operation region on the events of AC plug/unplug, but
since we are not using firmware's interface to do the backlight setting
on these affected laptops, we do not want the firmware to use some
arbitrary value from its ASL variable to set the backlight level on
AC plug/unplug either.
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=76491
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=77091
Reported-and-tested-by: Igor Gnatenko i.gnatenko.br...@gmail.com
Reported-and-tested-by: Anton Gubarkov anton.gubar...@gmail.com
Signed-off-by: Aaron Lu aaron...@intel.com
Acked-by: Jani Nikula jani.nik...@intel.com
---
v2: add a debug message when ignoring opregion request as suggested by
Jani Nikula.
drivers/acpi/video.c | 3 ++-
drivers/gpu/drm/i915/intel_opregion.c | 9 +
include/acpi/video.h | 2 ++
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index fb9ffe9adc64..cf99d6d2d491 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -241,13 +241,14 @@ static bool acpi_video_use_native_backlight(void)
return use_native_backlight_dmi;
}
-static bool acpi_video_verify_backlight_support(void)
+bool acpi_video_verify_backlight_support(void)
{
if (acpi_osi_is_win8() acpi_video_use_native_backlight()
backlight_device_registered(BACKLIGHT_RAW))
return false;
return acpi_video_backlight_support();
}
+EXPORT_SYMBOL(acpi_video_verify_backlight_support);
/* backlight device sysfs support */
static int acpi_video_get_brightness(struct backlight_device *bd)
diff --git a/drivers/gpu/drm/i915/intel_opregion.c
b/drivers/gpu/drm/i915/intel_opregion.c
index 2e2c71fcc9ed..4f6b53998d79 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -403,6 +403,15 @@ static u32 asle_set_backlight(struct drm_device *dev, u32
bclp)
DRM_DEBUG_DRIVER(bclp = 0x%08x\n, bclp);
+ /*
+* If the acpi_video interface is not supposed to be used, don't
+* bother processing backlight level change requests from firmware.
+*/
+ if (!acpi_video_verify_backlight_support()) {
+ DRM_DEBUG_KMS(opregion backlight request ignored\n);
+ return 0;
+ }
+
if (!(bclp ASLE_BCLP_VALID))
return ASLC_BACKLIGHT_FAILED;
diff --git a/include/acpi/video.h b/include/acpi/video.h
index ea4c7bbded4d..92f8c4bffefb 100644
--- a/include/acpi/video.h
+++ b/include/acpi/video.h
@@ -22,6 +22,7 @@ extern void acpi_video_unregister(void);
extern void acpi_video_unregister_backlight(void);
extern int acpi_video_get_edid(struct acpi_device *device, int type,
int device_id, void **edid);
+extern bool acpi_video_verify_backlight_support(void);
#else
static inline int acpi_video_register(void) { return 0; }
static inline void acpi_video_unregister(void) { return; }
@@ -31,6 +32,7 @@ static inline int acpi_video_get_edid(struct acpi_device
*device, int type,
{
return -ENODEV;
}
+static bool acpi_video_verify_backlight_support() { return false; }
#endif
#endif
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Avoid struct_mutex recursion when unmapping dma-buf
A request to unmap the dma-buf may result from freeing the imported
GEM object in e.g. radeon - an action where the struct_mutex is already
held. In this circumstance, we cannot take the mutex again and so must
defer our updating of the object state until we can take the mutex (i.e.
defer it to our workqueue).
[ 331.792605] =
[ 331.792782] [ INFO: possible recursive locking detected ]
[ 331.792971] 3.16.0-0.rc3.git3.1.fc21.x86_64 #1 Tainted: GW
[ 331.793012] -
[ 331.793012] Xorg.bin/1015 is trying to acquire lock:
[ 331.793012] (dev-struct_mutex){+.+.+.}, at: [a010a919]
i915_gem_unmap_dma_buf+0x39/0x110 [i915]
[ 331.793012]
but task is already holding lock:
[ 331.793012] (dev-struct_mutex){+.+.+.}, at: [a0084902]
drm_gem_object_handle_unreference_unlocked+0x102/0x130 [drm]
[ 331.793012]
other info that might help us debug this:
[ 331.793012] Possible unsafe locking scenario:
[ 331.793012]CPU0
[ 331.793012]
[ 331.793012] lock(dev-struct_mutex);
[ 331.793012] lock(dev-struct_mutex);
[ 331.793012]
*** DEADLOCK ***
[ 331.793012] May be due to missing lock nesting notation
[ 331.793012] 1 lock held by Xorg.bin/1015:
[ 331.793012] #0: (dev-struct_mutex){+.+.+.}, at: [a0084902]
drm_gem_object_handle_unreference_unlocked+0x102/0x130 [drm]
[ 331.793012]
stack backtrace:
[ 331.793012] CPU: 0 PID: 1015 Comm: Xorg.bin Tainted: GW
3.16.0-0.rc3.git3.1.fc21.x86_64 #1
[ 331.793012] Hardware name: LENOVO 4058CTO/4058CTO, BIOS 6FET93WW (3.23 )
10/12/2012
[ 331.793012] ad742009 88024ff4ba80
81807cec
[ 331.793012] 82bc8240 88024ff4bb60 81100fd0
81024369
[ 331.793012] 88024ff4bac0 810e1b3d 8800
007583ac
[ 331.793012] Call Trace:
[ 331.793012] [81807cec] dump_stack+0x4d/0x66
[ 331.793012] [81100fd0] __lock_acquire+0x1450/0x1ca0
[ 331.793012] [81024369] ? sched_clock+0x9/0x10
[ 331.793012] [810e1b3d] ? sched_clock_local+0x1d/0x90
[ 331.793012] [81024369] ? sched_clock+0x9/0x10
[ 331.793012] [810e1b3d] ? sched_clock_local+0x1d/0x90
[ 331.793012] [81102104] lock_acquire+0xa4/0x1d0
[ 331.793012] [a010a919] ? i915_gem_unmap_dma_buf+0x39/0x110 [i915]
[ 331.793012] [8180ccd5] mutex_lock_nested+0x85/0x440
[ 331.793012] [a010a919] ? i915_gem_unmap_dma_buf+0x39/0x110 [i915]
[ 331.793012] [a010a919] ? i915_gem_unmap_dma_buf+0x39/0x110 [i915]
[ 331.793012] [a010a919] i915_gem_unmap_dma_buf+0x39/0x110 [i915]
[ 331.793012] [81532591] dma_buf_unmap_attachment+0x51/0x80
[ 331.793012] [a009c6c2] drm_prime_gem_destroy+0x22/0x40 [drm]
[ 331.793012] [a0468112] radeon_gem_object_free+0x42/0x70 [radeon]
[ 331.793012] [a0084387] drm_gem_object_free+0x27/0x40 [drm]
[ 331.793012] [a0084920]
drm_gem_object_handle_unreference_unlocked+0x120/0x130 [drm]
[ 331.793012] [a00849ff] drm_gem_handle_delete+0xcf/0x1a0 [drm]
[ 331.793012] [a0085205] drm_gem_close_ioctl+0x25/0x30 [drm]
[ 331.793012] [a0082cdf] drm_ioctl+0x1df/0x6a0 [drm]
[ 331.793012] [81810af6] ? _raw_spin_unlock_irqrestore+0x36/0x70
[ 331.793012] [810ff72d] ? trace_hardirqs_on_caller+0x15d/0x200
[ 331.793012] [810ff7dd] ? trace_hardirqs_on+0xd/0x10
[ 331.793012] [a043604c] radeon_drm_ioctl+0x4c/0x80 [radeon]
[ 331.793012] [812628d0] do_vfs_ioctl+0x2f0/0x520
[ 331.793012] [8126ef6a] ? __fget+0x12a/0x2f0
[ 331.793012] [8126ee45] ? __fget+0x5/0x2f0
[ 331.793012] [8126f1a0] ? __fget_light+0x30/0x160
[ 331.793012] [81262b81] SyS_ioctl+0x81/0xa0
[ 331.793012] [818118e9] system_call_fastpath+0x16/0x1b
Reported-by: Shawn Starr shawn.st...@rogers.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80984
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 36 ++
1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 580aa42..7d8a87e 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -89,21 +89,49 @@ err:
return ERR_PTR(ret);
}
+struct i915_gem_unmap_dma_buf_work {
+ struct work_struct base;
+ struct drm_i915_gem_object *obj;
+};
+
+static void __i915_gem_unmap_dma_buf_worker(struct work_struct *base)
+{
+ struct i915_gem_unmap_dma_buf_work *work =
+ container_of(base, typeof(*work), base);
+ struct drm_device *dev = work-obj-base.dev;
+
+ mutex_lock(dev-struct_mutex);
+ i915_gem_object_unpin_pages(work-obj);
+
Re: [Intel-gfx] [PULL] drm-intel-next
On Tue, Jul 1, 2014 at 10:24 AM, Jani Nikula jani.nik...@intel.com wrote: As discussed, it contains acpi-next as a dependency for Jesse's S0ix work through these merges (should not conflict, fingers crossed): Aside: This is acpi-next for 3.16, so _really_ shouldn't conflict. It's only in there since at the time I've merged Jesse's soix patches I didn't yet roll drm-intel-next forward to post-rc1 - I tend to only do that once drm-next has rolled forward first. -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] [RFC] drm/i915: Add variable gem object size support to i915
On Wed, Jun 25, 2014 at 12:46:57PM +0100, Siluvery, Arun wrote: On 25/06/2014 12:14, Damien Lespiau wrote: On Wed, Jun 25, 2014 at 11:51:33AM +0100, Damien Lespiau wrote: (This is not necessarily things one would need to take into account for this work, just a few thoughts). One thing I'm wondering is how fitting the size parameter really is when talking about inherently 2D buffers. For instance, let's take a Y-tiled texture with MIPLAYOUT_RIGHT, if we want to allocate mip map levels 0 and 1, and use the ioctl naively to reserve the LOD1 region in one go, we'll end up over allocating the space below LOD1 (if I'm not mistaken that is). This can be mitigated by several calls to this fallocate ioctl, to reserve columns of pages (in the case above, columns for the LOD1 region). So, how about trying to reduce this ioctl overhead by providing a list of (start, length) in the ioctl structure? One more thing to factor in is (let's assume one future hardware will support that): https://www.opengl.org/registry/specs/ARB/sparse_texture.txt So maybe what we really want is to be able to specify region of pages that could be specified in (x, y, width, height, stride) ? (idea popped when talking to Neil Roberts (I now have someone working on Mesa in the office). Hi Damien, Thank you for your comments and the idea to improve this ioctl. At the moment start, end of a region are expected to be page-aligned; ioctl can be modified to accept a multiple ranges and modify them in one go to reduce the overhead of the ioctl. We can define how we want to specify multiple ranges, if userspace can provide the list as (start, end) pairs kernel can directly use them but what would be the preferred way from the user point of view? This smells badly like premature optimization. For y-tiled a page-row is 32 lines which means even for giant mipmap levels we'll deal with just a few ioctls to establish/remove a mipmap level. If you can benchmark this in a real-world tests (or even a microbenchmark that includes a little bit of texture upload) I'll be impressed. So a linear start/end, page-aligned, sounds more than good enough to me for now. -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] 3.15-rc: regression in suspend
On Fri, Jun 27, 2014 at 03:37:16PM +0200, Jiri Kosina wrote: On Thu, 26 Jun 2014, Pavel Machek wrote: Ok, so I have set up machines for ktest / autobisect, and found out that 3.16-rc1 no longer has that problem. Oh well, bisect would not be fun, anyway... I am still seeing the problem with 3.16-rc2. I'm confused now. Is the bisect result commit 773875bfb6737982903c42d1ee88cf60af80089c Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Mon Jan 27 10:00:30 2014 +0100 drm/i915: Don't set the 8to6 dither flag when not scaling now the culprit or not? Or do we have 2 different bugs at hand here? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/2] drm/i915: Set M2_N2 registers during mode set
On Tue, Jul 01, 2014 at 10:39:52AM +0530, Vandana Kannan wrote:
On Jun-18-2014 9:22 PM, Daniel Vetter wrote:
On Wed, Jun 18, 2014 at 07:47:24PM +0530, Vandana Kannan wrote:
For Gen 8, set M2_N2 registers on every mode set. This is required to
make
sure M2_N2 registers are set during boot, resume from sleep for cross-
checking the state. The register is set only if DRRS is supported.
v2: Patch rebased
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/gpu/drm/i915/intel_display.c | 36
drivers/gpu/drm/i915/intel_dp.c | 14 --
drivers/gpu/drm/i915/intel_drv.h | 1 +
4 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 0640071..6bf6e00 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1989,6 +1989,9 @@ struct drm_i915_cmd_table {
#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev))
#define HAS_RUNTIME_PM(dev) (IS_GEN6(dev) || IS_HASWELL(dev) || \
IS_BROADWELL(dev) || IS_VALLEYVIEW(dev))
+#define HAS_DRRS(dev) (to_i915(dev)-drrs.connector \
+ to_i915(dev)-drrs.connector- \
+ panel.downclock_mode)
Didn't spot this the first time around, but HAS_* macros should be chip
invariants, so no runtime-dependent pointer chasing and similar things
here. Which also means you can't use this in the pipe config checks.
Solution for that is to set pipe_config-has_drrs bool when you set the
2nd set of dp m/n values in the pipe_config in intel_dp_compute config.
My apologies that this takes so long and that the documentation for our
pipe-config infrastructure is so bad (= doesn't exist). I'm slowly working
towards the goal of document all the different subsystems in our driver.
Thanks, Daniel
Hi Daniel,
I have addressed the review comments and resent the patches..
http://lists.freedesktop.org/archives/intel-gfx/2014-June/047862.html
and
http://lists.freedesktop.org/archives/intel-gfx/2014-June/047863.html
Please help review the patches..
Can you please start a new thread with both patches? They're splattered a
bit badly over the m-l ...
Thanks, Daniel
Thanks,
Vandana
#define INTEL_PCH_DEVICE_ID_MASK 0xff00
#define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5e8e711..fca5e02 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3987,8 +3987,12 @@ static void ironlake_crtc_enable(struct drm_crtc
*crtc)
if (intel_crtc-config.has_pch_encoder)
intel_prepare_shared_dpll(intel_crtc);
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 HAS_DRRS(dev))
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -4097,8 +4101,12 @@ static void haswell_crtc_enable(struct drm_crtc
*crtc)
if (intel_crtc-active)
return;
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 HAS_DRRS(dev))
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -4614,8 +4622,12 @@ static void valleyview_crtc_enable(struct drm_crtc
*crtc)
/* Set up the display plane register */
dspcntr = DISPPLANE_GAMMA_ENABLE;
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 HAS_DRRS(dev))
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -4706,8 +4718,12 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
else
dspcntr |= DISPPLANE_SEL_PIPE_B;
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 HAS_DRRS(dev))
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -5494,6 +5510,18 @@ static void intel_dp_set_m_n(struct intel_crtc
*crtc)
Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot
On Thu, Jul 03, 2014 at 11:44:34PM +0300, Jani Nikula wrote:
On Thu, 03 Jul 2014, Clint Taylor clinton.a.tay...@intel.com wrote:
On 07/02/2014 01:35 AM, Jani Nikula wrote:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.
Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
Ver3: moved SYS_RESTART check earlier, new name for pp_div.
Ver4: Minor issue changes
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
[Jani: rebased on current -nightly.]
Signed-off-by: Jani Nikula jani.nik...@intel.com
---
I ended up doing the rebase myself, but I'd like to have a quick review
before pushing.
Quick review complete. Everything appears OK.
See Paulo's review; want to take over?
Imo this is -fixes material, but it looks like it's not yet complete. I'll
wait for a resend safe when someone yells at me that the current patch is
ok-ish (maybe just needs a pimped commit message or a comment somewhere to
address Paulo's concerns).
-Daniel
Jani.
Thanks,
Jani.
---
drivers/gpu/drm/i915/intel_dp.c | 40
drivers/gpu/drm/i915/intel_drv.h | 2 ++
2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index b5ec48913b47..f0d23c435cf6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -336,6 +338,36 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to guarantee T12
timing */
+static int edp_notify_handler(struct notifier_block *this, unsigned long
code,
+void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+ edp_notifier);
+ struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp_div;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if (!is_edp(intel_dp) || code != SYS_RESTART)
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ pp_div = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
+ pp_div = PP_REFERENCE_DIVIDER_MASK;
+
+ /* 0x1F write to PP_DIV_REG sets max cycle delay */
+ I915_WRITE(pp_div_reg, pp_div | 0x1F);
+ I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+ msleep(intel_dp-panel_power_cycle_delay);
+ }
+
+ return 0;
+}
+
static bool edp_have_panel_power(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3785,6 +3817,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
drm_modeset_lock(dev-mode_config.connection_mutex, NULL);
edp_panel_vdd_off_sync(intel_dp);
drm_modeset_unlock(dev-mode_config.connection_mutex);
+ if (intel_dp-edp_notifier.notifier_call) {
+ unregister_reboot_notifier(intel_dp-edp_notifier);
+ intel_dp-edp_notifier.notifier_call = NULL;
+ }
}
kfree(intel_dig_port);
}
@@ -4353,6 +4389,10 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
if (is_edp(intel_dp)) {
intel_dp_init_panel_power_timestamps(intel_dp);
intel_dp_init_panel_power_sequencer(dev, intel_dp, power_seq);
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-edp_notifier.notifier_call =
edp_notify_handler;
+ register_reboot_notifier(intel_dp-edp_notifier);
+ }
}
intel_dp_aux_init(intel_dp, intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 5f7c7bd94d90..87d1715db21d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -541,6 +541,8 @@ struct intel_dp {
unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+ struct notifier_block edp_notifier;
+
bool use_tps3;
struct intel_connector *attached_connector;
--
Jani Nikula, Intel Open Source Technology Center
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Kick out vga console
On Mon, Jun 30, 2014 at 07:59:55AM +0100, Chris Wilson wrote:
On Sat, Jun 28, 2014 at 11:55:19PM -0400, Ed Tomlinson wrote:
On Saturday 28 June 2014 15:28:22 Ed Tomlinson wrote:
Resend without html krud which causes list to bounce the message.
Hi
This commit ( a4de05268e674e8ed31df6348269e22d6c6a1803 ) hangs my boot
with 3.16-git. Reverting it lets the boot proceed.
I have an i7 with a built-in i915 and an pcie r7 260x. The R7 is the
primary console. The i915 is initialized
but does not have a physical display attached.
With the patch applied the boot stops at the messages:
[drm] Memory usable by graphics device = 2048M
[drm] Replacing VGA console driver
The issue looks like that we are ripping out the radeon fb_con whilst it
is active and that upsets everyone. In which case, I think the
compromise is:
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5f44581..4915f1d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1439,18 +1439,20 @@ static int i915_kick_out_vgacon(struct
drm_i915_private *dev_priv)
#else
static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
{
- int ret;
+ int ret = 0;
DRM_INFO(Replacing VGA console driver\n);
console_lock();
- ret = do_take_over_console(dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
- if (ret == 0) {
- ret = do_unregister_con_driver(vga_con);
-
- /* Ignore already unregistered. */
- if (ret == -ENODEV)
- ret = 0;
+ if (con_is_bound(vga_con)) {
+ ret = do_take_over_console(dummy_con, 0, MAX_NR_CONSOLES -
1, 1);
+ if (ret == 0) {
+ ret = do_unregister_con_driver(vga_con);
Hm, we should only conditionalize the take_over_console - unregistering
vga_con is kinda the point to make sure it's gone for real. Ed, can you
please retest with the if (con_is_bound) check just for the
do_take_over_console call?
Still puzzled wtf is going on here since as David says this should be a
no-op.
Thanks, Daniel
+
+ /* Ignore already unregistered. */
+ if (ret == -ENODEV)
+ ret = 0;
+ }
}
console_unlock();
-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
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 5/6] drm/i915: Don't clobber the GTT when it's within stolen memory
On Mon, Jun 30, 2014 at 01:25:29PM +0300, Jani Nikula wrote:
On Thu, 05 Jun 2014, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
On most gen2-4 platforms the GTT can be (or maybe always is?)
inside the stolen memory region. If that's the case, reduce the
size of the stolen memory appropriately to make make sure we
don't clobber the GTT.
v2: Deal with gen4 36 bit physical address
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80151
Picked up for -fixes (with Chris' irc-ack and cc: stable), thanks for the
patch.
-Daniel
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_gem_stolen.c | 44
++
drivers/gpu/drm/i915/i915_reg.h| 3 +++
2 files changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 62ef55b..7465ab0 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -74,6 +74,50 @@ static unsigned long i915_stolen_to_physical(struct
drm_device *dev)
if (base == 0)
return 0;
+ /* make sure we don't clobber the GTT if it's within stolen memory */
+ if (INTEL_INFO(dev)-gen = 4 !IS_G33(dev) !IS_G4X(dev)) {
+ struct {
+ u32 start, end;
+ } stolen[2] = {
+ { .start = base, .end = base +
dev_priv-gtt.stolen_size, },
+ { .start = base, .end = base +
dev_priv-gtt.stolen_size, },
+ };
+ u64 gtt_start, gtt_end;
+
+ gtt_start = I915_READ(PGTBL_CTL);
+ if (IS_GEN4(dev))
+ gtt_start = (gtt_start PGTBL_ADDRESS_LO_MASK) |
+ (gtt_start PGTBL_ADDRESS_HI_MASK) 28;
+ else
+ gtt_start = PGTBL_ADDRESS_LO_MASK;
+ gtt_end = gtt_start + gtt_total_entries(dev_priv-gtt) * 4;
+
+ if (gtt_start = stolen[0].start gtt_start stolen[0].end)
+ stolen[0].end = gtt_start;
+ if (gtt_end stolen[1].start gtt_end = stolen[1].end)
+ stolen[1].start = gtt_end;
+
+ /* pick the larger of the two chunks */
+ if (stolen[0].end - stolen[0].start
+ stolen[1].end - stolen[1].start) {
+ base = stolen[0].start;
+ dev_priv-gtt.stolen_size = stolen[0].end -
stolen[0].start;
+ } else {
+ base = stolen[1].start;
+ dev_priv-gtt.stolen_size = stolen[1].end -
stolen[1].start;
+ }
+
+ if (stolen[0].start != stolen[1].start ||
+ stolen[0].end != stolen[1].end) {
+ DRM_DEBUG_KMS(GTT within stolen memory at
0x%llx-0x%llx\n,
+ (unsigned long long) gtt_start,
+ (unsigned long long) gtt_end - 1);
+ DRM_DEBUG_KMS(Stolen memory adjusted to 0x%x-0x%x\n,
+ base, base + (u32)
dev_priv-gtt.stolen_size - 1);
+ }
+ }
+
+
/* Verify that nothing else uses this physical address. Stolen
* memory should be reserved by the BIOS and hidden from the
* kernel. So if the region is already marked as busy, something
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index 247be2a..619924b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -978,6 +978,9 @@ enum punit_power_well {
/*
* Instruction and interrupt control regs
*/
+#define PGTBL_CTL 0x02020
+#define PGTBL_ADDRESS_LO_MASK0xf000 /* bits [31:12] */
+#define PGTBL_ADDRESS_HI_MASK0x00f0 /* bits [35:32] (gen4) */
#define PGTBL_ER 0x02024
#define RENDER_RING_BASE 0x02000
#define BSD_RING_BASE 0x04000
--
1.8.5.5
___
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
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/2] drm/i915: Set M2_N2 registers during mode set
On Jul-07-2014 2:11 PM, Daniel Vetter wrote:
On Tue, Jul 01, 2014 at 10:39:52AM +0530, Vandana Kannan wrote:
On Jun-18-2014 9:22 PM, Daniel Vetter wrote:
On Wed, Jun 18, 2014 at 07:47:24PM +0530, Vandana Kannan wrote:
For Gen 8, set M2_N2 registers on every mode set. This is required to
make
sure M2_N2 registers are set during boot, resume from sleep for cross-
checking the state. The register is set only if DRRS is supported.
v2: Patch rebased
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/gpu/drm/i915/intel_display.c | 36
drivers/gpu/drm/i915/intel_dp.c | 14 --
drivers/gpu/drm/i915/intel_drv.h | 1 +
4 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 0640071..6bf6e00 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1989,6 +1989,9 @@ struct drm_i915_cmd_table {
#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev))
#define HAS_RUNTIME_PM(dev) (IS_GEN6(dev) || IS_HASWELL(dev) || \
IS_BROADWELL(dev) || IS_VALLEYVIEW(dev))
+#define HAS_DRRS(dev) (to_i915(dev)-drrs.connector \
+ to_i915(dev)-drrs.connector- \
+ panel.downclock_mode)
Didn't spot this the first time around, but HAS_* macros should be chip
invariants, so no runtime-dependent pointer chasing and similar things
here. Which also means you can't use this in the pipe config checks.
Solution for that is to set pipe_config-has_drrs bool when you set the
2nd set of dp m/n values in the pipe_config in intel_dp_compute config.
My apologies that this takes so long and that the documentation for our
pipe-config infrastructure is so bad (= doesn't exist). I'm slowly working
towards the goal of document all the different subsystems in our driver.
Thanks, Daniel
Hi Daniel,
I have addressed the review comments and resent the patches..
http://lists.freedesktop.org/archives/intel-gfx/2014-June/047862.html
and
http://lists.freedesktop.org/archives/intel-gfx/2014-June/047863.html
Please help review the patches..
Can you please start a new thread with both patches? They're splattered a
bit badly over the m-l ...
Thanks, Daniel
Sure..
I will resend them now..
- Vandana
Thanks,
Vandana
#define INTEL_PCH_DEVICE_ID_MASK 0xff00
#define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5e8e711..fca5e02 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3987,8 +3987,12 @@ static void ironlake_crtc_enable(struct drm_crtc
*crtc)
if (intel_crtc-config.has_pch_encoder)
intel_prepare_shared_dpll(intel_crtc);
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 HAS_DRRS(dev))
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -4097,8 +4101,12 @@ static void haswell_crtc_enable(struct drm_crtc
*crtc)
if (intel_crtc-active)
return;
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 HAS_DRRS(dev))
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -4614,8 +4622,12 @@ static void valleyview_crtc_enable(struct drm_crtc
*crtc)
/* Set up the display plane register */
dspcntr = DISPPLANE_GAMMA_ENABLE;
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 HAS_DRRS(dev))
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -4706,8 +4718,12 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
else
dspcntr |= DISPPLANE_SEL_PIPE_B;
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 HAS_DRRS(dev))
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -5494,6 +5510,18 @@ static void intel_dp_set_m_n(struct intel_crtc
*crtc)
Re: [Intel-gfx] [PATCH 2/5] drm/i915: preserve swizzle settings if necessary v3
On Fri, Jun 27, 2014 at 09:15:25AM -0700, Steve Aarnio wrote: On 06/11/2014 08:41 AM, Jesse Barnes wrote: On Wed, 11 Jun 2014 17:39:29 +0200 Daniel Vetter dan...@ffwll.ch wrote: On Wed, Jun 11, 2014 at 5:13 PM, Jesse Barnes jbar...@virtuousgeek.org wrote: - If you have a machine which uses tiled framebuffers and enables swizzling in the BIOS your code will a) drop the swizzle setup in gem_init_hw, breaking resume b) not set the swizzle settings correctly in swizzle_detect, breaking swap in/out and pwrite/pread. Not sure such a machine exists, but still. This would affect krh's MBA, which is why I wanted testing here... anyway I'll spin a new one and ask krh to test again. Hm, I've thought the issue with the MBA is that it used tiled fbs, but non-swizzled. And then a mess ensued when we've enabled it. But yeah, unfortunately with the new logic we need to retest :( Ah yeah I think you're right, either way, need more testing. Maybe we should have just gone with the first patch to never enable swizzling based on Art's assertion that it didn't matter. I hate to jump into the middle of a conversation that may or may not be related to a patch I just posted... but... There was a very long internal discussion that the Windows guys had with H/W. For Gen8+ H/W recommends disabling CSX swizzle. Technically, BDW still supports it, but there is a bug _somewhere_ that makes it problematic. In any case it goes away for sure with Gen9+, so disabling on Gen8 doesn't hurt. According to the other discussion, the H/W guys say that enabling actually hurts performance slightly, and the driver should leave the swizzle decisions to the memory controller. Patch to disable swizzling detection on gen8+ in i915_gem_tiling.c (only there, imo ok to keep the hw paths around for setting up the registers) welcome ;-) -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 1/2] drm/i915: Set M2_N2 registers during mode set
For Gen 8, set M2_N2 registers on every mode set. This is required to make
sure M2_N2 registers are set during boot, resume from sleep for cross-
checking the state. The register is set only if DRRS is supported.
v2: Patch rebased
v3: Daniel's review comments
- Removed HAS_DRRS(dev) and added bool has_drrs to pipe_config to
track drrs support
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_display.c | 36
drivers/gpu/drm/i915/intel_dp.c | 16 ++--
drivers/gpu/drm/i915/intel_drv.h | 2 ++
3 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index a72b55f..22bdea5f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4020,8 +4020,12 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
if (intel_crtc-config.has_pch_encoder)
intel_prepare_shared_dpll(intel_crtc);
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 intel_crtc-config.has_drrs)
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -4130,8 +4134,12 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
if (intel_crtc-active)
return;
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 intel_crtc-config.has_drrs)
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -4648,8 +4656,12 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
/* Set up the display plane register */
dspcntr = DISPPLANE_GAMMA_ENABLE;
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 intel_crtc-config.has_drrs)
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -4738,8 +4750,12 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
else
dspcntr |= DISPPLANE_SEL_PIPE_B;
- if (intel_crtc-config.has_dp_encoder)
+ if (intel_crtc-config.has_dp_encoder) {
intel_dp_set_m_n(intel_crtc);
+ if (INTEL_INFO(dev)-gen 8 intel_crtc-config.has_drrs)
+ intel_dp_set_m2_n2(intel_crtc,
+ intel_crtc-config.dp_m2_n2);
+ }
intel_set_pipe_timings(intel_crtc);
@@ -5530,6 +5546,18 @@ static void intel_dp_set_m_n(struct intel_crtc *crtc)
intel_cpu_transcoder_set_m_n(crtc, crtc-config.dp_m_n);
}
+void intel_dp_set_m2_n2(struct intel_crtc *crtc, struct intel_link_m_n *m_n)
+{
+ struct drm_device *dev = crtc-base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ enum transcoder transcoder = crtc-config.cpu_transcoder;
+
+ I915_WRITE(PIPE_DATA_M2(transcoder), TU_SIZE(m_n-tu) | m_n-gmch_m);
+ I915_WRITE(PIPE_DATA_N2(transcoder), m_n-gmch_n);
+ I915_WRITE(PIPE_LINK_M2(transcoder), m_n-link_m);
+ I915_WRITE(PIPE_LINK_N2(transcoder), m_n-link_n);
+}
+
static void vlv_update_pll(struct intel_crtc *crtc)
{
u32 dpll, dpll_md;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b5ec489..1c3960b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -780,20 +780,6 @@ intel_dp_set_clock(struct intel_encoder *encoder,
}
}
-static void
-intel_dp_set_m2_n2(struct intel_crtc *crtc, struct intel_link_m_n *m_n)
-{
- struct drm_device *dev = crtc-base.dev;
- struct drm_i915_private *dev_priv = dev-dev_private;
- enum transcoder transcoder = crtc-config.cpu_transcoder;
-
- I915_WRITE(PIPE_DATA_M2(transcoder),
- TU_SIZE(m_n-tu) | m_n-gmch_m);
- I915_WRITE(PIPE_DATA_N2(transcoder), m_n-gmch_n);
- I915_WRITE(PIPE_LINK_M2(transcoder), m_n-link_m);
- I915_WRITE(PIPE_LINK_N2(transcoder), m_n-link_n);
-}
-
bool
intel_dp_compute_config(struct intel_encoder *encoder,
struct intel_crtc_config *pipe_config)
@@ -819,6 +805,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
pipe_config-has_pch_encoder = true;
pipe_config-has_dp_encoder = true;
+ pipe_config-has_drrs =
Re: [Intel-gfx] [PATCH 03/11] drm/i915: Move vlv cdclk code to .get_display_clock_speed()
On Wed, Jun 25, 2014 at 11:47:50AM -0700, Jesse Barnes wrote:
On Fri, 13 Jun 2014 13:37:49 +0300
ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
We have a standard hook for reading out the current cdclk. Move the VLV
code from valleyview_cur_cdclk() to .get_display_clock_speed().
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 33 +
drivers/gpu/drm/i915/intel_drv.h | 1 -
drivers/gpu/drm/i915/intel_pm.c | 2 +-
3 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 36562b5..61d7ea2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4449,7 +4449,7 @@ static void valleyview_set_cdclk(struct drm_device
*dev, int cdclk)
struct drm_i915_private *dev_priv = dev-dev_private;
u32 val, cmd;
- WARN_ON(valleyview_cur_cdclk(dev_priv) != dev_priv-vlv_cdclk_freq);
+ WARN_ON(dev_priv-display.get_display_clock_speed(dev) !=
dev_priv-vlv_cdclk_freq);
dev_priv-vlv_cdclk_freq = cdclk;
if (cdclk = 32) /* jump to highest voltage for 400MHz too */
@@ -4506,24 +4506,6 @@ static void valleyview_set_cdclk(struct drm_device
*dev, int cdclk)
intel_i2c_reset(dev);
}
-int valleyview_cur_cdclk(struct drm_i915_private *dev_priv)
-{
- int cur_cdclk, vco;
- int divider;
-
- vco = valleyview_get_vco(dev_priv);
-
- mutex_lock(dev_priv-dpio_lock);
- divider = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
- mutex_unlock(dev_priv-dpio_lock);
-
- divider = DISPLAY_FREQUENCY_VALUES;
-
- cur_cdclk = DIV_ROUND_CLOSEST(vco 1, divider + 1);
-
- return cur_cdclk;
-}
-
static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
int max_pixclk)
{
@@ -5228,7 +5210,18 @@ static int intel_crtc_compute_config(struct
intel_crtc *crtc,
static int valleyview_get_display_clock_speed(struct drm_device *dev)
{
- return 40; /* FIXME */
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ int vco = valleyview_get_vco(dev_priv);
+ u32 val;
+ int divider;
+
+ mutex_lock(dev_priv-dpio_lock);
+ val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
+ mutex_unlock(dev_priv-dpio_lock);
+
+ divider = val DISPLAY_FREQUENCY_VALUES;
+
+ return DIV_ROUND_CLOSEST(vco 1, divider + 1);
}
static int i945_get_display_clock_speed(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 78d4124..65ce0bb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -717,7 +717,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
const char *intel_output_name(int output);
bool intel_has_pending_fb_unpin(struct drm_device *dev);
int intel_pch_rawclk(struct drm_device *dev);
-int valleyview_cur_cdclk(struct drm_i915_private *dev_priv);
void intel_mark_busy(struct drm_device *dev);
void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
struct intel_engine_cs *ring);
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index a9ddf74..67f019c1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5535,7 +5535,7 @@ static void valleyview_init_clock_gating(struct
drm_device *dev)
}
DRM_DEBUG_DRIVER(DDR speed: %d MHz, dev_priv-mem_freq);
- dev_priv-vlv_cdclk_freq = valleyview_cur_cdclk(dev_priv);
+ dev_priv-vlv_cdclk_freq =
dev_priv-display.get_display_clock_speed(dev);
DRM_DEBUG_DRIVER(Current CD clock rate: %d kHz,
dev_priv-vlv_cdclk_freq);
Looks like we only really use that on gen4 but so it seems harmless.
We have the same (or at least very similar) can we dot this dotclock with
the given display clock, if not boost kind of logic so imo fits into the
gen3 heritage of the vlv display block ...
-Daniel
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: State readout and cross-checking for dp_m2_n2
Adding relevant read out comparison code, in check_crtc_state, for the new
member of crtc_config, dp_m2_n2, which was introduced to store link_m_n
values for a DP downclock mode (if available). Suggested by Daniel.
v2: Changed patch title.
Daniel's review comments incorporated.
Added relevant state readout code for M2_N2. dp_m2_n2 comparison to be done
only when high RR is not in use (This is because alternate m_n register
programming will be done only when low RR is being used).
v3: Modified call to get_m2_n2 which had dp_m_n as param by mistake.
Compare dp_m_n and dp_m2_n2 for gen 7 and below. compare the structures
based on DRRS state for gen 8 and above.
Save and restore M2 N2 registers for gen 7 and below
v4: For Gen=8, check M_N registers against dp_m_n and dp_m2_n2 as there is
only one set of M_N registers
v5: Removed the chunk which saves and restores M2_N2 registers. Modified
get_m_n() to get M2_N2 registers as well. Modified the macro which compares
hw.dp_m_n against sw.dp_m2_n2/sw.dp_m_n for gen 8.
v6: Added check to compare dp_m2_n2 only when DRRS is enabled
v7: Modified drrs check to use has_drrs
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Jani Nikula jani.nik...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 70 +++-
1 file changed, 62 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 22bdea5f..a76522b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7152,7 +7152,8 @@ static void intel_pch_transcoder_get_m_n(struct
intel_crtc *crtc,
static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc,
enum transcoder transcoder,
-struct intel_link_m_n *m_n)
+struct intel_link_m_n *m_n,
+struct intel_link_m_n *m2_n2)
{
struct drm_device *dev = crtc-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -7166,6 +7167,15 @@ static void intel_cpu_transcoder_get_m_n(struct
intel_crtc *crtc,
m_n-gmch_n = I915_READ(PIPE_DATA_N1(transcoder));
m_n-tu = ((I915_READ(PIPE_DATA_M1(transcoder))
TU_SIZE_MASK) TU_SIZE_SHIFT) + 1;
+ if (m2_n2 INTEL_INFO(dev)-gen 8) {
+ m2_n2-link_m = I915_READ(PIPE_LINK_M2(transcoder));
+ m2_n2-link_n = I915_READ(PIPE_LINK_N2(transcoder));
+ m2_n2-gmch_m = I915_READ(PIPE_DATA_M2(transcoder))
+~TU_SIZE_MASK;
+ m2_n2-gmch_n = I915_READ(PIPE_DATA_N2(transcoder));
+ m2_n2-tu = ((I915_READ(PIPE_DATA_M2(transcoder))
+TU_SIZE_MASK) TU_SIZE_SHIFT) + 1;
+ }
} else {
m_n-link_m = I915_READ(PIPE_LINK_M_G4X(pipe));
m_n-link_n = I915_READ(PIPE_LINK_N_G4X(pipe));
@@ -7184,14 +7194,15 @@ void intel_dp_get_m_n(struct intel_crtc *crtc,
intel_pch_transcoder_get_m_n(crtc, pipe_config-dp_m_n);
else
intel_cpu_transcoder_get_m_n(crtc, pipe_config-cpu_transcoder,
-pipe_config-dp_m_n);
+pipe_config-dp_m_n,
+pipe_config-dp_m2_n2);
}
static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc,
struct intel_crtc_config *pipe_config)
{
intel_cpu_transcoder_get_m_n(crtc, pipe_config-cpu_transcoder,
-pipe_config-fdi_m_n);
+pipe_config-fdi_m_n, NULL);
}
static void ironlake_get_pfit_config(struct intel_crtc *crtc,
@@ -9946,6 +9957,15 @@ static void intel_dump_pipe_config(struct intel_crtc
*crtc,
pipe_config-dp_m_n.gmch_m, pipe_config-dp_m_n.gmch_n,
pipe_config-dp_m_n.link_m, pipe_config-dp_m_n.link_n,
pipe_config-dp_m_n.tu);
+
+ DRM_DEBUG_KMS(dp: %i, gmch_m2: %u, gmch_n2: %u, link_m2: %u, link_n2:
%u, tu2: %u\n,
+ pipe_config-has_dp_encoder,
+ pipe_config-dp_m2_n2.gmch_m,
+ pipe_config-dp_m2_n2.gmch_n,
+ pipe_config-dp_m2_n2.link_m,
+ pipe_config-dp_m2_n2.link_n,
+ pipe_config-dp_m2_n2.tu);
+
DRM_DEBUG_KMS(requested mode:\n);
drm_mode_debug_printmodeline(pipe_config-requested_mode);
DRM_DEBUG_KMS(adjusted mode:\n);
@@ -10326,6 +10346,22 @@ intel_pipe_config_compare(struct drm_device *dev,
return false; \
}
+/* This is required for
Re: [Intel-gfx] [PATCH 07/11] drm/i915: Warn if there's a cdclk change in progess
On Wed, Jun 25, 2014 at 10:34:48PM +0300, Ville Syrjälä wrote: On Wed, Jun 25, 2014 at 11:55:58AM -0700, Jesse Barnes wrote: On Fri, 13 Jun 2014 13:37:53 +0300 ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com If someone is interested in the current cdclk frquency it should be stable and not in process of changing frquency. Warn if the current and requested cdclk don't match in .get_display_clock_spee() on vlv. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 29dddec..601e97e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5234,6 +5234,10 @@ static int valleyview_get_display_clock_speed(struct drm_device *dev) divider = val DISPLAY_FREQUENCY_VALUES; + WARN((val DISPLAY_FREQUENCY_STATUS) != + (divider DISPLAY_FREQUENCY_STATUS_SHIFT), + cdclk change in progress\n); + return DIV_ROUND_CLOSEST(vco 1, divider + 1); } Hm, there's not much we can do in this case, so rather than warn maybe we should try a wait instead, and only warn if it times out? Even then there's not much we can do aside from poking the PUnit folks. This shouldn't happen unless we somehow messed up and triggered a cdclk change and didn't wait for it to complete, which would be a driver bug. So I think a simple WARN seems sufficient. I concur with Ville here so merged the patch. If we hit this we can figure out where exactly we've been wrong and whether there's a legitimate case where we need to wait for cdclk to settle. -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] [WIP][PATCH 11/11] drm/i915: Turn off clocks when disp2d is powered down
On Wed, Jun 25, 2014 at 12:03:59PM -0700, Jesse Barnes wrote:
On Fri, 13 Jun 2014 13:37:57 +0300
ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Set some bits in CCK/CCU to turn off display clocks when disp2d is power
gated. Not sure this really helps with anything. Docs aren't all that clear.
XXX: Doesn't actually work. CCK_DISPLAY_REF_CLOCK_CONTROL and CCU_ICLK_5
writes don't have any effect on the registers for some reason. When clock
gating disp2d via Punit CCK_DISPLAY_REF_CLOCK_CONTROL trunk off force bit
gets set but again direct write has no effect.
---
drivers/gpu/drm/i915/i915_reg.h | 5 +
drivers/gpu/drm/i915/intel_pm.c | 35 +++
2 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index 2aa9a3c..be88b13 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -584,12 +584,17 @@ enum punit_power_well {
#define DSI_PLL_M1_DIV_SHIFT 0
#define DSI_PLL_M1_DIV_MASK (0x1ff 0)
#define CCK_DISPLAY_CLOCK_CONTROL 0x6b
+#define CCK_DISPLAY_REF_CLOCK_CONTROL 0x6c
#define DISPLAY_TRUNK_FORCE_ON(1 17)
#define DISPLAY_TRUNK_FORCE_OFF (1 16)
#define DISPLAY_FREQUENCY_STATUS (0x1f 8)
#define DISPLAY_FREQUENCY_STATUS_SHIFT8
#define DISPLAY_FREQUENCY_VALUES (0x1f 0)
+#define CCU_ICLK_5 0x114
+#define DISPSS_CLKREQ (1 1)
+#define DISPBEND_CLKREQ (1 0)
+
/**
* DOC: DPIO
*
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index d8e20d3..96614c3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6057,6 +6057,20 @@ static void vlv_display_power_well_enable(struct
drm_i915_private *dev_priv,
{
WARN_ON_ONCE(power_well-data != PUNIT_POWER_WELL_DISP2D);
+ mutex_lock(dev_priv-dpio_lock);
+ /*
+* (re)enable ref clocks at CCU
+* FIXME maybe move to cmnlane?
+*/
+ vlv_ccu_write(dev_priv, CCU_ICLK_5,
+ vlv_ccu_read(dev_priv, CCU_ICLK_5) |
+ DISPBEND_CLKREQ | DISPSS_CLKREQ);
+ /*
+* Punit clears CCK trunk force off bits
+* automagically while ungating disp2d
+*/
+ mutex_unlock(dev_priv-dpio_lock);
+
vlv_set_power_well(dev_priv, power_well, true);
spin_lock_irq(dev_priv-irq_lock);
@@ -6085,6 +6099,27 @@ static void vlv_display_power_well_disable(struct
drm_i915_private *dev_priv,
spin_unlock_irq(dev_priv-irq_lock);
vlv_set_power_well(dev_priv, power_well, false);
+
+ mutex_lock(dev_priv-dpio_lock);
+ /*
+* Punit doesn't set the CCK trunk force off bits when power gating
+* disp2d. It does set them when clock gating disp2d, but we ask it
+* to power gate instead of clock gate.
+*/
+ vlv_cck_write(dev_priv, CCK_DISPLAY_CLOCK_CONTROL,
+ vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL) |
+ DISPLAY_TRUNK_FORCE_OFF);
+ vlv_cck_write(dev_priv, CCK_DISPLAY_REF_CLOCK_CONTROL,
+ vlv_cck_read(dev_priv, CCK_DISPLAY_REF_CLOCK_CONTROL) |
+ DISPLAY_TRUNK_FORCE_OFF);
+ /*
+* disable ref clocks at CCU
+* FIXME maybe move to cmnlane?
+*/
+ vlv_ccu_write(dev_priv, CCU_ICLK_5,
+ vlv_ccu_read(dev_priv, CCU_ICLK_5)
+ ~(DISPBEND_CLKREQ | DISPSS_CLKREQ));
+ mutex_unlock(dev_priv-dpio_lock);
}
static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private
*dev_priv,
I guess you could ping the hw folks about this, maybe starting with
Cesar, to see if we can drop the power any further by doing this or
poking some other reg...
Pulled the entire series except this one here int dinq. Thanks for
patchesreview.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 3/3] drm/i915: gmch: fix stuck primary plane due to memory self-refresh mode
On Fri, Jun 27, 2014 at 09:38:52PM +0300, Imre Deak wrote: Hi Egbert, On Fri, 2014-06-27 at 15:55 +0200, Egbert Eich wrote: Chris Wilson writes: On Fri, Jun 27, 2014 at 12:07:47AM +0200, Egbert Eich wrote: Hi Daniel, hi Imre, Daniel Vetter writes: Adding Egbert since he's done the original hack here. Imre please keep him on cc. -Daniel I finally managed to get this set of patches tested on the platform that exhibited the intermittent blanking problem when terminating the Xserver. I can confirm that Imre's patches resolve the issue and that g4x_fixup_plane() which I had introduced after extensive experiments is no longer needed to prevent the blanking from happening. If you want I can provide a patch to back this out with the appropriate comments once Imre's patches are in. That would be ideal. Is there a chance that Imre's patches will go into the Intel repo any time soon? Then I could use the commit Id in the patch description. Yes, Deepak promised to review it early next week, so it could be applied after that. I guess we could add your Tested-by too. Back from my vacation and patches are merged. Egbert, please submit your patch so that I can merge it. Thanks for patches, reviewtesting. -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 v2 1/2] intel-gpu-tools: pass argc/argv to simple main
From: Tim Gore tim.g...@intel.com
Quite a few single tests do not use the igt_simple_main
macro because they want access to argc/argv. So change the
igt_simple_main macro to pass these arguments through to the
__real_mainxxx function, and change these tests to use
the macro.
Also rename the igt_simple_init to __igt_simple_init to
indicate that it should be accessed via the macros
Signed-off-by: Tim Gore tim.g...@intel.com
---
lib/igt_core.c | 4 ++--
lib/igt_core.h | 19 +++
tests/gem_ctx_basic.c| 6 +-
tests/gem_exec_blt.c | 5 +
tests/gem_gtt_speed.c| 7 ++-
tests/gem_hang.c | 5 +
tests/gem_render_copy.c | 4 +---
tests/gem_render_linear_blits.c | 9 +++--
tests/gem_render_tiled_blits.c | 7 ++-
tests/gem_seqno_wrap.c | 11 ---
tests/gem_stress.c | 5 +
tests/gen3_mixed_blits.c | 5 +
tests/gen3_render_linear_blits.c | 5 +
tests/gen3_render_mixed_blits.c | 5 +
tests/gen3_render_tiledx_blits.c | 5 +
tests/gen3_render_tiledy_blits.c | 5 +
tests/igt_simulation.c | 2 +-
17 files changed, 35 insertions(+), 74 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 7ac7ebe..b822bc9 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -450,7 +450,7 @@ void igt_subtest_init(int argc, char **argv)
}
/**
- * igt_simple_init:
+ * __igt_simple_init:
*
* This initializes a simple test without any support for subtests.
*
@@ -458,7 +458,7 @@ void igt_subtest_init(int argc, char **argv)
* #igt_simple_main block instead of stitching the tests's main() function
together
* manually.
*/
-void igt_simple_init(void)
+void __igt_simple_init(void)
{
print_version();
diff --git a/lib/igt_core.h b/lib/igt_core.h
index e252eba..75ee60c 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -166,23 +166,26 @@ bool igt_only_list_subtests(void);
*
* Init for simple tests without subtests
*/
-void igt_simple_init(void);
+void __igt_simple_init(void);
/**
* igt_simple_main:
*
* This is a magic control flow block used instead of a main() function for
- * simple tests. Open-coding the main() function is only recommended if
- * the test needs to parse additional cmdline arguments of its own.
+ * simple tests. It calls __igt_simple_init() to do basic setup
+ * and passes argc/v through to the __real_main function that
+ * follows the macro.
+ * Open-coding the main() function is only recommended if there is a
+ * requirement that cannot be met otherwise
*/
#define igt_simple_main \
- static void igt_tokencat(__real_main, __LINE__)(void); \
+ static void igt_tokencat(__real_main, __LINE__)(int argc, char **argv);
\
int main(int argc, char **argv) { \
- igt_simple_init(); \
- igt_tokencat(__real_main, __LINE__)(); \
- exit(0); \
+ __igt_simple_init(); \
+ igt_tokencat(__real_main, __LINE__)(argc, argv); \
+ igt_exit(); \
} \
- static void igt_tokencat(__real_main, __LINE__)(void) \
+ static void igt_tokencat(__real_main, __LINE__)(int argc, char **argv) \
__attribute__((format(printf, 1, 2)))
void igt_skip(const char *f, ...) __attribute__((noreturn));
diff --git a/tests/gem_ctx_basic.c b/tests/gem_ctx_basic.c
index 3e9b688..fe770ea 100644
--- a/tests/gem_ctx_basic.c
+++ b/tests/gem_ctx_basic.c
@@ -145,12 +145,10 @@ static void parse(int argc, char *argv[])
}
}
-int main(int argc, char *argv[])
+igt_simple_main
{
int i;
- igt_simple_init();
-
fd = drm_open_any_render();
devid = intel_get_drm_devid(fd);
@@ -173,6 +171,4 @@ int main(int argc, char *argv[])
free(threads);
close(fd);
-
- return 0;
}
diff --git a/tests/gem_exec_blt.c b/tests/gem_exec_blt.c
index 3bcef18..3d092fe 100644
--- a/tests/gem_exec_blt.c
+++ b/tests/gem_exec_blt.c
@@ -253,12 +253,10 @@ static void run(int object_size)
close(fd);
}
-int main(int argc, char **argv)
+igt_simple_main
{
int i;
- igt_simple_init();
-
igt_skip_on_simulation();
if (argc 1) {
@@ -270,5 +268,4 @@ int main(int argc, char **argv)
} else
run(OBJECT_SIZE);
- return 0;
}
diff --git a/tests/gem_gtt_speed.c b/tests/gem_gtt_speed.c
index 385eeb7..42c3694 100644
--- a/tests/gem_gtt_speed.c
+++ b/tests/gem_gtt_speed.c
@@ -50,7 +50,7 @@ static double elapsed(const struct timeval *start,
return (1e6*(end-tv_sec - start-tv_sec) + (end-tv_usec -
start-tv_usec))/loop;
}
-int main(int argc, char **argv)
+igt_simple_main
{
struct timeval start, end;
uint8_t *buf;
@@ -59,15 +59,13 @@ int main(int argc, char **argv)
int loop, i, tiling;
int fd;
- igt_simple_init();
-
igt_skip_on_simulation();
[Intel-gfx] [PATCH v2 2/2] intel-gpu-tools: Re-use igt_subtest_init for simple tests
From: Tim Gore tim.g...@intel.com
igt_subtest_init mainly does stuff that we also want for
simple/single tests, such as looking for --list-subtests
and --help options and calling common_init. So just call
this from igt_simple_init and then set tests_with_subtests
to false. NOTE that this means that check_igt_exit is now
registered as an exit handler for single tests, so need to
make sure that ALL tests exit via igt_exit.
Signed-off-by: Tim Gore tim.g...@intel.com
---
lib/igt_core.c | 37 +
lib/igt_core.h | 4 ++--
tests/gem_gtt_hog.c| 2 +-
tests/igt_simulation.c | 4 ++--
4 files changed, 26 insertions(+), 21 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index b822bc9..7871360 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -457,14 +457,21 @@ void igt_subtest_init(int argc, char **argv)
* If there's not a reason to the contrary it's less error prone to just use an
* #igt_simple_main block instead of stitching the tests's main() function
together
* manually.
+ * We now call through to the igt_subtest_init function since we want most of
its
+ * functionality. Note that this will install the igt_exit_handler so you need
to
+ * exit via igt_exit(), Dont call exit()
+ *
*/
-void __igt_simple_init(void)
+void __igt_simple_init(int argc, char **argv)
{
- print_version();
-
- oom_adjust_for_doom();
-
- common_init();
+/* use igt_subtest_init, it does mostly what we want */
+ igt_subtest_init(argc, argv);
+ /* but of course we dont have subtests - so mark this */
+ test_with_subtests = false;
+ if (list_subtests)
+ igt_exit();
+ if (run_single_subtest)
+ igt_fail(1);/* we dont have subtests ! */
}
/*
@@ -565,7 +572,7 @@ void igt_skip(const char *f, ...)
assert(in_fixture);
__igt_fixture_end();
} else {
- exit(IGT_EXIT_SKIP);
+ igt_exit();
}
}
@@ -655,7 +662,7 @@ void igt_fail(int exitcode)
__igt_fixture_end();
}
- exit(exitcode);
+ igt_exit();
}
}
@@ -713,18 +720,16 @@ void igt_exit(void)
if (igt_only_list_subtests())
exit(IGT_EXIT_SUCCESS);
- if (!test_with_subtests)
- exit(IGT_EXIT_SUCCESS);
-
- /* Calling this without calling one of the above is a failure */
- assert(skipped_one || succeeded_one || failed_one);
+ if (test_with_subtests)
+ /* Calling this without calling one of the above is a failure */
+ assert(skipped_one || succeeded_one || failed_one);
if (failed_one)
exit(igt_exitcode);
- else if (succeeded_one)
- exit(IGT_EXIT_SUCCESS);
- else
+ else if (skipped_one)
exit(IGT_EXIT_SKIP);
+ else
+ exit(IGT_EXIT_SUCCESS);
}
/* fork support code */
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 75ee60c..448beed 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -166,7 +166,7 @@ bool igt_only_list_subtests(void);
*
* Init for simple tests without subtests
*/
-void __igt_simple_init(void);
+void __igt_simple_init(int argc, char **argv);
/**
* igt_simple_main:
@@ -181,7 +181,7 @@ void __igt_simple_init(void);
#define igt_simple_main \
static void igt_tokencat(__real_main, __LINE__)(int argc, char **argv);
\
int main(int argc, char **argv) { \
- __igt_simple_init(); \
+ __igt_simple_init(argc, argv); \
igt_tokencat(__real_main, __LINE__)(argc, argv); \
igt_exit(); \
} \
diff --git a/tests/gem_gtt_hog.c b/tests/gem_gtt_hog.c
index 5d47540..f607ea0 100644
--- a/tests/gem_gtt_hog.c
+++ b/tests/gem_gtt_hog.c
@@ -150,7 +150,7 @@ static void run(data_t *data, int child)
munmap(ptr, size);
igt_assert(x == canary);
- exit(0);
+ _exit(0);
}
igt_simple_main
diff --git a/tests/igt_simulation.c b/tests/igt_simulation.c
index 6d8a59a..5d38026 100644
--- a/tests/igt_simulation.c
+++ b/tests/igt_simulation.c
@@ -53,11 +53,11 @@ static int do_fork(void)
assert(0);
case 0:
if (simple) {
- __igt_simple_init();
+ __igt_simple_init(1, argv_run);
igt_skip_on_simulation();
- exit(0);
+ igt_exit();
} else {
if (list_subtests)
igt_subtest_init(2, argv_list);
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 0/2] Single tests to respond to --list-subtests
From: Tim Gore tim.g...@intel.com A step towards towards removing the distinction between single and multiple tests. The first step is to change the igt_simple_main macro to pass argc/v through to the real_main function, so that several simple tests that want argc/v can still use this macro. Once this is done, all the simple tests can use this macro and we then modify this macro and the igt_simple_init function so that argc/v are available to igt_simple_init. Finally modify igt_simple_init to call the igt_subtest_init function to parse the cmdline args for --list-subtest and --help. There are some subtleties introduced by the fact that igt_subtest_init registers the check_igt_exit exit handler, so now single as well as multiple tests must always exit via igt_exit. v2: a) Moved some comments into the Doxygen function comments. b) Renamed the igt_simple_init function to __igt_simple_init to indicate that it should only be called from the framework macros. c) A few simple tests had return statements for error exits, so replaced these with igt_fail calls. This ensures that the igt_exit function is called as required by the framework. Tim Gore (2): intel-gpu-tools: pass argc/argv to simple main intel-gpu-tools: Re-use igt_subtest_init for simple tests lib/igt_core.c | 39 ++- lib/igt_core.h | 19 +++ tests/gem_ctx_basic.c| 6 +- tests/gem_exec_blt.c | 5 + tests/gem_gtt_hog.c | 2 +- tests/gem_gtt_speed.c| 7 ++- tests/gem_hang.c | 5 + tests/gem_render_copy.c | 4 +--- tests/gem_render_linear_blits.c | 9 +++-- tests/gem_render_tiled_blits.c | 7 ++- tests/gem_seqno_wrap.c | 11 --- tests/gem_stress.c | 5 + tests/gen3_mixed_blits.c | 5 + tests/gen3_render_linear_blits.c | 5 + tests/gen3_render_mixed_blits.c | 5 + tests/gen3_render_tiledx_blits.c | 5 + tests/gen3_render_tiledy_blits.c | 5 + tests/igt_simulation.c | 4 ++-- 18 files changed, 57 insertions(+), 91 deletions(-) -- 1.9.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Kick out vga console
Daniel,
I am not quite sure I understand what you want me to test?
Do you want me to try it without:
+ if (ret == 0) {
+ ret = do_unregister_con_driver(vga_con);
Thanks
Ed
On Monday 07 July 2014 10:48:26 Daniel Vetter wrote:
On Mon, Jun 30, 2014 at 07:59:55AM +0100, Chris Wilson wrote:
On Sat, Jun 28, 2014 at 11:55:19PM -0400, Ed Tomlinson wrote:
On Saturday 28 June 2014 15:28:22 Ed Tomlinson wrote:
Resend without html krud which causes list to bounce the message.
Hi
This commit ( a4de05268e674e8ed31df6348269e22d6c6a1803 ) hangs my boot
with 3.16-git. Reverting it lets the boot proceed.
I have an i7 with a built-in i915 and an pcie r7 260x. The R7 is the
primary console. The i915 is initialized
but does not have a physical display attached.
With the patch applied the boot stops at the messages:
[drm] Memory usable by graphics device = 2048M
[drm] Replacing VGA console driver
The issue looks like that we are ripping out the radeon fb_con whilst it
is active and that upsets everyone. In which case, I think the
compromise is:
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index 5f44581..4915f1d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1439,18 +1439,20 @@ static int i915_kick_out_vgacon(struct
drm_i915_private *dev_priv)
#else
static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
{
- int ret;
+ int ret = 0;
DRM_INFO(Replacing VGA console driver\n);
console_lock();
- ret = do_take_over_console(dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
- if (ret == 0) {
- ret = do_unregister_con_driver(vga_con);
-
- /* Ignore already unregistered. */
- if (ret == -ENODEV)
- ret = 0;
+ if (con_is_bound(vga_con)) {
+ ret = do_take_over_console(dummy_con, 0, MAX_NR_CONSOLES -
1, 1);
+ if (ret == 0) {
+ ret = do_unregister_con_driver(vga_con);
Hm, we should only conditionalize the take_over_console - unregistering
vga_con is kinda the point to make sure it's gone for real. Ed, can you
please retest with the if (con_is_bound) check just for the
do_take_over_console call?
Still puzzled wtf is going on here since as David says this should be a
no-op.
Thanks, Daniel
+
+ /* Ignore already unregistered. */
+ if (ret == -ENODEV)
+ ret = 0;
+ }
}
console_unlock();
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] scripts: add a script to help run tests with Piglit
Add a script to facilitate running the tests with Piglit by providing
simplified options for listing, filtering and creating summaries of test
runs.
Signed-off-by: Thomas Wood thomas.w...@intel.com
---
.gitignore | 3 ++
scripts/Makefile.am | 2 +-
scripts/run-tests.sh | 120 +++
3 files changed, 124 insertions(+), 1 deletion(-)
create mode 100755 scripts/run-tests.sh
diff --git a/.gitignore b/.gitignore
index 5b9ac8f..f40b4f6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -88,3 +88,6 @@ version.h
gtk-doc.make
gtk-doc.m4
+
+piglit
+results
diff --git a/scripts/Makefile.am b/scripts/Makefile.am
index baf3612..b77f0eb 100644
--- a/scripts/Makefile.am
+++ b/scripts/Makefile.am
@@ -1,3 +1,3 @@
-dist_noinst_SCRIPTS = who.sh
+dist_noinst_SCRIPTS = who.sh run-tests.sh
noinst_PYTHON = throttle.py
diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh
new file mode 100755
index 000..cdb6e08
--- /dev/null
+++ b/scripts/run-tests.sh
@@ -0,0 +1,120 @@
+#!/bin/bash
+#
+# 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.
+
+
+ROOT=`dirname $0`
+ROOT=`readlink -f $ROOT/..`
+IGT_TEST_ROOT=$ROOT/tests
+RESULTS=$ROOT/results
+PIGLIT=`which piglit 2 /dev/null`
+
+if [ ! -d $IGT_TEST_ROOT ]; then
+ echo Error: could not find tests directory.
+ exit 1
+fi
+
+if [ ! -f $IGT_TEST_ROOT/single-tests.txt ]; then
+ echo Error: test list not found.
+ echo Please run make in the tests directory to generate the test list.
+fi
+
+SINGLE_TEST_LIST=`cat $IGT_TEST_ROOT/single-tests.txt | sed -e '/TESTLIST/d'
-e 's/ /\n/g'`
+MULTI_TEST_LIST=`cat $IGT_TEST_ROOT/multi-tests.txt | sed -e '/TESTLIST/d' -e
's/ /\n/g'`
+
+function download_piglit {
+ git clone git://anongit.freedesktop.org/piglit $ROOT/piglit
+}
+
+function print_help {
+ echo Usage: run-tests.sh [options]
+ echo Available options:
+ echo -d download Piglit to $ROOT/piglit
+ echo -h display this help message
+ echo -l list all available tests
+ echo -r directory store the results in directory
+ echo (default: $RESULTS)
+ echo -s create html summary
+ echo -t regex only include tests that match the regular
expression
+ echo -v enable verbose mode
+ echo -x regex exclude tests that match the regular expression
+ echo
+ echo Useful patterns for test filtering are described in
tests/NAMING-CONVENTION
+}
+
+function list_tests {
+ echo $SINGLE_TEST_LIST
+ for test in $MULTI_TEST_LIST; do
+ SUBTESTS=`$IGT_TEST_ROOT/$test --list-subtests`
+ for subtest in $SUBTESTS; do
+ echo $test/$subtest
+ done
+ done
+}
+
+while getopts :dhlr:st:vx: opt; do
+ case $opt in
+ d) download_piglit; exit ;;
+ h) print_help; exit ;;
+ l) list_tests; exit ;;
+ r) RESULTS=$OPTARG ;;
+ s) SUMMARY=html ;;
+ t) FILTER=-t $OPTARG ;;
+ v) VERBOSE=-v ;;
+ x) EXCLUDE=-x $OPTARG ;;
+ :)
+ echo Option -$OPTARG requires an argument.
+ exit 1
+ ;;
+ \?)
+ echo Unknown option: -$OPTARG
+ print_help
+ exit 1
+ ;;
+ esac
+done
+shift $(($OPTIND-1))
+
+if [ x$1 != x ]; then
+ echo Unknown option: $1
+ print_help
+ exit 1
+fi
+
+if [ x$PIGLIT == x ]; then
+ PIGLIT=$ROOT/piglit/piglit
+fi
+
+if [ ! -x $PIGLIT ]; then
+ echo Could not find Piglit.
+ echo Please install Piglit or use -d to download Piglit locally.
+ exit 1
+fi
+
+mkdir -p
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Kick out vga console
Daniel,
Just to be sure. The intel card here should not be claiming the real console.
It does
not have an output device and the bios set set so the radeon is the primary
device.
Ed
On Monday 07 July 2014 10:48:26 Daniel Vetter wrote:
On Mon, Jun 30, 2014 at 07:59:55AM +0100, Chris Wilson wrote:
On Sat, Jun 28, 2014 at 11:55:19PM -0400, Ed Tomlinson wrote:
On Saturday 28 June 2014 15:28:22 Ed Tomlinson wrote:
Resend without html krud which causes list to bounce the message.
Hi
This commit ( a4de05268e674e8ed31df6348269e22d6c6a1803 ) hangs my boot
with 3.16-git. Reverting it lets the boot proceed.
I have an i7 with a built-in i915 and an pcie r7 260x. The R7 is the
primary console. The i915 is initialized
but does not have a physical display attached.
With the patch applied the boot stops at the messages:
[drm] Memory usable by graphics device = 2048M
[drm] Replacing VGA console driver
The issue looks like that we are ripping out the radeon fb_con whilst it
is active and that upsets everyone. In which case, I think the
compromise is:
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index 5f44581..4915f1d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1439,18 +1439,20 @@ static int i915_kick_out_vgacon(struct
drm_i915_private *dev_priv)
#else
static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
{
- int ret;
+ int ret = 0;
DRM_INFO(Replacing VGA console driver\n);
console_lock();
- ret = do_take_over_console(dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
- if (ret == 0) {
- ret = do_unregister_con_driver(vga_con);
-
- /* Ignore already unregistered. */
- if (ret == -ENODEV)
- ret = 0;
+ if (con_is_bound(vga_con)) {
+ ret = do_take_over_console(dummy_con, 0, MAX_NR_CONSOLES -
1, 1);
+ if (ret == 0) {
+ ret = do_unregister_con_driver(vga_con);
Hm, we should only conditionalize the take_over_console - unregistering
vga_con is kinda the point to make sure it's gone for real. Ed, can you
please retest with the if (con_is_bound) check just for the
do_take_over_console call?
Still puzzled wtf is going on here since as David says this should be a
no-op.
Thanks, Daniel
+
+ /* Ignore already unregistered. */
+ if (ret == -ENODEV)
+ ret = 0;
+ }
}
console_unlock();
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/bdw: 3D_CHICKEN3 has write mask bits
From: Michel Thierry michel.thie...@intel.com The workaround to limit SDE poly depth FIFO to 2 is not applied because 3D Chicken-3 mask bit is not set. WaLimitSizeOfSDEPolyFifo is only for BDW-A and could be removed. Signed-off-by: Michel Thierry michel.thie...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 31ae2b4..ae68df6 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5360,7 +5360,7 @@ static void gen8_init_clock_gating(struct drm_device *dev) I915_WRITE(GAMTARBMODE, _MASKED_BIT_ENABLE(ARB_MODE_BWGTLB_DISABLE)); I915_WRITE(_3D_CHICKEN3, - _3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(2)); + _MASKED_BIT_ENABLE(_3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(2))); I915_WRITE(COMMON_SLICE_CHICKEN2, _MASKED_BIT_ENABLE(GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE)); -- 1.9.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 41/53] drm/i915/bdw: Avoid non-lite-restore preemptions
On Mon, Jun 23, 2014 at 11:52:11AM +, Mateo Lozano, Oscar wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Wednesday, June 18, 2014 9:49 PM To: Mateo Lozano, Oscar Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 41/53] drm/i915/bdw: Avoid non-lite-restore preemptions On Fri, Jun 13, 2014 at 04:37:59PM +0100, oscar.ma...@intel.com wrote: From: Oscar Mateo oscar.ma...@intel.com In the current Execlists feeding mechanism, full preemption is not supported yet: only lite-restores are allowed (this is: the GPU simply samples a new tail pointer for the context currently in execution). But we have identified an scenario in which a full preemption occurs: 1) We submit two contexts for execution (A B). 2) The GPU finishes with the first one (A), switches to the second one (B) and informs us. 3) We submit B again (hoping to cause a lite restore) together with C, but in the time we spend writing to the ELSP, the GPU finishes B. 4) The GPU start executing B again (since we told it so). 5) We receive a B finished interrupt and, mistakenly, we submit C (again) and D, causing a full preemption of B. By keeping a better track of our submissions, we can avoid the scenario described above. How? I don't see a way to fundamentally avoid the above race, and I don't really see an issue with it - the gpu should notice that there's not really any work done and then switch to C. Or am I completely missing the point here? With no clue at all this looks really scary. The race is avoided by keeping track of how many times a context has been submitted to the hardware and by better discriminating the received context switch interrupts: in the example, when we have submitted B twice, we won´t submit C and D as soon as we receive the notification that B is completed because we were expecting to get a LITE_RESTORE and we didn´t, so we know a second completion will be received shortly. Without this explicit checking, the race condition happens and, somehow, the batch buffer execution order gets messed with. This can be verified with the IGT test I sent together with the series. I don´t know the exact mechanism by which the pre-emption messes with the execution order but, since other people is working on the Scheduler + Preemption on Execlists, I didn´t try to fix it. In these series, only Lite Restores are supported (other kind of preemptions WARN). I´ll add this clarification to the commit message. Yeah, please elaborate more clearly in the commit message what exactly seems to go wrong (and where you're unsure with your model of how the hardware reacts). And please also reference the precise testcase (and precise failure mode without this patch) so that if anyone stumbles over this again we have some breadcrumbs to figure things out. And some stern warnings as comments in the code to warn the unsuspecting reader. -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/2] drm/i915: rework digital port IRQ handling (v2)
On Tue, Jun 24, 2014 at 03:00:51PM -0700, Todd Previte wrote:
This looks like it's good to go.
As an aside, I don't *think* any of the compliance testing stuff I'm working
on cares whether it's short of long pulse (1.1a compliance), but it will be
interesting to see if/when/where it might have an effect.
Reviewed-by: Todd Previte tprev...@gmail.com
Queued for -next, thanks for the patch.
-Daniel
Dave Airlie mailto:airl...@gmail.com
Tuesday, June 17, 2014 6:29 PM
From: Dave Airlie airl...@redhat.com
The digital ports from Ironlake and up have the ability to distinguish
between long and short HPD pulses. Displayport 1.1 only uses the short
form to request link retraining usually, so we haven't really needed
support for it until now.
However with DP 1.2 MST we need to handle the short irqs on their
own outside the modesetting locking the long hpd's involve. This
patch adds the framework to distinguish between short/long to the
current code base, to lay the basis for future DP 1.2 MST work.
This should mean we get better bisectability in case of regression
due to the new irq handling.
v2: add GM45 support (untested, due to lack of hw)
Signed-off-by: Dave Airlie airl...@redhat.com
---
drivers/gpu/drm/i915/i915_drv.h | 5 ++
drivers/gpu/drm/i915/i915_irq.c | 160
+--
drivers/gpu/drm/i915/intel_ddi.c | 3 +
drivers/gpu/drm/i915/intel_dp.c | 20 +
drivers/gpu/drm/i915/intel_drv.h | 2 +
5 files changed, 182 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 8f68678..5fd5bf3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1551,6 +1551,11 @@ struct drm_i915_private {
struct i915_runtime_pm pm;
+ struct intel_digital_port *hpd_irq_port[I915_MAX_PORTS];
+ u32 long_hpd_port_mask;
+ u32 short_hpd_port_mask;
+ struct work_struct dig_port_work;
+
/* Old dri1 support infrastructure, beware the dragons ya fools entering
* here! */
struct i915_dri1_state dri1;
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index cbf31cb..9913c08 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1096,6 +1096,53 @@ static bool intel_hpd_irq_event(struct drm_device
*dev,
return true;
}
+static void i915_digport_work_func(struct work_struct *work)
+{
+ struct drm_i915_private *dev_priv =
+ container_of(work, struct drm_i915_private, dig_port_work);
+ unsigned long irqflags;
+ u32 long_port_mask, short_port_mask;
+ struct intel_digital_port *intel_dig_port;
+ int i, ret;
+ u32 old_bits = 0;
+
+ spin_lock_irqsave(dev_priv-irq_lock, irqflags);
+ long_port_mask = dev_priv-long_hpd_port_mask;
+ dev_priv-long_hpd_port_mask = 0;
+ short_port_mask = dev_priv-short_hpd_port_mask;
+ dev_priv-short_hpd_port_mask = 0;
+ spin_unlock_irqrestore(dev_priv-irq_lock, irqflags);
+
+ for (i = 0; i I915_MAX_PORTS; i++) {
+ bool valid = false;
+ bool long_hpd = false;
+ intel_dig_port = dev_priv-hpd_irq_port[i];
+ if (!intel_dig_port || !intel_dig_port-hpd_pulse)
+ continue;
+
+ if (long_port_mask (1 i)) {
+ valid = true;
+ long_hpd = true;
+ } else if (short_port_mask (1 i))
+ valid = true;
+
+ if (valid) {
+ ret = intel_dig_port-hpd_pulse(intel_dig_port, long_hpd);
+ if (ret == true) {
+ /* if we get true fallback to old school hpd */
+ old_bits |= (1 intel_dig_port-base.hpd_pin);
+ }
+ }
+ }
+
+ if (old_bits) {
+ spin_lock_irqsave(dev_priv-irq_lock, irqflags);
+ dev_priv-hpd_event_bits |= old_bits;
+ spin_unlock_irqrestore(dev_priv-irq_lock, irqflags);
+ schedule_work(dev_priv-hotplug_work);
+ }
+}
+
/*
* Handle hotplug events outside the interrupt handler proper.
*/
@@ -1520,23 +1567,104 @@ static irqreturn_t gen8_gt_irq_handler(struct
drm_device *dev,
#define HPD_STORM_DETECT_PERIOD 1000
#define HPD_STORM_THRESHOLD 5
+static int ilk_port_to_hotplug_shift(enum port port)
+{
+ switch (port) {
+ case PORT_A:
+ case PORT_E:
+ default:
+ return -1;
+ case PORT_B:
+ return 0;
+ case PORT_C:
+ return 8;
+ case PORT_D:
+ return 16;
+ }
+}
+
+static int g4x_port_to_hotplug_shift(enum port port)
+{
+ switch (port) {
+ case PORT_A:
+ case PORT_E:
+ default:
+ return -1;
+ case PORT_B:
+ return 17;
+ case PORT_C:
+ return 19;
+ case PORT_D:
+ return 21;
+ }
+}
+
+static inline enum port get_port_from_pin(enum hpd_pin pin)
+{
+ switch (pin) {
+ case HPD_PORT_B:
+ return PORT_B;
+ case HPD_PORT_C:
+ return PORT_C;
+ case HPD_PORT_D:
+ return PORT_D;
+ default:
+ return PORT_A; /* no hpd */
+ }
+}
+
static inline void intel_hpd_irq_handler(struct drm_device *dev,
u32 hotplug_trigger,
+ u32 dig_hotplug_reg,
const u32 *hpd)
{
struct drm_i915_private *dev_priv = dev-dev_private;
int i;
+ enum port port;
bool storm_detected = false;
+ bool queue_dig = false, queue_hp = false;
+ u32
Re: [Intel-gfx] [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type
On Wed, Jun 25, 2014 at 08:48:32AM +0200, Paolo Bonzini wrote: It is only slightly better, but the right solution is to fix the driver. There is absolutely zero reason why a graphics driver should know about the vendor/device ids of the PCH. There is a very valid reason to know about the PCH since it _is_ part of the gpu. You don't notice this all that much in the driver since the hardware provides magic mmio regions in the main vga device register bar which remap to the relevant PCH register ranges. And an awful lot of other stuff like the MCH. So from an abstract pov you could only pass the intel igt to a guest if you'd forward the gfx device, the mch/host bridge and the pch. Thanks to these magic mmio windows we don't need those, except that someone forgot to forward the pch pci id. So the correct fix to forward intel gpus to guests is indeed to somehow fake the pch pci ids since the driver really needs them. Gross design, but that's how the hardware works. -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] [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type
On Wed, Jun 25, 2014 at 10:28:21AM +0800, Chen, Tiejun wrote: On 2014/6/24 10:59, Zhenyu Wang wrote: On 2014.06.19 17:53:51 +0800, Tiejun Chen wrote: Originally the reason to probe ISA bridge instead of Dev31:Fun0 is to make graphics device passthrough work easy for VMM, that only need to expose ISA bridge to let driver know the real hardware underneath. This is a requirement from virtualization team. Especially in that virtualized environments, XEN, there is irrelevant ISA bridge in the system with that legacy qemu version specific to xen, qemu-xen-traditional. So to work reliably, we should scan through all the ISA bridge devices and check for the first match, instead of only checking the first one. But actually, qemu-xen-traditional, is always enumerated with Dev31:Fun0, 00:1f.0 as follows: hw/pt-graphics.c: intel_pch_init() | + pci_isa_bridge_init(bus, PCI_DEVFN(0x1f, 0), ...); so this mean that isa bridge is still represented with Dev31:Func0 like the native OS. Furthermore, currently we're pushing VGA passthrough support into qemu upstream, and with some discussion, we wouldn't set the bridge class type and just expose this devfn. So we just go back to check devfn to make life normal. Signed-off-by: Tiejun Chen tiejun.c...@intel.com This was added historically when supporting graphics device passthrough. Looks qemu upstream can't accept multiple ISA bridge and our PCH is always on device 31: func0 as far as I know. Looks good to me. Reviewed-by: Zhenyu Wang zhen...@linux.intel.com Thanks for your review. Do you know when this can be applied? I'll hold off merging until we have buy-in from upstream quemu on a given approach (which should work for both linux and windows). -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] linux-next: Tree for Jun 19 (drm/i915)
On Wed, Jun 25, 2014 at 01:01:36AM +0200, Rafael J. Wysocki wrote: On Tuesday, June 24, 2014 02:43:02 PM Jani Nikula wrote: On Thu, 19 Jun 2014, Randy Dunlap rdun...@infradead.org wrote: On 06/18/14 23:16, Stephen Rothwell wrote: Hi all, The powerpc allyesconfig is again broken more than usual. Changes since 20140618: on i386: CONFIG_ACPI is not enabled. CC drivers/gpu/drm/i915/i915_drv.o ../drivers/gpu/drm/i915/i915_drv.c: In function 'i915_drm_freeze': ../drivers/gpu/drm/i915/i915_drv.c:547:2: error: implicit declaration of function 'acpi_target_system_state' [-Werror=implicit-function-declaration] ../drivers/gpu/drm/i915/i915_drv.c:547:36: error: 'ACPI_STATE_S3' undeclared (first use in this function) ../drivers/gpu/drm/i915/i915_drv.c:547:36: note: each undeclared identifier is reported only once for each function it appears in CC net/dccp/qpolicy.o cc1: some warnings being treated as errors make[5]: *** [drivers/gpu/drm/i915/i915_drv.o] Error 1 Thanks for the report, we'll fix it. Can anyone explain why include/linux/acpi_bus.h has #ifdef CONFIG_ACPI_SLEEP and conditional build for a dummy inline version of acpi_target_system_state(), *but* that does not get included or used if CONFIG_ACPI=n? Additionally, the combination of CONFIG_ACPI=y and CONFIG_ACPI_SLEEP=n does not seem to work at all. These two things look like bugs to me. Most likely not tested thoruoughly enough. So we'll really have to sprinkle #ifdef CONFIG_ACPI all over, instead of neatly using the dummy versions that someone has gone through the trouble of adding? No, we don't have to. Back from my vacation and I didn't see a conclusion to this issue here. Rafael, have you fixed this in your acpi tree or do I need to do something in drm-intel? -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] [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type
Il 07/07/2014 16:49, Daniel Vetter ha scritto: So the correct fix to forward intel gpus to guests is indeed to somehow fake the pch pci ids since the driver really needs them. Gross design, but that's how the hardware works. A way that could work for virtualization is this: if you find the card has a magic subsystem vendor id, fetch the subsystem device id and use _that_ as the PCH device id. Would that work for you? Paolo ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] No boot console/Plane B assertion value on 945GM hardware
On Sat, Jun 21, 2014 at 01:57:32PM +0200, Thomas Richter wrote: Hi Daniel, dear intel experts, this a bug report for the intel i945GM integrated graphics chipset (*NOT* the 830GM this time). Since at least 3.12.0, but also with the latest intel-drm-nightly, I have no boot console on the 945GM chipset, the screen just remains blank. /var/log/messages shows an assertion failure, see the attached kernel log for details, booted with drm.debug=0x0e. The problem appears if the kernel is booted with the vga=792 option - note that the panel in this laptop is a 1024x768 and thus this is the native resolution. The problem does not appear with a text console. This sounds remarkably similar to the deadlock the 830GM had a while ago when booting with a graphical console, also with vga=792. Should be unrelated - more likely it's some fumble in the modeset sequence, i945 is known to be picky in that area. Just to confirm: Latest 3.16-rc kernels are still broken? This is the kernel warning: Jun 21 13:44:26 kahvi kernel: [4.780120] WARNING: CPU: 1 PID: 239 at drivers/gpu/drm/i915/intel_display.c:1307 assert_planes_disabled+0x110/0x160 [i915]() Jun 21 13:44:26 kahvi kernel: [4.780126] plane B assertion failure, should be off on pipe B but is still active Can you please boot with drm.debug=0xe and grab a new dmesg with the backtrace? The machine is a Fujitsu Siemens T4215. lspci lists the following hardware: 00:00.0 Host bridge: Intel Corporation Mobile 945GM/PM/GMS, 943/940GML and 945GT Express Memory Controller Hub (rev 03) 00:02.0 VGA compatible controller: Intel Corporation Mobile 945GM/GMS, 943/940GML Express Integrated Graphics Controller (rev 03) 00:02.1 Display controller: Intel Corporation Mobile 945GM/GMS/GME, 943/940GML Express Integrated Graphics Controller (rev 03) 00:1b.0 Audio device: Intel Corporation NM10/ICH7 Family High Definition Audio Controller (rev 02) 00:1c.0 PCI bridge: Intel Corporation NM10/ICH7 Family PCI Express Port 1 (rev 02) 00:1c.1 PCI bridge: Intel Corporation NM10/ICH7 Family PCI Express Port 2 (rev 02) 00:1c.2 PCI bridge: Intel Corporation NM10/ICH7 Family PCI Express Port 3 (rev 02) 00:1d.0 USB controller: Intel Corporation NM10/ICH7 Family USB UHCI Controller #1 (rev 02) 00:1d.1 USB controller: Intel Corporation NM10/ICH7 Family USB UHCI Controller #2 (rev 02) 00:1d.2 USB controller: Intel Corporation NM10/ICH7 Family USB UHCI Controller #3 (rev 02) 00:1d.3 USB controller: Intel Corporation NM10/ICH7 Family USB UHCI Controller #4 (rev 02) 00:1d.7 USB controller: Intel Corporation NM10/ICH7 Family USB2 EHCI Controller (rev 02) 00:1e.0 PCI bridge: Intel Corporation 82801 Mobile PCI Bridge (rev e2) 00:1f.0 ISA bridge: Intel Corporation 82801GBM (ICH7-M) LPC Interface Bridge (rev 02) 00:1f.1 IDE interface: Intel Corporation 82801G (ICH7 Family) IDE Controller (rev 02) 00:1f.2 SATA controller: Intel Corporation 82801GBM/GHM (ICH7-M Family) SATA Controller [AHCI mode] (rev 02) 00:1f.3 SMBus: Intel Corporation NM10/ICH7 Family SMBus Controller (rev 02) 02:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8055 PCI-E Gigabit Ethernet Controller (rev 12) 05:00.0 Network controller: Intel Corporation PRO/Wireless 4965 AG or AGN [Kedron] Network Connection (rev 61) 08:03.0 CardBus bridge: O2 Micro, Inc. OZ711MP1/MS1 MemoryCardBus Controller (rev 20) 08:03.1 CardBus bridge: O2 Micro, Inc. OZ711MP1/MS1 MemoryCardBus Controller (rev 20) 08:03.2 SD Host controller: O2 Micro, Inc. Integrated MMC/SD Controller 08:03.3 Bridge: O2 Micro, Inc. Integrated MS/xD Controller As soon as X is initialized, graphics work again, I just don't have a boot console. So once X is running fbcon also works again, i.e. it's only the initial boot console that's black? 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] No boot console/Plane B assertion value on 945GM hardware
On Mon, Jul 07, 2014 at 05:04:41PM +0200, Daniel Vetter wrote: On Sat, Jun 21, 2014 at 01:57:32PM +0200, Thomas Richter wrote: Hi Daniel, dear intel experts, this a bug report for the intel i945GM integrated graphics chipset (*NOT* the 830GM this time). Since at least 3.12.0, but also with the latest intel-drm-nightly, I have no boot console on the 945GM chipset, the screen just remains blank. /var/log/messages shows an assertion failure, see the attached kernel log for details, booted with drm.debug=0x0e. The problem appears if the kernel is booted with the vga=792 option - note that the panel in this laptop is a 1024x768 and thus this is the native resolution. The problem does not appear with a text console. This sounds remarkably similar to the deadlock the 830GM had a while ago when booting with a graphical console, also with vga=792. Should be unrelated - more likely it's some fumble in the modeset sequence, i945 is known to be picky in that area. Just to confirm: Latest 3.16-rc kernels are still broken? This is the kernel warning: Jun 21 13:44:26 kahvi kernel: [4.780120] WARNING: CPU: 1 PID: 239 at drivers/gpu/drm/i915/intel_display.c:1307 assert_planes_disabled+0x110/0x160 [i915]() Jun 21 13:44:26 kahvi kernel: [4.780126] plane B assertion failure, should be off on pipe B but is still active Can you please boot with drm.debug=0xe and grab a new dmesg with the backtrace? The machine is a Fujitsu Siemens T4215. lspci lists the following hardware: 00:00.0 Host bridge: Intel Corporation Mobile 945GM/PM/GMS, 943/940GML and 945GT Express Memory Controller Hub (rev 03) 00:02.0 VGA compatible controller: Intel Corporation Mobile 945GM/GMS, 943/940GML Express Integrated Graphics Controller (rev 03) 00:02.1 Display controller: Intel Corporation Mobile 945GM/GMS/GME, 943/940GML Express Integrated Graphics Controller (rev 03) 00:1b.0 Audio device: Intel Corporation NM10/ICH7 Family High Definition Audio Controller (rev 02) 00:1c.0 PCI bridge: Intel Corporation NM10/ICH7 Family PCI Express Port 1 (rev 02) 00:1c.1 PCI bridge: Intel Corporation NM10/ICH7 Family PCI Express Port 2 (rev 02) 00:1c.2 PCI bridge: Intel Corporation NM10/ICH7 Family PCI Express Port 3 (rev 02) 00:1d.0 USB controller: Intel Corporation NM10/ICH7 Family USB UHCI Controller #1 (rev 02) 00:1d.1 USB controller: Intel Corporation NM10/ICH7 Family USB UHCI Controller #2 (rev 02) 00:1d.2 USB controller: Intel Corporation NM10/ICH7 Family USB UHCI Controller #3 (rev 02) 00:1d.3 USB controller: Intel Corporation NM10/ICH7 Family USB UHCI Controller #4 (rev 02) 00:1d.7 USB controller: Intel Corporation NM10/ICH7 Family USB2 EHCI Controller (rev 02) 00:1e.0 PCI bridge: Intel Corporation 82801 Mobile PCI Bridge (rev e2) 00:1f.0 ISA bridge: Intel Corporation 82801GBM (ICH7-M) LPC Interface Bridge (rev 02) 00:1f.1 IDE interface: Intel Corporation 82801G (ICH7 Family) IDE Controller (rev 02) 00:1f.2 SATA controller: Intel Corporation 82801GBM/GHM (ICH7-M Family) SATA Controller [AHCI mode] (rev 02) 00:1f.3 SMBus: Intel Corporation NM10/ICH7 Family SMBus Controller (rev 02) 02:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8055 PCI-E Gigabit Ethernet Controller (rev 12) 05:00.0 Network controller: Intel Corporation PRO/Wireless 4965 AG or AGN [Kedron] Network Connection (rev 61) 08:03.0 CardBus bridge: O2 Micro, Inc. OZ711MP1/MS1 MemoryCardBus Controller (rev 20) 08:03.1 CardBus bridge: O2 Micro, Inc. OZ711MP1/MS1 MemoryCardBus Controller (rev 20) 08:03.2 SD Host controller: O2 Micro, Inc. Integrated MMC/SD Controller 08:03.3 Bridge: O2 Micro, Inc. Integrated MS/xD Controller As soon as X is initialized, graphics work again, I just don't have a boot console. So once X is running fbcon also works again, i.e. it's only the initial boot console that's black? Maybe go right ahead an file a bug on bugs.freedesktop.org against dri - drm (Intel). And don't forget to add [regression] to the summary so that we properly track this issue. Otherwise it might get lost again :( -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] screen update problems with Intel HD 4600 + virtual screen
On Mon, Jun 23, 2014 at 12:57:44PM +0200, Krzysztof Halasa wrote: Chris Wilson ch...@chris-wilson.co.uk writes: I'll leave that to Daniel to try and combine FBC with CRTC viewports... Let me know if you need more tests, this problems happens on both my i5 4200M laptop and on a desktop PC with i7 4770k (this machine, now fixed). I'll file it under stuff we need to fix before enabling fbc for real. And we need a testcase for this, too. For now I guess you have to life with disabling fbc. -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] Linux 3.16-rc2
On Wed, Jul 02, 2014 at 06:18:41PM +0200, Thomas Meyer wrote:
Am Montag, den 30.06.2014, 11:09 +0100 schrieb Chris Wilson:
On Mon, Jun 30, 2014 at 12:02:20PM +0200, Pavel Machek wrote:
On Tue 2014-06-24 13:27:37, Chris Wilson wrote:
On Tue, Jun 24, 2014 at 02:24:30PM +0200, Thomas Meyer wrote:
Am Dienstag, den 24.06.2014, 12:57 +0100 schrieb Chris Wilson:
On Tue, Jun 24, 2014 at 02:06:24PM +0300, Jani Nikula wrote:
On Tue, 24 Jun 2014, Thomas Meyer tho...@m3y3r.de wrote:
the i915 driver is still broken in 3.16-rc2. Resume from ram
crashes the
X server.
This is not new to 3.16-rc2; apparently we've had it since
v3.15-rc4
[1]. Also related [2].
Chris, any fresh ideas?
Nope. The bug is https://bugs.freedesktop.org/show_bug.cgi?id=76554
everything we know and have tried is there. Which is not much more
than
at time of the original incarnation:
commit 50aa253d820ad4577e2231202f2c8fd89f9dc4e6
Author: Keith Packard kei...@keithp.com
Date: Tue Oct 14 17:20:35 2008 -0700
i915: Fix up ring initialization to cover G45 oddities
G45 appears quite sensitive to ring initialization register
writes,
sometimes leaving the HEAD register with the START register
contents. Check
to make sure HEAD is reset correctly when START is written, and
fix it up,
screaming loudly.
-Chris
Hi,
so why not revert 78f2975eec9faff353a6194e854d3d39907bab68 (drm/i915:
Move all ring resets before setting the HWS page) ?
Because that patch was in response to a boot time regression.
It seems we are in a fairly ugly fix one board, it breaks another
iterations, right?
(BTW, if you apply patch to fix this bug, could you Cc me? I have strange
feeling
that it will break my setup... Actually, it probably makes sense to Cc
all the people
who reported problems with ring initialization...
What patch caused the boot time regression you are talking about? We need
to get
list of commits involved in this, and revert the original one...
commit 9991ae787a0c87fe7c783b4b6f4754c3cdbb6213
Author: Chris Wilson ch...@chris-wilson.co.uk
Date: Wed Apr 2 16:36:07 2014 +0100
drm/i915: Move all ring resets before setting the HWS page
In commit a51435a3137ad8ae75c288c39bd2d8b2696bae8f
Author: Naresh Kumar Kachhi naresh.kumar.kac...@intel.com
Date: Wed Mar 12 16:39:40 2014 +0530
drm/i915: disable rings before HW status page setup
we reordered stopping the rings to do so before we set the HWS register.
However, there is an extra workaround for g45 to reset the rings twice,
and for consistency we should apply that workaround before setting the
HWS to be sure that the rings are truly stopped.
Cc: Naresh Kumar Kachhi naresh.kumar.kac...@intel.com
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
commit a51435a3137ad8ae75c288c39bd2d8b2696bae8f
Author: Naresh Kumar Kachhi naresh.kumar.kac...@intel.com
Date: Wed Mar 12 16:39:40 2014 +0530
drm/i915: disable rings before HW status page setup
Rings should be idle before issuing sync_flush
(in intel_ring_setup_status_page). This patch moves the ring
disabling before doing the HW status page setup.
Signed-off-by: Naresh Kumar Kachhi naresh.kumar.kac...@intel.com
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
Hi,
this patch on top of v3.16-rc3-62-gd92a333 makes the resume from ram
regression go away on my machine:
Hm, we could conditionalize this hack on IS_G4X ... Chris, thoughts?
-Daniel
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 279488a..b896ac8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -459,22 +459,25 @@ static bool stop_ring(struct intel_engine_cs *ring)
{
struct drm_i915_private *dev_priv = to_i915(ring-dev);
- if (!IS_GEN2(ring-dev)) {
- I915_WRITE_MODE(ring, _MASKED_BIT_ENABLE(STOP_RING));
- if (wait_for_atomic((I915_READ_MODE(ring) MODE_IDLE) != 0,
1000)) {
- DRM_ERROR(%s :timed out trying to stop ring\n,
ring-name);
- return false;
- }
- }
-
+// if (!IS_GEN2(ring-dev)) {
+// I915_WRITE_MODE(ring, _MASKED_BIT_ENABLE(STOP_RING));
+// if (wait_for_atomic((I915_READ_MODE(ring) MODE_IDLE) != 0,
1000)) {
+// DRM_ERROR(%s :timed out trying to stop
Re: [Intel-gfx] [PATCH v4 maintainer-tools] qf: Use git remote rm instead of git remote remove
On Tue, Jun 24, 2014 at 05:20:36PM +0100, Damien Lespiau wrote: 'remove' is not recognized is a slightly older git (1.7.9.5) on a slightly older distro. Use 'rm' instead, which also work on the git version listing 'remove' in git remote help (1.8.3.1). Signed-off-by: Damien Lespiau damien.lesp...@intel.com Merged, thanks for the patch. -Daniel --- qf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qf b/qf index 67a3ba6..adf95ca 100755 --- a/qf +++ b/qf @@ -125,7 +125,7 @@ function repo_init # setup for quilt patch repo git clone --local --no-checkout -- . patches cd patches - git remote remove origin + git remote rm origin # include remotes and branches and all that from parent repo git config include.path ../../.git/config -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Linux 3.16-rc2
On Mon, 7 Jul 2014, Daniel Vetter wrote: this patch on top of v3.16-rc3-62-gd92a333 makes the resume from ram regression go away on my machine: Hm, we could conditionalize this hack on IS_G4X ... Chris, thoughts? For the record, commenting out the code in stop_ring() the way Thomas did doesn't fix my problem (the one reported in [1]). [1] https://bugs.freedesktop.org/show_bug.cgi?id=76554 -- Jiri Kosina SUSE Labs ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] drm/i915: Implement basic Displayport automated testing function for EDID operations
On Tue, Jun 24, 2014 at 03:12:51PM -0700, Todd Previte wrote:
Implements some of the basic EDID tests for Displayport compliance. These
tests
include reading the EDID, verifying the checksum and writing the test
responses
back to the sink device.
Signed-off-by: Todd Previte tprev...@gmail.com
---
drivers/gpu/drm/i915/intel_dp.c | 36 +++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 43fcabe..d060853 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3352,8 +3352,42 @@ intel_dp_autotest_video_pattern(struct intel_dp
*intel_dp)
static uint8_t
intel_dp_autotest_edid(struct intel_dp *intel_dp)
{
+ struct drm_connector *connector = intel_dp-attached_connector-base;
+ struct i2c_adapter *adapter = intel_dp-aux.ddc;
+ struct edid *edid_read = NULL;
+ uint8_t *edid_data = NULL;
uint8_t test_result = DP_TEST_NAK;
- return test_result;
+ uint32_t i = 0, ret = 0, checksum = 0;
+ struct drm_display_mode *use_mode = NULL;
+ dp_compliance_mode comp_mode_type = DP_COMPLIANCE_MODE_PREFERRED;
+ int mode_count = 0;
+
+ edid_read = drm_get_edid(connector, adapter);
+
+ DRM_DEBUG_KMS(Displayport: EDID automated test\n);
+
+ if (edid_read) {
+ test_result = true;
+ edid_data = (uint8_t*) edid_read;
+ // Compute checksum
Checkpatch isn't approving of // style comments, and there's other issues.
-Daniel
+ for (i = 0; i 128; i++)
+ checksum += edid_data[i];
+
+ DRM_DEBUG_KMS(Displayport: EDID test - computed byte sum =
%02x\n, checksum);
+ // Verify EDID checksum
+ if (checksum % 256 == 0) {
+ /* Write the checksum to EDID checksum register */
+ ret = drm_dp_dpcd_write(intel_dp-aux,
DP_TEST_EDID_CHECKSUM, edid_read-checksum, 1);
+ // Reponse is ACK and and checksum written
+ test_result = DP_TEST_ACK | DP_TEST_EDID_CHECKSUM_WRITE;
+ DRM_DEBUG_KMS(Displayport: EDID test - checksum =
%02x\n, edid_read-checksum);
+ }
+ else {
+ // Invalid checksum, set for failsafe mode
+ comp_mode_type = DP_COMPLIANCE_MODE_FAILSAFE;
+ }
+ }
+return test_result;
}
/* Displayport compliance testing - PHY pattern testing */
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+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 4/8] drm/i915: Rename ctx-id to ctx-handle
On Thu, Jul 03, 2014 at 12:01:35PM +, Mateo Lozano, Oscar wrote: -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Thursday, July 03, 2014 10:53 AM To: Mateo Lozano, Oscar; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 4/8] drm/i915: Rename ctx-id to ctx-handle On Thu, Jul 03, 2014 at 10:30:52AM +0100, Chris Wilson wrote: On Thu, Jun 26, 2014 at 02:24:15PM +0100, oscar.ma...@intel.com wrote: From: Oscar Mateo oscar.ma...@intel.com This is an Execlists preparatory patch, since they make context ID become an overloaded term: - In the software, it was used to distinguish which context userspace was trying to use. - In the BSpec, the term is used to describe the 20-bits long field the hardware uses to it to discriminate the contexts that are submitted to the ELSP and inform the driver about their current status (via Context Switch Interrupts and Context Status Buffers). Initially, I tried to make the different meanings converge, but it proved impossible: - The software ctx-id is per-filp, while the hardware one needs to be globally unique. - Also, we multiplex several backing states objects per intel_context, and all of them need unique HW IDs. - I tried adding a per-filp ID and then composing the HW context ID as: ctx-id + file_priv-id + ring-id, but the fact that the hardware only uses 20-bits means we have to artificially limit the number of filps or contexts the userspace can create. The ctx-handle bits are done with this Cocci patch (plus manual frobbing of the struct declaration): @@ struct intel_context c; @@ - (c).id + c.handle @@ struct intel_context *c; @@ - (c)-id + c-handle Also, while we are at it, s/DEFAULT_CONTEXT_ID/DEFAULT_CONTEXT_HANDLE. Signed-off-by: Oscar Mateo oscar.ma...@intel.com Let's go whole hog here and call it ctx-user_handle. ACK And it's unsigned and only 32bits... ACK, I´ll change the type to unit32_t Aside when resending please pull in all the r-b tags from Jesse so that lazy me has less hassle when merging this ;-) -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] tests/gem_exec_parse: use gem_uses_aliasing_ppgtt
Suggested by Brad Volking.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
tests/gem_exec_parse.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index f7376e391ee9..5bab4db777b3 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -198,7 +198,7 @@ int fd;
igt_main
{
igt_fixture {
- int parser_version = 0, has_ppgtt = 0;
+ int parser_version = 0;
drm_i915_getparam_t gp;
int rc;
@@ -209,10 +209,7 @@ igt_main
rc = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, gp);
igt_require(!rc parser_version 0);
- gp.param = I915_PARAM_HAS_ALIASING_PPGTT;
- gp.value = has_ppgtt;
- rc = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, gp);
- igt_require(!rc has_ppgtt 0);
+ igt_require(gem_uses_aliasing_ppgtt(fd));
handle = gem_create(fd, 4096);
--
2.0.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 00/44] GPU scheduler for i915 driver
On Fri, Jun 27, 2014 at 06:44:04AM +1000, Dave Airlie wrote: Implemented a batch buffer submission scheduler for the i915 DRM driver. While this seems very interesting, you might want to address in the commit msg or the cover email a) why this is needed, b) any improvements in speed, power consumption or throughput it generates, i.e. benchmarks. also some notes on what hw supports preemption. Also tests to both exercise the rescheduling (i.e. let a high-prio batch compete agains a large pile of low-prio batches) and augmenting our gpu sync tests (from Damien) with that, too. -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] Linux 3.16-rc2
On Mon, Jul 07, 2014 at 05:16:13PM +0200, Daniel Vetter wrote: On Wed, Jul 02, 2014 at 06:18:41PM +0200, Thomas Meyer wrote: Hi, this patch on top of v3.16-rc3-62-gd92a333 makes the resume from ram regression go away on my machine: Hm, we could conditionalize this hack on IS_G4X ... Chris, thoughts? As different machines favour different w/a, I think the issue is mostly timing related. It could be sequence of register writes, but we tried different orders early on. The next experiment I guess would be to insert small delays between each write to see if that helps. Or to write each register twice. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] lib/kms: Provide universal plane #define's
There hasn't been a libdrm release containing the universal plane definitions yet, so add them to igt_kms to allow compilation to succeed in the meantime. Signed-off-by: Matt Roper matthew.d.ro...@intel.com --- lib/igt_kms.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/lib/igt_kms.c b/lib/igt_kms.c index 2442c4b..d792008 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -42,6 +42,20 @@ #include igt_kms.h #include igt_aux.h +/* + * There hasn't been a release of libdrm containing these #define's yet, so + * copy them here to allow compilation to succeed in the mean time. + * + * We can drop these #define's and just make i-g-t depend on the proper libdrm + * version in the future. + */ +#define DRM_CLIENT_CAP_UNIVERSAL_PLANES 2 +#define DRM_PLANE_TYPE_OVERLAY 0 +#define DRM_PLANE_TYPE_PRIMARY 1 +#define DRM_PLANE_TYPE_CURSOR 2 + + + /** * SECTION:igt_kms * @short_description: Kernel modesetting support library -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] intel-gpu-tools: Re-use igt_subtest_init for simple tests
On Fri, Jun 27, 2014 at 03:15:37PM +0100, tim.g...@intel.com wrote:
From: Tim Gore tim.g...@intel.com
igt_subtest_init mainly does stuff that we also want for
simple/single tests, such as looking for --list-subtests
and --help options and calling common_init. So just call
this from igt_simple_init and then set tests_with_subtests
to false. NOTE that this means that check_igt_exit is now
registered as an exit handler for single tests, so need to
make sure that ALL tests exit via igt_exit.
Signed-off-by: Tim Gore tim.g...@intel.com
---
lib/igt_core.c | 32
lib/igt_core.h | 4 ++--
tests/gem_gtt_hog.c| 2 +-
tests/igt_simulation.c | 4 ++--
4 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 7ac7ebe..aaeaa3b 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -458,13 +458,15 @@ void igt_subtest_init(int argc, char **argv)
* #igt_simple_main block instead of stitching the tests's main() function
together
* manually.
*/
-void igt_simple_init(void)
+void igt_simple_init(int argc, char **argv)
{
- print_version();
-
- oom_adjust_for_doom();
-
- common_init();
+ /* Use the same init function as is used with subtests - we want most
of its functionality */
+ /* Note that this will install the igt_exit_handler so you need to exit
via igt_exit(),*/
+ /* Dont call exit()
*/
+ igt_subtest_init(argc, argv);
Not sure whether forcing the reuse of igt_subtest_init makes a lot of
sense since it requires a lot of follow-up changes all over. I'd just add
a bit of argparsing here with the required special cases, i.e.
- exit without doing anything for --list-subtests
- exit with 77 when anything is specified with --run-subtests
Also I prefer if the piglit changes come together with this patch so that
we can roll it all out together.
-Daniel
+ test_with_subtests = false;
+ if (list_subtests)
+ igt_exit();
}
/*
@@ -565,7 +567,7 @@ void igt_skip(const char *f, ...)
assert(in_fixture);
__igt_fixture_end();
} else {
- exit(IGT_EXIT_SKIP);
+ igt_exit();
}
}
@@ -655,7 +657,7 @@ void igt_fail(int exitcode)
__igt_fixture_end();
}
- exit(exitcode);
+ igt_exit();
}
}
@@ -713,18 +715,16 @@ void igt_exit(void)
if (igt_only_list_subtests())
exit(IGT_EXIT_SUCCESS);
- if (!test_with_subtests)
- exit(IGT_EXIT_SUCCESS);
-
- /* Calling this without calling one of the above is a failure */
- assert(skipped_one || succeeded_one || failed_one);
+ if (test_with_subtests)
+ /* Calling this without calling one of the above is a failure */
+ assert(skipped_one || succeeded_one || failed_one);
if (failed_one)
exit(igt_exitcode);
- else if (succeeded_one)
- exit(IGT_EXIT_SUCCESS);
- else
+ else if (skipped_one)
exit(IGT_EXIT_SKIP);
+ else
+ exit(IGT_EXIT_SUCCESS);
}
/* fork support code */
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 9853e6b..cabbc3b 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -166,7 +166,7 @@ bool igt_only_list_subtests(void);
*
* Init for simple tests without subtests
*/
-void igt_simple_init(void);
+void igt_simple_init(int argc, char **argv);
/**
* igt_simple_main:
@@ -178,7 +178,7 @@ void igt_simple_init(void);
#define igt_simple_main \
static void igt_tokencat(__real_main, __LINE__)(int argc, char **argv);
\
int main(int argc, char **argv) { \
- igt_simple_init(); \
+ igt_simple_init(argc, argv); \
igt_tokencat(__real_main, __LINE__)(argc, argv); \
igt_exit(); \
} \
diff --git a/tests/gem_gtt_hog.c b/tests/gem_gtt_hog.c
index 5d47540..f607ea0 100644
--- a/tests/gem_gtt_hog.c
+++ b/tests/gem_gtt_hog.c
@@ -150,7 +150,7 @@ static void run(data_t *data, int child)
munmap(ptr, size);
igt_assert(x == canary);
- exit(0);
+ _exit(0);
}
igt_simple_main
diff --git a/tests/igt_simulation.c b/tests/igt_simulation.c
index 15cbe64..3048c9e 100644
--- a/tests/igt_simulation.c
+++ b/tests/igt_simulation.c
@@ -53,11 +53,11 @@ static int do_fork(void)
assert(0);
case 0:
if (simple) {
- igt_simple_init();
+ igt_simple_init(1, argv_run);
igt_skip_on_simulation();
- exit(0);
+ igt_exit();
} else {
if (list_subtests)
igt_subtest_init(2, argv_list);
--
Re: [Intel-gfx] [PATCH 1/2] intel-gpu-tools: pass argc/argv to simple main
On Fri, Jun 27, 2014 at 03:15:36PM +0100, tim.g...@intel.com wrote:
From: Tim Gore tim.g...@intel.com
Quite a few single tests do not use the igt_simple_main
macro because they want access to argc/argv. So change the
igt_simple_main macro to pass these arguments through to the
__real_mainxxx function, and change these tests to use
the macro.
Signed-off-by: Tim Gore tim.g...@intel.com
---
lib/igt_core.h | 8
tests/gem_ctx_basic.c| 6 +-
tests/gem_exec_blt.c | 5 +
tests/gem_gtt_speed.c| 5 +
tests/gem_hang.c | 5 +
tests/gem_render_copy.c | 4 +---
tests/gem_render_linear_blits.c | 5 +
tests/gem_render_tiled_blits.c | 5 +
tests/gem_seqno_wrap.c | 11 ---
tests/gem_stress.c | 5 +
tests/gen3_mixed_blits.c | 5 +
tests/gen3_render_linear_blits.c | 5 +
tests/gen3_render_mixed_blits.c | 5 +
tests/gen3_render_tiledx_blits.c | 5 +
tests/gen3_render_tiledy_blits.c | 5 +
15 files changed, 21 insertions(+), 63 deletions(-)
diff --git a/lib/igt_core.h b/lib/igt_core.h
index e252eba..9853e6b 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -176,13 +176,13 @@ void igt_simple_init(void);
* the test needs to parse additional cmdline arguments of its own.
*/
#define igt_simple_main \
- static void igt_tokencat(__real_main, __LINE__)(void); \
+ static void igt_tokencat(__real_main, __LINE__)(int argc, char **argv);
\
int main(int argc, char **argv) { \
igt_simple_init(); \
- igt_tokencat(__real_main, __LINE__)(); \
- exit(0); \
+ igt_tokencat(__real_main, __LINE__)(argc, argv); \
+ igt_exit(); \
} \
- static void igt_tokencat(__real_main, __LINE__)(void) \
+ static void igt_tokencat(__real_main, __LINE__)(int argc, char **argv) \
Hm, I'd just add argc/argv to igt_simple_init like you do in the next
patch and update the relevant tests which have their own main. That would
be more in line with subtest-tests which have their own additional
arguments, too.
Also having functions with magic/predefined arguments is a bit too much
magic for my taste. If we keep the signature of real_main as-is we'll
avoid (highly unlikely, but still) accidental name clashes.
And with my suggestion for patch 2 to just have a bare-bones argv parsing
for simple tests we also don't need to call igt_exit.
-Daniel
__attribute__((format(printf, 1, 2)))
void igt_skip(const char *f, ...) __attribute__((noreturn));
diff --git a/tests/gem_ctx_basic.c b/tests/gem_ctx_basic.c
index 3e9b688..fe770ea 100644
--- a/tests/gem_ctx_basic.c
+++ b/tests/gem_ctx_basic.c
@@ -145,12 +145,10 @@ static void parse(int argc, char *argv[])
}
}
-int main(int argc, char *argv[])
+igt_simple_main
{
int i;
- igt_simple_init();
-
fd = drm_open_any_render();
devid = intel_get_drm_devid(fd);
@@ -173,6 +171,4 @@ int main(int argc, char *argv[])
free(threads);
close(fd);
-
- return 0;
}
diff --git a/tests/gem_exec_blt.c b/tests/gem_exec_blt.c
index 3bcef18..3d092fe 100644
--- a/tests/gem_exec_blt.c
+++ b/tests/gem_exec_blt.c
@@ -253,12 +253,10 @@ static void run(int object_size)
close(fd);
}
-int main(int argc, char **argv)
+igt_simple_main
{
int i;
- igt_simple_init();
-
igt_skip_on_simulation();
if (argc 1) {
@@ -270,5 +268,4 @@ int main(int argc, char **argv)
} else
run(OBJECT_SIZE);
- return 0;
}
diff --git a/tests/gem_gtt_speed.c b/tests/gem_gtt_speed.c
index 385eeb7..fa20de0 100644
--- a/tests/gem_gtt_speed.c
+++ b/tests/gem_gtt_speed.c
@@ -50,7 +50,7 @@ static double elapsed(const struct timeval *start,
return (1e6*(end-tv_sec - start-tv_sec) + (end-tv_usec -
start-tv_usec))/loop;
}
-int main(int argc, char **argv)
+igt_simple_main
{
struct timeval start, end;
uint8_t *buf;
@@ -59,8 +59,6 @@ int main(int argc, char **argv)
int loop, i, tiling;
int fd;
- igt_simple_init();
-
igt_skip_on_simulation();
if (argc 1)
@@ -329,5 +327,4 @@ int main(int argc, char **argv)
gem_close(fd, handle);
close(fd);
- return 0;
}
diff --git a/tests/gem_hang.c b/tests/gem_hang.c
index 6248244..a4f4d10 100644
--- a/tests/gem_hang.c
+++ b/tests/gem_hang.c
@@ -68,12 +68,10 @@ gpu_hang(void)
intel_batchbuffer_flush(batch);
}
-int main(int argc, char **argv)
+igt_simple_main
{
int fd;
- igt_simple_init();
-
igt_assert_f(argc == 2,
usage: %s disabled pipe number\n,
argv[0]);
@@ -93,5 +91,4 @@ int main(int argc, char **argv)
close(fd);
- return 0;
}
diff --git
Re: [Intel-gfx] [PATCH] drm/i915: Don't pretend ips is always enabled on BDW.
On Mon, Jun 30, 2014 at 04:17:00PM -0300, Paulo Zanoni wrote: 2014-06-30 8:45 GMT-03:00 Rodrigo Vivi rodrigo.v...@intel.com: As pointed out before we don't have a reliable way to read back ips status on BDW without the risk to disable it when reading. However now we are pretending that IPS on BDW is always on and getting people confused about it. So this patch allows people to know if ips was ever attempted to be enabled. Even if the current status is impossible to be ascertain. v2: (spotted by Paulo): * A version that at least compiles * with more clear messages * let Cheryview on the safe side until we aren't sure that checking ips state on ips won't disable it. Cc: Paulo Zanoni paulo.r.zan...@intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com Queued for -next, thanks for the patch. -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: Remove magic to prevent blank screen on gen4 chipsets
Since the root cause is understood now and with the fix
commit 564ed191f5d816d24501664296991ec70327e2bc
Author: Imre Deak imre.d...@intel.com
Date: Fri Jun 13 14:54:21 2014 +0300
drm/i915: gmch: fix stuck primary plane due to memory self-refresh mode
in place the magic for G4x chipsets introduced with commit
commit 61bc95c1fbbb6a08b55bbe161fdf1ea5493fc595
Author: Egbert Eich e...@suse.com
Date: Mon Mar 4 09:24:38 2013 -0500
DRM/i915: On G45 enable cursor plane briefly after enabling the display
plane.
to avoided occasional screen blanking on mode changes can finally
be removed.
It's been verified that Imre's fix also resolves the said issue.
Signed-off-by: Egbert Eich e...@suse.de
Tested-by: Stefan Dirsch sndir...@suse.de
---
drivers/gpu/drm/i915/intel_display.c | 27 ---
1 file changed, 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b9bc030..8c3dcdf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3894,30 +3894,6 @@ static void intel_crtc_dpms_overlay(struct intel_crtc
*intel_crtc, bool enable)
*/
}
-/**
- * i9xx_fixup_plane - ugly workaround for G45 to fire up the hardware
- * cursor plane briefly if not already running after enabling the display
- * plane.
- * This workaround avoids occasional blank screens when self refresh is
- * enabled.
- */
-static void
-g4x_fixup_plane(struct drm_i915_private *dev_priv, enum pipe pipe)
-{
- u32 cntl = I915_READ(CURCNTR(pipe));
-
- if ((cntl CURSOR_MODE) == 0) {
- u32 fw_bcl_self = I915_READ(FW_BLC_SELF);
-
- I915_WRITE(FW_BLC_SELF, fw_bcl_self ~FW_BLC_SELF_EN);
- I915_WRITE(CURCNTR(pipe), CURSOR_MODE_64_ARGB_AX);
- intel_wait_for_vblank(dev_priv-dev, pipe);
- I915_WRITE(CURCNTR(pipe), cntl);
- I915_WRITE(CURBASE(pipe), I915_READ(CURBASE(pipe)));
- I915_WRITE(FW_BLC_SELF, fw_bcl_self);
- }
-}
-
static void intel_crtc_enable_planes(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc-dev;
@@ -3930,9 +3906,6 @@ static void intel_crtc_enable_planes(struct drm_crtc
*crtc)
intel_enable_primary_hw_plane(dev_priv, plane, pipe);
intel_enable_planes(crtc);
- /* The fixup needs to happen before cursor is enabled */
- if (IS_G4X(dev))
- g4x_fixup_plane(dev_priv, pipe);
intel_crtc_update_cursor(crtc, true);
intel_crtc_dpms_overlay(intel_crtc, true);
--
1.8.4.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Grab GPU output
Hi, We're working on Video Wall project on Linux, I need to know if it's possible to grab Intel gpu output frames or not ? I also need to know if It's possible to create virtual outputs for an Intel gpu to use gpu acceleration and then grab the output frames ? Regards ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Support pf CRC source on haswell transcoder edp
On Thu, May 29, 2014 at 02:10:22PM +0200, Daniel Vetter wrote: The always-on power well pixel path on haswell is routed such that it bypasses the panel fitter when we use is. Which means the pfit CRC source won't work in that configuration. Add a new disallow-bypass flags to the pfit pipe config state and set it when we want to use the pf CRC. Results in a bit of flicker, but should get the job done. We'll also undo do it afterwards to make sure other tests arent' negatively affected. Totally untested due to lack of hsw laptops around here. v2: s/disallow_bypass/force_power_well_on/ to avoid a double negative (Damien). v3: force_thru because roadsigns. v4: Don't forget the power wells! Also note that until the runtime pm for DPMS series is fully merged the simple disable/enable trick won't work since the -crtc_mode_set callback is still required to do nasty things. This stuff is tricky, but I think by both fixing up get_crtc_power_domains and the debugfs wa code we should always grab/drop the additional power well correctly. v5: Wrap in () as suggested by Damien to avoid setting reserved values for the edp transcoder path on bdw+ References: https://bugs.freedesktop.org/show_bug.cgi?id=72864 Cc: Damien Lespiau damien.lesp...@intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Somewhat tested it now, seems to do the right thing. Reviewed-by: Damien Lespiau damien.lesp...@intel.com Tested-by: Damien Lespiau damien.lesp...@intel.com -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] DRM/i915: Remove magic to prevent blank screen on gen4 chipsets
On Mon, Jul 07, 2014 at 06:20:34PM +0200, Egbert Eich wrote:
Since the root cause is understood now and with the fix
commit 564ed191f5d816d24501664296991ec70327e2bc
Author: Imre Deak imre.d...@intel.com
Date: Fri Jun 13 14:54:21 2014 +0300
drm/i915: gmch: fix stuck primary plane due to memory self-refresh mode
in place the magic for G4x chipsets introduced with commit
commit 61bc95c1fbbb6a08b55bbe161fdf1ea5493fc595
Author: Egbert Eich e...@suse.com
Date: Mon Mar 4 09:24:38 2013 -0500
DRM/i915: On G45 enable cursor plane briefly after enabling the
display plane.
to avoided occasional screen blanking on mode changes can finally
be removed.
It's been verified that Imre's fix also resolves the said issue.
Signed-off-by: Egbert Eich e...@suse.de
Tested-by: Stefan Dirsch sndir...@suse.de
Queued for -next, thanks for the patch.
-Daniel
---
drivers/gpu/drm/i915/intel_display.c | 27 ---
1 file changed, 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b9bc030..8c3dcdf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3894,30 +3894,6 @@ static void intel_crtc_dpms_overlay(struct intel_crtc
*intel_crtc, bool enable)
*/
}
-/**
- * i9xx_fixup_plane - ugly workaround for G45 to fire up the hardware
- * cursor plane briefly if not already running after enabling the display
- * plane.
- * This workaround avoids occasional blank screens when self refresh is
- * enabled.
- */
-static void
-g4x_fixup_plane(struct drm_i915_private *dev_priv, enum pipe pipe)
-{
- u32 cntl = I915_READ(CURCNTR(pipe));
-
- if ((cntl CURSOR_MODE) == 0) {
- u32 fw_bcl_self = I915_READ(FW_BLC_SELF);
-
- I915_WRITE(FW_BLC_SELF, fw_bcl_self ~FW_BLC_SELF_EN);
- I915_WRITE(CURCNTR(pipe), CURSOR_MODE_64_ARGB_AX);
- intel_wait_for_vblank(dev_priv-dev, pipe);
- I915_WRITE(CURCNTR(pipe), cntl);
- I915_WRITE(CURBASE(pipe), I915_READ(CURBASE(pipe)));
- I915_WRITE(FW_BLC_SELF, fw_bcl_self);
- }
-}
-
static void intel_crtc_enable_planes(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc-dev;
@@ -3930,9 +3906,6 @@ static void intel_crtc_enable_planes(struct drm_crtc
*crtc)
intel_enable_primary_hw_plane(dev_priv, plane, pipe);
intel_enable_planes(crtc);
- /* The fixup needs to happen before cursor is enabled */
- if (IS_G4X(dev))
- g4x_fixup_plane(dev_priv, pipe);
intel_crtc_update_cursor(crtc, true);
intel_crtc_dpms_overlay(intel_crtc, true);
--
1.8.4.5
--
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 i-g-t 0/6] Make kms_pipe_crc_basic cover pipe A + eDP on HSW
On Wed, May 28, 2014 at 07:23:58PM +0100, Damien Lespiau wrote: We currenly have a bug on HSW where asking for the panel fitter CRC when the power well is down returns 0x, but kms_pipe_crc_basic passes as it only tests that the CRCs obtained for a FB are consistent over time. We consistently read 0x, so the test was passing. With this series we now scan out 2 different fbs and make sure the CRCs for those two fbs are indeed, different. This makes: $ sudo ./tests/kms_pipe_crc_basic --run-subtest read-crc-pipe-A-frame-sequence fail when the power well is down, while: $ sudo ./tests/kms_pipe_crc_basic --run-subtest read-crc-pipe-B-frame-sequence succeeds, using pipe B turning the power well on. -- Damien Damien Lespiau (6): kms_pipe_crc_basic: Split the main test function a bit more kms_pipe_crc_basic: Cycle between 2 differently colored buffer kms_pipe_crc_basic: Make the number of CRCs a parameter kms_pipe_crc_basic: Bump the number of collected CRCs to 60 kms_pipe_crc_basic: Add a bit a debugging output lib: Add a --igt-debug command line option So merged 4 of those patches, excluding the ones with review comments from Daniel, there are not the meat of the series anyway. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] lib: Reset 'position_changed' after a drmModeSetCrtc()
So the next commit won't trigger a drmModeSetCrtc() if the primary plane doesn't have any update needing it. This shouldn't be a problem at the moment as we don't allow the primary plane to be of a different size than the CRTC viewport, but it will most likely change in the future and we don't want to have that bug there. Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- lib/igt_kms.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/igt_kms.c b/lib/igt_kms.c index d792008..82bdec5 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -1025,6 +1025,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary, primary-pipe-enabled = (fb_id != 0); primary-fb_changed = false; + primary-position_changed = false; return 0; } -- 1.8.3.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] pipe_crc: Make collect_crc() ensure the CRC looks somewhat valid
Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- lib/igt_debugfs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c index 5c4bbc5..d4a6cf6 100644 --- a/lib/igt_debugfs.c +++ b/lib/igt_debugfs.c @@ -527,6 +527,8 @@ void igt_pipe_crc_collect_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out_crc) igt_pipe_crc_start(pipe_crc); read_one_crc(pipe_crc, out_crc); igt_pipe_crc_stop(pipe_crc); + + igt_assert(!igt_crc_is_null(out_crc)); } /* -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/2] pipe_crc: Warn if the CRC values is 0xffffffff
This is what we read when the CRC logic in in a powered down well. We
really don't want that to happen.
In theory, it's possible 0x to be a valid CRC value, so I don't
assert here.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
lib/igt_debugfs.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 2f655a1..5c4bbc5 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -199,9 +199,14 @@ bool igt_crc_is_null(igt_crc_t *crc)
{
int i;
- for (i = 0; i crc-n_words; i++)
+ for (i = 0; i crc-n_words; i++) {
+ if (crc-crc[i] == 0x)
+ igt_warn(Suspicious CRC: it looks like the CRC
+read back was from a register in a powered
+down well\n);
if (crc-crc[i])
return false;
+ }
return true;
}
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [I-G-T][PATCH] Add panning test for primary plane.
On Fri, May 23, 2014 at 08:28:47AM +0800, Yi Sun wrote:
Get CRCs of a full red and a full blue surface as reference.
Create a big framebuffer that is twice width and twice height as the
current display mode.
Fill the top left quarter with red, bottom right quarter with blue
Check the scanned out image with the CRTC at position (0, 0) of the
framebuffer and it should be the same CRC as the full red fb
Check the scanned out image with the CRTC at position (hdisplay,
vdisplay) and it should be the same CRC as the full blue fb
Signed-off-by: Lei Liu lei.a@intel.com
Signed-off-by: Yi Sun yi@intel.com
I've very sorry for the time it took to get this patch somewhere on top
of my TODO list. To redeem myself I've rebased it, fixed it up and
send the result to the mailing list.
I went ahead with only testing the primary plane at the moment. I think
instead of set_panning() it would make more sense to have a
igt_plane_set_source_rectangle() so it'd work better with the drm plane
API. That's for another time though.
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index fffad9f..5cdc48b 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -532,6 +532,13 @@ void igt_display_init(igt_display_t *display, int drm_fd)
display-outputs = calloc(display-n_outputs, sizeof(igt_output_t));
igt_assert(display-outputs);
+ /*
+ * Be default display can scan out from the original position of the
frame buffer.
+ * But we can change this position making display scan out with a
offset.
+ */
+ display-buffer_x = 0;
+ display-buffer_y = 0;
So, nop. the panning offset is a property of a plane (each plane can
have one), so that's something we need in the plane structure.
for (i = 0; i display-n_outputs; i++) {
igt_output_t *output = display-outputs[i];
@@ -830,13 +837,13 @@ static int igt_output_commit(igt_output_t *output)
igt_output_name(output),
pipe_name(output-config.pipe),
fb_id,
- 0, 0,
+ display-buffer_x, display-buffer_y,
mode-hdisplay, mode-vdisplay);
ret = drmModeSetCrtc(display-drm_fd,
crtc_id,
fb_id,
- 0, 0, /* x, y */
+ display-buffer_x,
display-buffer_y, /* x, y */
output-id,
1,
mode);
@@ -849,7 +856,7 @@ static int igt_output_commit(igt_output_t *output)
ret = drmModeSetCrtc(display-drm_fd,
crtc_id,
fb_id,
- 0, 0, /* x, y */
+ display-buffer_x,
display-buffer_y, /* x, y */
We are disabling the pipe here, setting the panning doesn't do anything
in that case.
NULL, /* connectors */
0,/* n_connectors */
NULL /* mode */);
@@ -987,6 +994,26 @@ void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb
*fb)
plane-fb_changed = true;
}
+void igt_plane_set_fb_offset(igt_plane_t *plane, struct igt_fb *fb, int x,
int y)
+{
This function foward declaration needs to be added to the header file.
+ igt_pipe_t *pipe = plane-pipe;
+ igt_display_t *display = pipe-display;
+
+ (*display).buffer_x = x;
+ (*display).buffer_y = y;
Eek. display-buffer_x!
+
+ LOG(display, %c.%d: plane_set_fb(%d)\n, pipe_name(pipe-pipe),
+ plane-index, fb ? fb-fb_id : 0);
Of course, the logging message needs to be updated with set_panning(x, y)
+ plane-fb = fb;
This is against the orthogonality principle of the (any!) API, setting
the fb and setting the panning (x,y) are two different things, each one
with its own entry point.
+
+ if (plane-is_primary)
+ pipe-need_set_crtc = true;
+ else if (plane-is_cursor)
+ pipe-need_set_cursor = true;
+
+ plane-fb_changed = true;
For state tracking, we entirely switched to defining what has changed in
the setters (here, the panning (x,y)) and resolve which ioctl() needs to
be called in the commit function (because the idea is to be able to test
the different paths we have to drive the display (legacy, universal
planes + properties, atomic ioctl).
+}
void igt_plane_set_position(igt_plane_t *plane, int x, int y)
{
igt_pipe_t *pipe = plane-pipe;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 439a634..cb9370e 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -139,6 +139,8 @@ struct
[Intel-gfx] [PATCH i-g-t 1/2] kms_plane: Add panning test for primary plane
From: Yi Sun yi@intel.com
Get CRCs of a full red and a full blue surface as reference.
Create a big framebuffer that is twice width and twice height as the
current display mode.
Fill the top left quarter with red, bottom right quarter with blue
Check the scanned out image with the CRTC at position (0, 0) of the
framebuffer and it should be the same CRC as the full red fb
Check the scanned out image with the CRTC at position (hdisplay,
vdisplay) and it should be the same CRC as the full blue fb
v2: Fix a few things here and there (Damien)
Cc: Lei Liu lei.a@intel.com
Cc: Yi Sun yi@intel.com
Signed-off-by: Lei Liu lei.a@intel.com
Signed-off-by: Yi Sun yi@intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
lib/igt_kms.c | 21 +++--
lib/igt_kms.h | 4 ++
tests/kms_plane.c | 130 ++
3 files changed, 152 insertions(+), 3 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 82bdec5..34311c8 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -979,7 +979,8 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
*primary,
/* Primary planes can't be windowed when using a legacy commit */
igt_assert((primary-crtc_x == 0 primary-crtc_y == 0));
- if (!primary-fb_changed !primary-position_changed)
+ if (!primary-fb_changed !primary-position_changed
+ !primary-panning_changed)
return 0;
crtc_id = output-config.crtc-crtc_id;
@@ -996,13 +997,13 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
*primary,
igt_output_name(output),
pipe_name(output-config.pipe),
fb_id,
- 0, 0,
+ primary-pan_x, primary-pan_y,
mode-hdisplay, mode-vdisplay);
ret = drmModeSetCrtc(display-drm_fd,
crtc_id,
fb_id,
-0, 0, /* x, y */
+primary-pan_x, primary-pan_y,
output-id,
1,
mode);
@@ -1254,6 +1255,20 @@ void igt_plane_set_position(igt_plane_t *plane, int x,
int y)
plane-position_changed = true;
}
+void igt_plane_set_panning(igt_plane_t *plane, int x, int y)
+{
+ igt_pipe_t *pipe = plane-pipe;
+ igt_display_t *display = pipe-display;
+
+ LOG(display, %c.%d: plane_set_panning(%d,%d)\n, pipe_name(pipe-pipe),
+ plane-index, x, y);
+
+ plane-pan_x = x;
+ plane-pan_y = y;
+
+ plane-panning_changed = true;
+}
+
void igt_wait_for_vblank(int drm_fd, enum pipe pipe)
{
drmVBlank wait_vbl;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 95ba112..a079fc2 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -113,6 +113,7 @@ typedef struct {
unsigned int is_cursor: 1;
unsigned int fb_changed : 1;
unsigned int position_changed : 1;
+ unsigned int panning_changed : 1;
/*
* drm_plane can be NULL for primary and cursor planes (when not
* using the atomic modeset API)
@@ -121,6 +122,8 @@ typedef struct {
struct igt_fb *fb;
/* position within pipe_src_w x pipe_src_h */
int crtc_x, crtc_y;
+ /* panning offset within the fb */
+ unsigned int pan_x, pan_y;
} igt_plane_t;
struct igt_pipe {
@@ -170,6 +173,7 @@ igt_plane_t *igt_output_get_plane(igt_output_t *output,
enum igt_plane plane);
void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
void igt_plane_set_position(igt_plane_t *plane, int x, int y);
+void igt_plane_set_panning(igt_plane_t *plane, int x, int y);
void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index 45c4a77..7437641 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -45,7 +45,9 @@ typedef struct {
igt_pipe_crc_t *pipe_crc;
} data_t;
+static color_t red = { 1.0f, 0.0f, 0.0f };
static color_t green = { 0.0f, 1.0f, 0.0f };
+static color_t blue = { 0.0f, 0.0f, 1.0f };
/*
* Common code across all tests, acting on data_t
@@ -211,6 +213,124 @@ test_plane_position(data_t *data, enum pipe pipe, enum
igt_plane plane,
flags);
}
+/*
+ * Plane panning test.
+ * - We start by grabbing reference CRCs of a full red and a full blue fb
+ * being scanned out on the primary plane
+ * - Then we create a big fb, sized (2 * hdisplay, 2 * vdisplay) and:
+ * - fill the top left quarter with red
+ * - fill the bottom right quarter with blue
+ * - The TEST_PANNING_TOP_LEFT test makes sure that with panning at (0, 0)
+ * we do get the same CRC than the full red fb.
+ * - The TEST_PANNING_BOTTOM_RIGHT test makes sure that with panning at
+ *
Re: [Intel-gfx] [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type
Il 07/07/2014 19:54, Daniel Vetter ha scritto: On Mon, Jul 07, 2014 at 04:57:45PM +0200, Paolo Bonzini wrote: Il 07/07/2014 16:49, Daniel Vetter ha scritto: So the correct fix to forward intel gpus to guests is indeed to somehow fake the pch pci ids since the driver really needs them. Gross design, but that's how the hardware works. A way that could work for virtualization is this: if you find the card has a magic subsystem vendor id, fetch the subsystem device id and use _that_ as the PCH device id. Would that work for you? I guess for quemu it also depends upon what windows does since we can't change that. If we care about that part. Another consideration is supporting older kernels, if that's possible at all. Yes, but right now it's more important to get something that's not too gross for the future, for both Linux and Windows. Hacks for existing guests can be done separately, especially since they might differ between Linux (check ISA bridge) and Windows (check 1f.0). Paolo ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 01/44] drm/i915: Corrected 'file_priv' to 'file' in 'i915_driver_preclose()'
On Mon, Jun 30, 2014 at 02:03:18PM -0700, Jesse Barnes wrote:
On Thu, 26 Jun 2014 18:23:52 +0100
john.c.harri...@intel.com wrote:
From: John Harrison john.c.harri...@intel.com
The 'i915_driver_preclose()' function has a parameter called 'file_priv'.
However, this is misleading as the structure it points to is a 'drm_file'
not a
'drm_i915_file_private'. It should be named just 'file' to avoid confusion.
sob line is missing, but I've added that since we work for the same
company ;-) Please make sure you'll get these details right, checkpatch.pl
will help. Queued for -next, thanks for the patch.
-Daniel
---
drivers/gpu/drm/i915/i915_dma.c |6 +++---
drivers/gpu/drm/i915/i915_drv.h |6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index b9159ad..6cce55b 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1916,11 +1916,11 @@ void i915_driver_lastclose(struct drm_device * dev)
i915_dma_cleanup(dev);
}
-void i915_driver_preclose(struct drm_device * dev, struct drm_file
*file_priv)
+void i915_driver_preclose(struct drm_device *dev, struct drm_file *file)
{
mutex_lock(dev-struct_mutex);
- i915_gem_context_close(dev, file_priv);
- i915_gem_release(dev, file_priv);
+ i915_gem_context_close(dev, file);
+ i915_gem_release(dev, file);
mutex_unlock(dev-struct_mutex);
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index bea9ab40..7a96ca0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2044,12 +2044,12 @@ void i915_update_dri1_breadcrumb(struct drm_device
*dev);
extern void i915_kernel_lost_context(struct drm_device * dev);
extern int i915_driver_load(struct drm_device *, unsigned long flags);
extern int i915_driver_unload(struct drm_device *);
-extern int i915_driver_open(struct drm_device *dev, struct drm_file
*file_priv);
+extern int i915_driver_open(struct drm_device *dev, struct drm_file *file);
extern void i915_driver_lastclose(struct drm_device * dev);
extern void i915_driver_preclose(struct drm_device *dev,
-struct drm_file *file_priv);
+struct drm_file *file);
extern void i915_driver_postclose(struct drm_device *dev,
- struct drm_file *file_priv);
+ struct drm_file *file);
extern int i915_driver_device_is_agp(struct drm_device * dev);
#ifdef CONFIG_COMPAT
extern long i915_compat_ioctl(struct file *filp, unsigned int cmd,
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
--
Jesse Barnes, Intel Open Source Technology Center
___
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] [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type
On Mon, Jul 07, 2014 at 07:58:30PM +0200, Paolo Bonzini wrote: Il 07/07/2014 19:54, Daniel Vetter ha scritto: On Mon, Jul 07, 2014 at 04:57:45PM +0200, Paolo Bonzini wrote: Il 07/07/2014 16:49, Daniel Vetter ha scritto: So the correct fix to forward intel gpus to guests is indeed to somehow fake the pch pci ids since the driver really needs them. Gross design, but that's how the hardware works. A way that could work for virtualization is this: if you find the card has a magic subsystem vendor id, fetch the subsystem device id and use _that_ as the PCH device id. Would that work for you? I guess for quemu it also depends upon what windows does since we can't change that. If we care about that part. Another consideration is supporting older kernels, if that's possible at all. Yes, but right now it's more important to get something that's not too gross for the future, for both Linux and Windows. Hacks for existing guests can be done separately, especially since they might differ between Linux (check ISA bridge) and Windows (check 1f.0). Well old Linux also checked 1f.0, so kinda the same really. As long as 1f.0 is an isa bridge. Wrt Windows I don't really expect them to change this (they're probably more focuesed on the windows hypervisor or whatever). In the end if the approach is ok for quemu and isn't much worse than what we currently have I don't mind at all about the i915.ko code. I just want to avoid flip-flopping around on the hack du jour like we seem to do just now. -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] [RFC 03/44] drm/i915: Add extra add_request calls
On Mon, Jun 30, 2014 at 02:10:16PM -0700, Jesse Barnes wrote: On Thu, 26 Jun 2014 18:23:54 +0100 john.c.harri...@intel.com wrote: I think no_flush would be more in line with some of the other functions in the kernel. wo makes me think of write only. But it's not a big deal. I do wonder about the rules for when add_request is needed though, and I need to look later in the series for the usage. When I looked at it in relation to fences, it didn't seem to be a good fit since it looked like requests got freed when the active list was cleared, vs when they were actually consumed by some user. Yeah, wo_flush is highly confusing while no_flush is rather clear. There's also the question of how this all will interfere with execlists since those patches also have the need to keep track of stuff, but slightly different. I'll go through your rfc for some light reading but I think we should settle execlists first before proceeding with the schedule in earnest. -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] [RFC 02/44] drm/i915: Added getparam for native sync
On Thu, Jun 26, 2014 at 06:23:53PM +0100, john.c.harri...@intel.com wrote: From: John Harrison john.c.harri...@intel.com Validation tests need a run time mechanism for querying whether or not the driver supports the Android native sync facility. --- drivers/gpu/drm/i915/i915_dma.c |7 +++ include/uapi/drm/i915_drm.h |1 + 2 files changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 6cce55b..67f2918 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1022,6 +1022,13 @@ static int i915_getparam(struct drm_device *dev, void *data, case I915_PARAM_CMD_PARSER_VERSION: value = i915_cmd_parser_get_version(); break; + case I915_PARAM_HAS_NATIVE_SYNC: +#ifdef CONFIG_DRM_I915_SYNC + value = 1; +#else + value = 0; +#endif New userspace ABI (which this is) needs to come with open-source users. Also we do the announce new features to userspace patch generally last in a series to avoid unecessary test failures. Finally infrastructure only used by tests should be done in debugfs, which has more lax abi guarantees. And one more: syncpt support and the scheduler are orthogonal imo, and as part of proper syncpt support we also need to destage the android syncpt stuff first (since i915 can't depend upon stuff from drivers/staging). Thus far I have seen neglible efforts from Android people to make this happen :( -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] [RFC 07/44] drm/i915: Disable 'get seqno' workaround for VLV
On Wed, Jul 02, 2014 at 10:51:23AM -0700, Jesse Barnes wrote: On Thu, 26 Jun 2014 18:23:58 +0100 john.c.harri...@intel.com wrote: From: John Harrison john.c.harri...@intel.com There is a workaround for a hardware bug when reading the seqno from the status page. The bug does not exist on VLV however, the workaround was still being applied. --- drivers/gpu/drm/i915/intel_ringbuffer.c |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 279488a..bad5db0 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1960,7 +1960,10 @@ int intel_init_render_ring_buffer(struct drm_device *dev) ring-irq_put = gen6_ring_put_irq; } ring-irq_enable_mask = GT_RENDER_USER_INTERRUPT; - ring-get_seqno = gen6_ring_get_seqno; + if (IS_VALLEYVIEW(dev)) + ring-get_seqno = ring_get_seqno; + else + ring-get_seqno = gen6_ring_get_seqno; ring-set_seqno = ring_set_seqno; ring-semaphore.sync_to = gen6_ring_sync; ring-semaphore.signal = gen6_signal; Assuming this has been well tested: Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org I have my doubts ... the seqno race is fairly hard to reproduce really and needs some serious beating. Also highly timing dependent. My best guess is that Oscar's irq handling race fixes fixed the underlying bug on gen6+, so I think we should instead dare to rip out this w/a completely and see what happens. Doing this on gen6+ will at least give us serious amounts of test coverage. -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] [RFC 08/44] drm/i915: Added GPU scheduler config option
On Thu, Jun 26, 2014 at 06:23:59PM +0100, john.c.harri...@intel.com wrote: From: John Harrison john.c.harri...@intel.com Added a Kconfig option for enabling/disabling the GPU scheduler. --- drivers/gpu/drm/i915/Kconfig |8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 437e182..22a036b 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -81,3 +81,11 @@ config DRM_I915_UMS enable this only if you have ancient versions of the DDX drivers. If in doubt, say N. + +config DRM_I915_SCHEDULER + bool Enable GPU scheduler on Intel hardware + depends on DRM_I915 + default y + help + Choose this option to enable GPU task scheduling for improved + performance and efficiency. NACK. We ship one driver in one well tested config, everything else is a nightmare. There's very few exceptions (currently MMU_NOTIFIER and optional FBDEV support which have some really good reasons attached to them). And I'm still grumpy about the MMU_NOTIFIER one ;-) -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] [RFC 09/44] drm/i915: Start of GPU scheduler
On Thu, Jun 26, 2014 at 06:24:00PM +0100, john.c.harri...@intel.com wrote:
From: John Harrison john.c.harri...@intel.com
Created GPU scheduler source files with only a basic init function.
Same critique as for Oscar's execlist: Please don't order patches by
adding unused leave code and structures first, but start by wiring up
wired up (but mayb be still partially stubbed-out) code.
The aim is to make review of individual patches possible with as little
context as required - for otherwise (i.e. if you have to keep all the code
in mind till the end since only then it really gets plugged in) splitting
up the patches is a superficial exercise and doesn't really help the
reviewer.
/rant
-Daniel
---
drivers/gpu/drm/i915/Makefile |1 +
drivers/gpu/drm/i915/i915_drv.h |4 +++
drivers/gpu/drm/i915/i915_gem.c |3 ++
drivers/gpu/drm/i915/i915_scheduler.c | 59
+
drivers/gpu/drm/i915/i915_scheduler.h | 40 ++
5 files changed, 107 insertions(+)
create mode 100644 drivers/gpu/drm/i915/i915_scheduler.c
create mode 100644 drivers/gpu/drm/i915/i915_scheduler.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index cad1683..12817a8 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -11,6 +11,7 @@ i915-y := i915_drv.o \
i915_params.o \
i915_suspend.o \
i915_sysfs.o \
+ i915_scheduler.o \
intel_pm.o
i915-$(CONFIG_COMPAT) += i915_ioc32.o
i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 53f6fe5..6e592d3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1331,6 +1331,8 @@ struct intel_pipe_crc {
wait_queue_head_t wq;
};
+struct i915_scheduler;
+
struct drm_i915_private {
struct drm_device *dev;
struct kmem_cache *slab;
@@ -1540,6 +1542,8 @@ struct drm_i915_private {
struct i915_runtime_pm pm;
+ struct i915_scheduler *scheduler;
+
/* Old dri1 support infrastructure, beware the dragons ya fools entering
* here! */
struct i915_dri1_state dri1;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 898660c..b784eb2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -37,6 +37,7 @@
#include linux/swap.h
#include linux/pci.h
#include linux/dma-buf.h
+#include i915_scheduler.h
static void i915_gem_object_flush_gtt_write_domain(struct
drm_i915_gem_object *obj);
static void i915_gem_object_flush_cpu_write_domain(struct
drm_i915_gem_object *obj,
@@ -4669,6 +4670,8 @@ static int i915_gem_init_rings(struct drm_device *dev)
goto cleanup_vebox_ring;
}
+ i915_scheduler_init(dev);
+
ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
if (ret)
goto cleanup_bsd2_ring;
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c
b/drivers/gpu/drm/i915/i915_scheduler.c
new file mode 100644
index 000..9ec0225
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -0,0 +1,59 @@
+/*
+ * 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.
+ *
+ */
+
+#include i915_drv.h
+#include intel_drv.h
+#include i915_scheduler.h
+
+#ifdef CONFIG_DRM_I915_SCHEDULER
+
+int i915_scheduler_init(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct i915_scheduler *scheduler = dev_priv-scheduler;
+
+ if (scheduler)
+ return 0;
+
+ scheduler = kzalloc(sizeof(*scheduler), GFP_KERNEL);
+ if (!scheduler)
+ return -ENOMEM;
+
+ spin_lock_init(scheduler-lock);
+
+ scheduler-index =
Re: [Intel-gfx] [RFC 10/44] drm/i915: Prepare retire_requests to handle out-of-order seqnos
On Thu, Jun 26, 2014 at 06:24:01PM +0100, john.c.harri...@intel.com wrote:
From: John Harrison john.c.harri...@intel.com
A major point of the GPU scheduler is that it re-orders batch buffers after
they
have been submitted to the driver. Rather than attempting to re-assign seqno
values, it is much simpler to have each batch buffer keep its initially
assigned
number and modify the rest of the driver to cope with seqnos being returned
out
of order. In practice, very little code actually needs updating to cope.
One such place is the retire request handler. Rather than stopping as soon as
an
uncompleted seqno is found, it must now keep iterating through the requests in
case later seqnos have completed. There is also a problem with doing the free
of
the request before the move to inactive. Thus the requests are now moved to a
temporary list first, then the objects de-activated and finally the requests
on
the temporary list are freed.
I still hold that we should track requests, not seqno+ring pairs. At least
the plan with Maarten's fencing patches is to embedded the generic struct
fence into our i915_gem_request structure. And struct fence will also be
the kernel-internal represenation of a android native sync fence.
So splatter ring+seqno-request/fence lookups all over the place isn't a
good way forward. It's ok for bring up, but for merging we should do that
kind of large-scale refactoring upfront to reduce rebase churn. Oscar
knows how this works.
-Daniel
---
drivers/gpu/drm/i915/i915_gem.c | 60
+--
1 file changed, 32 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b784eb2..7e53446 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2602,7 +2602,10 @@ void i915_gem_reset(struct drm_device *dev)
void
i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
{
+ struct drm_i915_gem_object *obj, *obj_next;
+ struct drm_i915_gem_request *req, *req_next;
uint32_t seqno;
+ LIST_HEAD(deferred_request_free);
if (list_empty(ring-request_list))
return;
@@ -2611,43 +2614,35 @@ i915_gem_retire_requests_ring(struct intel_engine_cs
*ring)
seqno = ring-get_seqno(ring, true);
- /* Move any buffers on the active list that are no longer referenced
- * by the ringbuffer to the flushing/inactive lists as appropriate,
- * before we free the context associated with the requests.
+ /* Note that seqno values might be out of order due to rescheduling and
+ * pre-emption. Thus both lists must be processed in their entirety
+ * rather than stopping at the first 'non-passed' entry.
*/
- while (!list_empty(ring-active_list)) {
- struct drm_i915_gem_object *obj;
-
- obj = list_first_entry(ring-active_list,
- struct drm_i915_gem_object,
- ring_list);
-
- if (!i915_seqno_passed(seqno, obj-last_read_seqno))
- break;
- i915_gem_object_move_to_inactive(obj);
- }
-
-
- while (!list_empty(ring-request_list)) {
- struct drm_i915_gem_request *request;
-
- request = list_first_entry(ring-request_list,
-struct drm_i915_gem_request,
-list);
-
- if (!i915_seqno_passed(seqno, request-seqno))
- break;
+ list_for_each_entry_safe(req, req_next, ring-request_list, list) {
+ if (!i915_seqno_passed(seqno, req-seqno))
+ continue;
- trace_i915_gem_request_retire(ring, request-seqno);
+ trace_i915_gem_request_retire(ring, req-seqno);
/* We know the GPU must have read the request to have
* sent us the seqno + interrupt, so use the position
* of tail of the request to update the last known position
* of the GPU head.
*/
- ring-buffer-last_retired_head = request-tail;
+ ring-buffer-last_retired_head = req-tail;
- i915_gem_free_request(request);
+ list_move_tail(req-list, deferred_request_free);
+ }
+
+ /* Move any buffers on the active list that are no longer referenced
+ * by the ringbuffer to the flushing/inactive lists as appropriate,
+ * before we free the context associated with the requests.
+ */
+ list_for_each_entry_safe(obj, obj_next, ring-active_list, ring_list) {
+ if (!i915_seqno_passed(seqno, obj-last_read_seqno))
+ continue;
+
+ i915_gem_object_move_to_inactive(obj);
}
if (unlikely(ring-trace_irq_seqno
@@ -2656,6 +2651,15 @@
Re: [Intel-gfx] [RFC 14/44] drm/i915: Added getparam for GPU scheduler
On Wed, Jul 02, 2014 at 11:21:42AM -0700, Jesse Barnes wrote:
On Thu, 26 Jun 2014 18:24:05 +0100
john.c.harri...@intel.com wrote:
From: John Harrison john.c.harri...@intel.com
This is required by user land validation programs that need to know whether
the
scheduler is available for testing or not.
---
drivers/gpu/drm/i915/i915_dma.c |3 +++
include/uapi/drm/i915_drm.h |1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index 6c9ce82..1668316 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1035,6 +1035,9 @@ static int i915_getparam(struct drm_device *dev, void
*data,
value = 0;
#endif
break;
+ case I915_PARAM_HAS_GPU_SCHEDULER:
+ value = i915_scheduler_is_enabled(dev);
+ break;
default:
DRM_DEBUG(Unknown parameter %d\n, param-param);
return -EINVAL;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index bf54c78..de6f603 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -341,6 +341,7 @@ typedef struct drm_i915_irq_wait {
#define I915_PARAM_HAS_WT 27
#define I915_PARAM_CMD_PARSER_VERSION 28
#define I915_PARAM_HAS_NATIVE_SYNC 30
+#define I915_PARAM_HAS_GPU_SCHEDULER31
typedef struct drm_i915_getparam {
int param;
I guess we have plenty of getparam space available. But another option
would be for tests to check for a debugfs file that dumps scheduler
info instead, and save the get params for non-debug applications.
Yeah, pure testing interfaces should reside in debugfs - much less
stringent abi compatibility requirements for that stuff.
Also, I want to see these validation tests as igt patches.
-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] [RFC 15/44] drm/i915: Added deferred work handler for scheduler
On Thu, Jun 26, 2014 at 06:24:06PM +0100, john.c.harri...@intel.com wrote:
From: John Harrison john.c.harri...@intel.com
The scheduler needs to do interrupt triggered work that is too complex to do
in
the interrupt handler. Thus it requires a deferred work handler to process
this
work asynchronously.
---
drivers/gpu/drm/i915/i915_dma.c |3 +++
drivers/gpu/drm/i915/i915_drv.h | 10 ++
drivers/gpu/drm/i915/i915_gem.c | 27 +++
drivers/gpu/drm/i915/i915_scheduler.c |7 +++
drivers/gpu/drm/i915/i915_scheduler.h |1 +
5 files changed, 48 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 1668316..d1356f3 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1813,6 +1813,9 @@ int i915_driver_unload(struct drm_device *dev)
WARN_ON(unregister_oom_notifier(dev_priv-mm.oom_notifier));
unregister_shrinker(dev_priv-mm.shrinker);
+ /* Cancel the scheduler work handler, which should be idle now. */
+ cancel_work_sync(dev_priv-mm.scheduler_work);
+
io_mapping_free(dev_priv-gtt.mappable);
arch_phys_wc_del(dev_priv-gtt.mtrr);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0977653..fbafa68 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1075,6 +1075,16 @@ struct i915_gem_mm {
struct delayed_work idle_work;
/**
+ * New scheme is to get an interrupt after every work packet
+ * in order to allow the low latency scheduling of pending
+ * packets. The idea behind adding new packets to a pending
+ * queue rather than directly into the hardware ring buffer
+ * is to allow high priority packets to over take low priority
+ * ones.
+ */
+ struct work_struct scheduler_work;
Latency for work items isn't too awesome, and e.g. Oscar's execlist code
latches the next context right away from the irq handler. Why can't we do
something similar for the scheduler? Fishing the next item out of a
priority queue shouldn't be expensive ...
-Daniel
+
+ /**
* Are we in a non-interruptible section of code like
* modesetting?
*/
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fece5e7..57b24f0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2712,6 +2712,29 @@ i915_gem_idle_work_handler(struct work_struct *work)
intel_mark_idle(dev_priv-dev);
}
+#ifdef CONFIG_DRM_I915_SCHEDULER
+static void
+i915_gem_scheduler_work_handler(struct work_struct *work)
+{
+ struct intel_engine_cs *ring;
+ struct drm_i915_private *dev_priv;
+ struct drm_device *dev;
+ int i;
+
+ dev_priv = container_of(work, struct drm_i915_private,
mm.scheduler_work);
+ dev = dev_priv-dev;
+
+ mutex_lock(dev-struct_mutex);
+
+ /* Do stuff: */
+ for_each_ring(ring, dev_priv, i) {
+ i915_scheduler_remove(ring);
+ }
+
+ mutex_unlock(dev-struct_mutex);
+}
+#endif
+
/**
* Ensures that an object will eventually get non-busy by flushing any
required
* write domains, emitting any outstanding lazy request and retiring and
@@ -4916,6 +4939,10 @@ i915_gem_load(struct drm_device *dev)
i915_gem_retire_work_handler);
INIT_DELAYED_WORK(dev_priv-mm.idle_work,
i915_gem_idle_work_handler);
+#ifdef CONFIG_DRM_I915_SCHEDULER
+ INIT_WORK(dev_priv-mm.scheduler_work,
+ i915_gem_scheduler_work_handler);
+#endif
init_waitqueue_head(dev_priv-gpu_error.reset_queue);
/* On GEN3 we really need to make sure the ARB C3 LP bit is set */
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c
b/drivers/gpu/drm/i915/i915_scheduler.c
index 66a6568..37f8a98 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -58,6 +58,13 @@ int i915_scheduler_init(struct drm_device *dev)
return 0;
}
+int i915_scheduler_remove(struct intel_engine_cs *ring)
+{
+ /* Do stuff... */
+
+ return 0;
+}
+
bool i915_scheduler_is_seqno_in_flight(struct intel_engine_cs *ring,
uint32_t seqno, bool *completed)
{
diff --git a/drivers/gpu/drm/i915/i915_scheduler.h
b/drivers/gpu/drm/i915/i915_scheduler.h
index 95641f6..6b2cc51 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.h
+++ b/drivers/gpu/drm/i915/i915_scheduler.h
@@ -38,6 +38,7 @@ struct i915_scheduler {
uint32_tindex;
};
+int i915_scheduler_remove(struct intel_engine_cs *ring);
booli915_scheduler_is_seqno_in_flight(struct intel_engine_cs *ring,
uint32_t seqno, bool *completed);
--
1.7.9.5
Re: [Intel-gfx] [RFC 17/44] drm/i915: Prelude to splitting i915_gem_do_execbuffer in two
On Wed, Jul 02, 2014 at 11:34:23AM -0700, Jesse Barnes wrote: On Thu, 26 Jun 2014 18:24:08 +0100 john.c.harri...@intel.com wrote: From: John Harrison john.c.harri...@intel.com The scheduler decouples the submission of batch buffers to the driver with their submission to the hardware. This basically means splitting the execbuffer() function in half. This change rearranges some code ready for the split to occur. --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index ec274ef..fda9187 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -32,6 +32,7 @@ #include i915_trace.h #include intel_drv.h #include linux/dma_remapping.h +#include i915_scheduler.h #define __EXEC_OBJECT_HAS_PIN (131) #define __EXEC_OBJECT_HAS_FENCE (130) @@ -874,10 +875,7 @@ i915_gem_execbuffer_move_to_gpu(struct intel_engine_cs *ring, if (flush_domains I915_GEM_DOMAIN_GTT) wmb(); - /* Unconditionally invalidate gpu caches and ensure that we do flush -* any residual writes from the previous batch. -*/ - return intel_ring_invalidate_all_caches(ring); + return 0; } static bool @@ -1219,8 +1217,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, } } - intel_runtime_pm_get(dev_priv); - ret = i915_mutex_lock_interruptible(dev); if (ret) goto pre_mutex_err; @@ -1331,6 +1327,20 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, if (ret) goto err; + i915_gem_execbuffer_move_to_active(eb-vmas, ring); + + /* To be split into two functions here... */ + + intel_runtime_pm_get(dev_priv); + + /* Unconditionally invalidate gpu caches and ensure that we do flush +* any residual writes from the previous batch. +*/ + ret = intel_ring_invalidate_all_caches(ring); + if (ret) + goto err; + + /* Switch to the correct context for the batch */ ret = i915_switch_context(ring, ctx); if (ret) goto err; @@ -1381,7 +1391,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, trace_i915_gem_ring_dispatch(ring, intel_ring_get_seqno(ring), flags); - i915_gem_execbuffer_move_to_active(eb-vmas, ring); i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj); err: I'd like Chris to take a look too, but it looks safe afaict. Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org switch_context can fail with EINTR so we really can't move stuff to the active list before that point. Or we need to make sure that all the stuff between the old and new move_to_active callsite can't fail. Or we need to track this and tell userspace with an EIO and adjusted reset stats that something between our point of no return where the kernel committed to executing the batch failed. Or we need to unrol move_to_active (which is currently not really possible). -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] [RFC 44/44] drm/i915: Fake batch support for page flips
On Thu, Jun 26, 2014 at 06:24:35PM +0100, john.c.harri...@intel.com wrote:
From: John Harrison john.c.harri...@intel.com
Any commands written to the ring without the scheduler's knowledge can get
lost
during a pre-emption event. This checkin updates the page flip code to send
the
ring commands via the scheduler's 'fake batch' interface. Thus the page flip
is
kept safe from being clobbered.
Same comment as with the execlist series: Can't we just use mmio flips
instead? We could just restrict the scheduler to more recent platforms if
mmio flips aren't available on all platforms ...
-Daniel
---
drivers/gpu/drm/i915/intel_display.c | 84
--
1 file changed, 40 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index fa1ffbb..8bbc5d3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9099,8 +9099,8 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
uint32_t flags)
{
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- uint32_t plane_bit = 0;
- int len, ret;
+ uint32_t plane_bit = 0, sched_flags;
+ int ret;
switch (intel_crtc-plane) {
case PLANE_A:
@@ -9117,18 +9117,6 @@ static int intel_gen7_queue_flip(struct drm_device
*dev,
return -ENODEV;
}
- len = 4;
- if (ring-id == RCS) {
- len += 6;
- /*
- * On Gen 8, SRM is now taking an extra dword to accommodate
- * 48bits addresses, and we need a NOOP for the batch size to
- * stay even.
- */
- if (IS_GEN8(dev))
- len += 2;
- }
-
/*
* BSpec MI_DISPLAY_FLIP for IVB:
* The full packet must be contained within the same cache line.
@@ -9139,13 +9127,7 @@ static int intel_gen7_queue_flip(struct drm_device
*dev,
* then do the cacheline alignment, and finally emit the
* MI_DISPLAY_FLIP.
*/
- ret = intel_ring_cacheline_align(ring);
- if (ret)
- return ret;
-
- ret = intel_ring_begin(ring, len);
- if (ret)
- return ret;
+ sched_flags = i915_ebp_sf_cacheline_align;
/* Unmask the flip-done completion message. Note that the bspec says
that
* we should do this for both the BCS and RCS, and that we must not
unmask
@@ -9157,32 +9139,46 @@ static int intel_gen7_queue_flip(struct drm_device
*dev,
* to zero does lead to lockups within MI_DISPLAY_FLIP.
*/
if (ring-id == RCS) {
- intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
- intel_ring_emit(ring, DERRMR);
- intel_ring_emit(ring, ~(DERRMR_PIPEA_PRI_FLIP_DONE |
- DERRMR_PIPEB_PRI_FLIP_DONE |
- DERRMR_PIPEC_PRI_FLIP_DONE));
- if (IS_GEN8(dev))
- intel_ring_emit(ring, MI_STORE_REGISTER_MEM_GEN8(1) |
- MI_SRM_LRM_GLOBAL_GTT);
- else
- intel_ring_emit(ring, MI_STORE_REGISTER_MEM(1) |
- MI_SRM_LRM_GLOBAL_GTT);
- intel_ring_emit(ring, DERRMR);
- intel_ring_emit(ring, ring-scratch.gtt_offset + 256);
- if (IS_GEN8(dev)) {
- intel_ring_emit(ring, 0);
- intel_ring_emit(ring, MI_NOOP);
- }
- }
+ uint32_t cmds[] = {
+ MI_LOAD_REGISTER_IMM(1),
+ DERRMR,
+ ~(DERRMR_PIPEA_PRI_FLIP_DONE |
+ DERRMR_PIPEB_PRI_FLIP_DONE |
+ DERRMR_PIPEC_PRI_FLIP_DONE),
+ IS_GEN8(dev) ? (MI_STORE_REGISTER_MEM_GEN8(1) |
+ MI_SRM_LRM_GLOBAL_GTT) :
+(MI_STORE_REGISTER_MEM(1) |
+ MI_SRM_LRM_GLOBAL_GTT),
+ DERRMR,
+ ring-scratch.gtt_offset + 256,
+// if (IS_GEN8(dev)) {
+ 0,
+ MI_NOOP,
+// }
+ MI_DISPLAY_FLIP_I915 | plane_bit,
+ fb-pitches[0] | obj-tiling_mode,
+ intel_crtc-unpin_work-gtt_offset,
+ MI_NOOP
+ };
+ uint32_t len = sizeof(cmds) / sizeof(*cmds);
+
+ ret = i915_scheduler_queue_nonbatch(ring, cmds, len, obj, 1,
sched_flags);
+ } else {
+ uint32_t cmds[] = {
+ MI_DISPLAY_FLIP_I915 | plane_bit,
+ fb-pitches[0] | obj-tiling_mode,
+ intel_crtc-unpin_work-gtt_offset,
+
Re: [Intel-gfx] [PATCH] drm/i915: Fix some NUM_RING iterators
On Mon, Jun 30, 2014 at 11:27:25AM +, Mateo Lozano, Oscar wrote:
-
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
Of Ben Widawsky
Sent: Saturday, June 28, 2014 10:56 PM
To: Chris Wilson; Rodrigo Vivi; Widawsky, Benjamin; Intel GFX
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix some NUM_RING iterators
On Sat, Jun 28, 2014 at 08:28:55PM +0100, Chris Wilson wrote:
On Sat, Jun 28, 2014 at 08:26:15AM -0700, Ben Widawsky wrote:
On Sat, Jun 28, 2014 at 07:20:38AM +0100, Chris Wilson wrote:
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 86362de..6e5250d 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -848,7 +848,7 @@ static uint32_t
i915_error_generate_code(struct
drm_i915_private *dev_priv,
* synchronization commands which almost always appear
in
the
case
* strictly a client bug. Use instdone to
differentiate those
some.
*/
- for (i = 0; i I915_NUM_RINGS; i++) {
+ for (i = 0; i I915_ACTIVE_RINGS(dev_priv-dev); i++)
{
if (error-ring[i].hangcheck_action ==
HANGCHECK_HUNG) {
if (ring_id)
*ring_id = i;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index e72017b..67e2919 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -90,6 +90,8 @@ struct intel_engine_cs {
} id;
#define I915_NUM_RINGS 5
#define LAST_USER_RING (VECS + 1)
+#define I915_ACTIVE_RINGS(dev)
hweight8(INTEL_INFO(dev)-ring_mask)
What does the popcount of the mask have to do with the validity of
the arrays being iterated over in this patch?
-Chris
The popcount of the mask represents the number of rings available on
the specific SKU, as opposed to the total number of rings on any SKU
ever.
It is not always correct to iterate on all rings in the system.
Please note, the patch is incomplete. I have a couple of other,
perhaps more interesting, cases which I've missed.
You still iterate over holes in the ring mask, and the iteration here
is over a completely different array, not rings.
-Chris
For the holes, I mentioned that in the commit message of the yet to be
submitted v2; it's not really an issue in the way things are today.
When/if we add a new ring, it will be.
What you're asking for has already been submitted multiple times with
seemingly no traction. I do realize the fixes (with my v2) are due to bugs
introduced in patches I've not yet submitted, so I think for that reason,
it's
fair to drop this patch.
I'd rather the other patch get in (for_each_active_ring), but it's tied up
with
execlists atm, and I continue to think this is a useful way to iterate over
the
rings in error conditions and during reset.
I dropped that patch, since it received some resistance and I couldn´t
really justify it on the Execlists series anymore (on the latest
versions we don´t introduce new for i I915_NUM_RINGS). I imagine the
patch could be sent again as a standalone?
With Chris' patch to no longer tear down ring structures over reset/system
suspend we should be able to always use ring_for_each. If not that means
we still have some fun to look at.
In any case I'm always happy to merge such drive-by cleanup patches, no
need to have a big patch series to justify it. Well as long as it's indeed
a step forward, which occasionally is a contentions topic ...
-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] linux-next: Tree for Jun 19 (drm/i915)
On Monday, July 07, 2014 04:54:23 PM Daniel Vetter wrote: On Wed, Jun 25, 2014 at 01:01:36AM +0200, Rafael J. Wysocki wrote: On Tuesday, June 24, 2014 02:43:02 PM Jani Nikula wrote: On Thu, 19 Jun 2014, Randy Dunlap rdun...@infradead.org wrote: On 06/18/14 23:16, Stephen Rothwell wrote: Hi all, The powerpc allyesconfig is again broken more than usual. Changes since 20140618: on i386: CONFIG_ACPI is not enabled. CC drivers/gpu/drm/i915/i915_drv.o ../drivers/gpu/drm/i915/i915_drv.c: In function 'i915_drm_freeze': ../drivers/gpu/drm/i915/i915_drv.c:547:2: error: implicit declaration of function 'acpi_target_system_state' [-Werror=implicit-function-declaration] ../drivers/gpu/drm/i915/i915_drv.c:547:36: error: 'ACPI_STATE_S3' undeclared (first use in this function) ../drivers/gpu/drm/i915/i915_drv.c:547:36: note: each undeclared identifier is reported only once for each function it appears in CC net/dccp/qpolicy.o cc1: some warnings being treated as errors make[5]: *** [drivers/gpu/drm/i915/i915_drv.o] Error 1 Thanks for the report, we'll fix it. Can anyone explain why include/linux/acpi_bus.h has #ifdef CONFIG_ACPI_SLEEP and conditional build for a dummy inline version of acpi_target_system_state(), *but* that does not get included or used if CONFIG_ACPI=n? Additionally, the combination of CONFIG_ACPI=y and CONFIG_ACPI_SLEEP=n does not seem to work at all. These two things look like bugs to me. Most likely not tested thoruoughly enough. So we'll really have to sprinkle #ifdef CONFIG_ACPI all over, instead of neatly using the dummy versions that someone has gone through the trouble of adding? No, we don't have to. Back from my vacation and I didn't see a conclusion to this issue here. Rafael, have you fixed this in your acpi tree or do I need to do something in drm-intel? I was on vacation too. :-) Please have a look if i915 includes acpi/acpi_bus.h directly anywhere. If so, it should include linux/acpi.h instead. I'll fix up the rest in the ACPI tree. Rafael ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix VCS2's ring name.
On Tue, Jul 01, 2014 at 02:41:36AM -0700, Rodrigo Vivi wrote: It just fix a typo. v2: removing underscore to let this like all other ring names (Oscar) Cc: Oscar Mateo oscar.ma...@intel.com Reviewed-by (v1): Ben Widawsky benjamin.widaw...@intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Queued for -next, thanks for the patch. -Daniel --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 2faef26..22c2b9a 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2224,7 +2224,7 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev) return -EINVAL; } - ring-name = bds2_ring; + ring-name = bsd2 ring; ring-id = VCS2; ring-write_tail = ring_write_tail; -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.
Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
Ver3: moved SYS_RESTART check earlier, new name for pp_div.
Ver4: Minor issue changes
Ver5: Move registration of reboot notifier to edp_connector_init,
Added warning comment to handler about lack of PM notification.
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 42 ++
drivers/gpu/drm/i915/intel_drv.h |2 ++
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b5ec489..7204dee 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -336,6 +338,37 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to guarantee T12 timing
+ This function only applicable when panel PM state is not to be tracked */
+static int edp_notify_handler(struct notifier_block *this, unsigned long code,
+ void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+edp_notifier);
+ struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp_div;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if (!is_edp(intel_dp) || code != SYS_RESTART)
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ pp_div = I915_READ(pp_div_reg);
+ pp_div = PP_REFERENCE_DIVIDER_MASK;
+
+ /* 0x1F write to PP_DIV_REG sets max cycle delay */
+ I915_WRITE(pp_div_reg, pp_div | 0x1F);
+ I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+ msleep(intel_dp-panel_power_cycle_delay);
+ }
+
+ return 0;
+}
+
static bool edp_have_panel_power(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3785,6 +3818,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
drm_modeset_lock(dev-mode_config.connection_mutex, NULL);
edp_panel_vdd_off_sync(intel_dp);
drm_modeset_unlock(dev-mode_config.connection_mutex);
+ if (intel_dp-edp_notifier.notifier_call) {
+ unregister_reboot_notifier(intel_dp-edp_notifier);
+ intel_dp-edp_notifier.notifier_call = NULL;
+ }
}
kfree(intel_dig_port);
}
@@ -4262,6 +4299,11 @@ static bool intel_edp_init_connector(struct intel_dp
*intel_dp,
}
mutex_unlock(dev-mode_config.mutex);
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-edp_notifier.notifier_call = edp_notify_handler;
+ register_reboot_notifier(intel_dp-edp_notifier);
+ }
+
intel_panel_init(intel_connector-panel, fixed_mode, downclock_mode);
intel_panel_setup_backlight(connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5f7c7bd..87d1715 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -541,6 +541,8 @@ struct intel_dp {
unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+ struct notifier_block edp_notifier;
+
bool use_tps3;
struct intel_connector *attached_connector;
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Updating comments.
On Mon, Jun 30, 2014 at 06:16:50PM -0700, Ben Widawsky wrote: On Mon, Jun 30, 2014 at 09:51:11AM -0700, Rodrigo Vivi wrote: ring index calculation table was out of date after other rings were added, although the formula is flexible and scale when adding new rings. So this patch just update the comments and add a brief explanation why to use sync_seqno[ring index]. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/i915_gem.c | 2 ++ drivers/gpu/drm/i915/intel_ringbuffer.h | 8 +--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index f6d1238..e85c85c 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2842,6 +2842,8 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj, idx = intel_ring_sync_index(from, to); seqno = obj-last_read_seqno; + /* Optimization: Avoid semaphore sync when we are sure we already +* waited for an object with higher seqno */ if (seqno = from-semaphore.sync_seqno[idx]) return 0; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index e72017b..2e8b516 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -238,9 +238,11 @@ intel_ring_sync_index(struct intel_engine_cs *ring, int idx; /* -* cs - 0 = vcs, 1 = bcs -* vcs - 0 = bcs, 1 = cs, -* bcs - 0 = cs, 1 = vcs. +* rcs - 0 = vcs, 1 = bcs, 2 = vecs, 3 = vcs2; +* vcs - 0 = bcs, 1 = vecs, 2 = vcs2, 3 = rcs; +* bcs - 0 = vecs, 1 = vcs2. 2 = rcs, 3 = vcs; +* vecs - 0 = vcs2, 1 = rcs, 2 = vcs, 3 = bcs; +* vcs2 - 0 = rcs, 1 = vcs, 2 = bcs, 3 = vecs; */ I'd be a favor of dropping this table, and instead explaining the goal of the math (to save the dword) tbh I don't mind either way ... idx = (other - ring) - 1; I'm guessing this hunk is from your private branch? Applied here without fuzz ... In any event, the topmost comment is a nice addition: Indeed. Reviewed-by: Ben Widawsky b...@bwidawsk.net Queued for -next, thanks for the patch. -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] linux-next: Tree for Jun 19 (drm/i915)
On Mon, Jul 07, 2014 at 10:01:27PM +0200, Rafael J. Wysocki wrote: On Monday, July 07, 2014 04:54:23 PM Daniel Vetter wrote: On Wed, Jun 25, 2014 at 01:01:36AM +0200, Rafael J. Wysocki wrote: On Tuesday, June 24, 2014 02:43:02 PM Jani Nikula wrote: On Thu, 19 Jun 2014, Randy Dunlap rdun...@infradead.org wrote: On 06/18/14 23:16, Stephen Rothwell wrote: Hi all, The powerpc allyesconfig is again broken more than usual. Changes since 20140618: on i386: CONFIG_ACPI is not enabled. CC drivers/gpu/drm/i915/i915_drv.o ../drivers/gpu/drm/i915/i915_drv.c: In function 'i915_drm_freeze': ../drivers/gpu/drm/i915/i915_drv.c:547:2: error: implicit declaration of function 'acpi_target_system_state' [-Werror=implicit-function-declaration] ../drivers/gpu/drm/i915/i915_drv.c:547:36: error: 'ACPI_STATE_S3' undeclared (first use in this function) ../drivers/gpu/drm/i915/i915_drv.c:547:36: note: each undeclared identifier is reported only once for each function it appears in CC net/dccp/qpolicy.o cc1: some warnings being treated as errors make[5]: *** [drivers/gpu/drm/i915/i915_drv.o] Error 1 Thanks for the report, we'll fix it. Can anyone explain why include/linux/acpi_bus.h has #ifdef CONFIG_ACPI_SLEEP and conditional build for a dummy inline version of acpi_target_system_state(), *but* that does not get included or used if CONFIG_ACPI=n? Additionally, the combination of CONFIG_ACPI=y and CONFIG_ACPI_SLEEP=n does not seem to work at all. These two things look like bugs to me. Most likely not tested thoruoughly enough. So we'll really have to sprinkle #ifdef CONFIG_ACPI all over, instead of neatly using the dummy versions that someone has gone through the trouble of adding? No, we don't have to. Back from my vacation and I didn't see a conclusion to this issue here. Rafael, have you fixed this in your acpi tree or do I need to do something in drm-intel? I was on vacation too. :-) Please have a look if i915 includes acpi/acpi_bus.h directly anywhere. If so, it should include linux/acpi.h instead. I'll fix up the rest in the ACPI tree. We seem to only use linux/acpi.h and acpi/(video|button).h, at least according to a grep include.*acpi. So I think we're good in i915 land. Thanks for taking care of this. -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 10/10] drm/i915: Enable semaphores on BDW
On Mon, Jun 30, 2014 at 05:14:50PM -0700, Ben Widawsky wrote: On Mon, Jun 30, 2014 at 09:53:44AM -0700, Rodrigo Vivi wrote: Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Reviewed-by: Ben Widawsky b...@bwidawsk.net Pulled in entire series. Yay! -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 3/3] drm/i915: Trace point callbacks for validation
On Tue, Jul 01, 2014 at 05:24:23PM +0100, daniele.ceraolospu...@intel.com wrote: From: Daniele Ceraolo Spurio daniele.ceraolospu...@intel.com These callbacks can be assigned to specific functions inside an external validation kernel module. This module can then extract run-time information to make sure everything is working as expected. Specifically, these two callbacks extract information about full PPGTT and batch dispatch. Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospu...@intel.com --- drivers/gpu/drm/i915/i915_gem_context.c| 3 +++ drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 +++ drivers/gpu/drm/i915/i915_trace.h | 22 ++ 3 files changed, 28 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 99f7022..120a319 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -90,6 +90,9 @@ #include i915_drv.h #include i915_trace.h +i915_ppgtt_validation_callback_t ppgtt_validation_callback = NULL; +EXPORT_SYMBOL_GPL(ppgtt_validation_callback); + /* This is a HW constraint. The value below is the largest known requirement * I've seen in a spec to date, and that was a workaround for a non-shipping * part. It should be safe to decrease this, but it's more future proof as is. diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 0b2b76e..7feb977 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -33,6 +33,9 @@ #include intel_drv.h #include linux/dma_remapping.h +i915_gem_ring_dispatch_callback_t i915_gem_ring_dispatch_validation_callback = NULL; +EXPORT_SYMBOL_GPL(i915_gem_ring_dispatch_validation_callback); + #define __EXEC_OBJECT_HAS_PIN (131) #define __EXEC_OBJECT_HAS_FENCE (130) #define __EXEC_OBJECT_NEEDS_BIAS (128) diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h index 4e73e3a..98c6f47 100644 --- a/drivers/gpu/drm/i915/i915_trace.h +++ b/drivers/gpu/drm/i915/i915_trace.h @@ -15,6 +15,18 @@ #define TRACE_SYSTEM_STRING __stringify(TRACE_SYSTEM) #define TRACE_INCLUDE_FILE i915_trace +/* validation callbacks */ + +typedef void (*i915_ppgtt_validation_callback_t)(unsigned int action_code, + struct i915_hw_ppgtt *ppgtt); +extern i915_ppgtt_validation_callback_t ppgtt_validation_callback; + +typedef void (*i915_gem_ring_dispatch_callback_t)(struct intel_engine_cs *ring, + u32 seqno, + u32 flags, + struct intel_context *ctx); +extern i915_gem_ring_dispatch_callback_t i915_gem_ring_dispatch_validation_callback; + /* pipe updates */ TRACE_EVENT(i915_pipe_update_start, @@ -371,6 +383,10 @@ TRACE_EVENT(i915_gem_ring_dispatch, __entry-flags = flags; __entry-vm = ctx-vm; i915_trace_irq_get(ring, seqno); + +if (i915_gem_ring_dispatch_validation_callback) + i915_gem_ring_dispatch_validation_callback(ring, + seqno, flags, ctx); Nack on two grounds: - I already get flames for the irq_get in here from the tracepoint maintainer - he really doesn't like us adding randome stuff with side-effects here. - If we want extended validation interfaces we should upstream those, not just the hooks. Cheers, Daniel ), TP_printk(dev=%u, ring=%u, seqno=%u, flags=%x, vm=%p, @@ -607,6 +623,9 @@ TRACE_EVENT(create_vm_for_ctx, __entry-vm = ppgtt-base; __entry-dev = ppgtt-base.dev-primary-index; __entry-pid = (unsigned int)task_pid_nr(current); + + if (ppgtt_validation_callback) + ppgtt_validation_callback(0, ppgtt); ), TP_printk(dev=%u, task_pid=%u, vm=%p, @@ -627,6 +646,9 @@ TRACE_EVENT(ppgtt_release, TP_fast_assign( __entry-vm = ppgtt-base; __entry-dev = ppgtt-base.dev-primary-index; + + if (ppgtt_validation_callback) + ppgtt_validation_callback(1, ppgtt); ), TP_printk(dev=%u, vm=%p, __entry-dev, __entry-vm) -- 1.8.5.2 ___ 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
Re: [Intel-gfx] pin OABUFFER to GGTT
On Tue, Jul 01, 2014 at 08:54:27PM +0100, Chris Wilson wrote: On Tue, Jul 01, 2014 at 05:16:30PM +, Mateo Lozano, Oscar wrote: The issue is they need: A) A buffer object. B) Bound to GGTT. C) That userspace knows the GGTT offset of, so that they can program OABUFFER with it. D) That userspace can map so that they can read the reported counters. They used to create a bo, call bo_pin on it, use args-offset to program OABUFFER (via MI_LOAD_REGISTER_IMM, I imagine), map it and read the counter values. They cannot do this anymore. The answer might be that all of this needs to be done by the kernel itself, but then we need to provide an interface to userspace... Yes. If you need to pin a buffer for a register, then it needs to be handled by the kernel. Especially one that provides information about other users. Short-circuiting the entire discussion here. Afaik there's two OA modes: - inline with the batch with MI_REPORT_PERF - global with the ringbuffer setup with the OABUFFER registers The later should indeed be fully controlled by the kernel as Chris suggested and exposed as an off-cpu performance monitoring unit through the perf subsystem. Chris has rfc patches floating somewhere to do this for other gpu perf data. One fun thing here is the coordination between these two OA modes since iirc they both use the same setup registers for the performance counter configuration. No idea yet how to solve this. But really userspace shouldn't program ggtt offset, not even debug/performance measuring tools. -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] WAs in init_clock_gating?
On Tue, Jul 01, 2014 at 04:51:07PM +, Mateo Lozano, Oscar wrote: Is there any reason why the WAs are applied in *_init_clock_gating? We are finding that some of them are lost during reset, and also the default context ends up with wrong values because the render context is restored saved before we get to gen8_init_clok_gating (at least with Execlists, I´m not sure this happens with MI_SET_CONTEXT because the context won´t be saved until the next switch). It's a historical accident since _very_ old hw only needed a bit of frobbing of the display clock gating bits. I believe this have been brought to the mailing list a couple of times, like: drm/i916: Init chv workarounds at render ring init My bsw is an unhappy camper if we delay the workaround init until init_clock_gating(). Move a bunch of it to the render ring init. FIXME: need to do this for all platforms since some of the registers also get clobbered at reset. Just need to figure out which registers those actually are. This patch is based on a slightly educated guess, but verifying on actual hw would be a good idea. Also should maybe move the init_clock_gating earlier too since we set up a bunch of clock gating stuff there that might be important for a properly working GT. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com And also: http://lists.freedesktop.org/archives/intel-gfx/2013-November/036482.html My concerns still apply. We need to move all work-arounds to the right places (a bunch of them also might need to get moved into the runtime pm code ...), and then we also need some test to make sure this all works. Since maintaining the full list of all w/a bits is currently out of the question (our code is too unstructured for this) I think we should have a per-platform list of w/a relevant registers + maybe bitmasks with stuff to ignore (e.g. the ring registers where the ring base addr might differ). Then the test would grab snapshots before after all the following operations and complain loud if anything changes: - gpu hangs (on all rings as prep for per-engine reset) - runtime pm (actually all the different power wells really, e.g. lpsp mode) - system suspend/resum - module reload That should at least catch all the bugs we've seen thus far. If it later on turns out that's not good enough we can go more fancy, but for now I prefer something simpler ... Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Revert drm/i915: Reject the pin ioctl on gen6+
On Thu, Jul 03, 2014 at 08:12:35AM +0100, Damien Lespiau wrote: This reverts commit 02f6bcccf7c324115747aae2f0addd6af5d321cd. The OA buffer can contain global data (in particular, not linked to a context or a single batch execution) about GPU events (eg. hw context switches, rc6 transitions, frequency changes, ...) and needs to be mapped to GGTT. The pin ioctl provided a way to do that. Admittedly, this change broke what seems to be a valid use case of pinning a buffer in GGTT, even when PPGTT is used (which is the reason invoked in the commit message). Global OA buffers should be handled by the kernel and exposed through perf, imo. I think I'll go lalala on this a bit longer ... -Daniel Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Tomasz Madajczak tomasz.madajc...@intel.com Cc: Adam Rutkowski adam.j.rutkow...@intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/i915_gem.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 1794a04..8019809 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4143,9 +4143,6 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data, struct drm_i915_gem_object *obj; int ret; - if (INTEL_INFO(dev)-gen = 6) - return -ENODEV; - ret = i915_mutex_lock_interruptible(dev); if (ret) return ret; -- 1.8.3.1 -- 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 i-g-t 1/2] pipe_crc: Warn if the CRC values is 0xffffffff
On Mon, Jul 07, 2014 at 06:03:29PM +0100, Damien Lespiau wrote:
This is what we read when the CRC logic in in a powered down well. We
really don't want that to happen.
In theory, it's possible 0x to be a valid CRC value, so I don't
assert here.
I think we also should go through all tests and make sure that we change
all color channels of for crc based tests to use the full crc space. That
aside, ack on both patches.
-Daniel
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
lib/igt_debugfs.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 2f655a1..5c4bbc5 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -199,9 +199,14 @@ bool igt_crc_is_null(igt_crc_t *crc)
{
int i;
- for (i = 0; i crc-n_words; i++)
+ for (i = 0; i crc-n_words; i++) {
+ if (crc-crc[i] == 0x)
+ igt_warn(Suspicious CRC: it looks like the CRC
+ read back was from a register in a powered
+ down well\n);
if (crc-crc[i])
return false;
+ }
return true;
}
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 1/2] kms_plane: Add panning test for primary plane
On Mon, Jul 07, 2014 at 06:04:45PM +0100, Damien Lespiau wrote:
From: Yi Sun yi@intel.com
Get CRCs of a full red and a full blue surface as reference.
Create a big framebuffer that is twice width and twice height as the
current display mode.
The interesting stuff happens for framebuffers with offset 4k (in
pixels iirc). Care to add such a subtest too on platforms that support
large enough strides (i.e. gen4+)?
-Daniel
Fill the top left quarter with red, bottom right quarter with blue
Check the scanned out image with the CRTC at position (0, 0) of the
framebuffer and it should be the same CRC as the full red fb
Check the scanned out image with the CRTC at position (hdisplay,
vdisplay) and it should be the same CRC as the full blue fb
v2: Fix a few things here and there (Damien)
Cc: Lei Liu lei.a@intel.com
Cc: Yi Sun yi@intel.com
Signed-off-by: Lei Liu lei.a@intel.com
Signed-off-by: Yi Sun yi@intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
lib/igt_kms.c | 21 +++--
lib/igt_kms.h | 4 ++
tests/kms_plane.c | 130
++
3 files changed, 152 insertions(+), 3 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 82bdec5..34311c8 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -979,7 +979,8 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
*primary,
/* Primary planes can't be windowed when using a legacy commit */
igt_assert((primary-crtc_x == 0 primary-crtc_y == 0));
- if (!primary-fb_changed !primary-position_changed)
+ if (!primary-fb_changed !primary-position_changed
+ !primary-panning_changed)
return 0;
crtc_id = output-config.crtc-crtc_id;
@@ -996,13 +997,13 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
*primary,
igt_output_name(output),
pipe_name(output-config.pipe),
fb_id,
- 0, 0,
+ primary-pan_x, primary-pan_y,
mode-hdisplay, mode-vdisplay);
ret = drmModeSetCrtc(display-drm_fd,
crtc_id,
fb_id,
- 0, 0, /* x, y */
+ primary-pan_x, primary-pan_y,
output-id,
1,
mode);
@@ -1254,6 +1255,20 @@ void igt_plane_set_position(igt_plane_t *plane, int x,
int y)
plane-position_changed = true;
}
+void igt_plane_set_panning(igt_plane_t *plane, int x, int y)
+{
+ igt_pipe_t *pipe = plane-pipe;
+ igt_display_t *display = pipe-display;
+
+ LOG(display, %c.%d: plane_set_panning(%d,%d)\n, pipe_name(pipe-pipe),
+ plane-index, x, y);
+
+ plane-pan_x = x;
+ plane-pan_y = y;
+
+ plane-panning_changed = true;
+}
+
void igt_wait_for_vblank(int drm_fd, enum pipe pipe)
{
drmVBlank wait_vbl;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 95ba112..a079fc2 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -113,6 +113,7 @@ typedef struct {
unsigned int is_cursor: 1;
unsigned int fb_changed : 1;
unsigned int position_changed : 1;
+ unsigned int panning_changed : 1;
/*
* drm_plane can be NULL for primary and cursor planes (when not
* using the atomic modeset API)
@@ -121,6 +122,8 @@ typedef struct {
struct igt_fb *fb;
/* position within pipe_src_w x pipe_src_h */
int crtc_x, crtc_y;
+ /* panning offset within the fb */
+ unsigned int pan_x, pan_y;
} igt_plane_t;
struct igt_pipe {
@@ -170,6 +173,7 @@ igt_plane_t *igt_output_get_plane(igt_output_t *output,
enum igt_plane plane);
void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
void igt_plane_set_position(igt_plane_t *plane, int x, int y);
+void igt_plane_set_panning(igt_plane_t *plane, int x, int y);
void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index 45c4a77..7437641 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -45,7 +45,9 @@ typedef struct {
igt_pipe_crc_t *pipe_crc;
} data_t;
+static color_t red = { 1.0f, 0.0f, 0.0f };
static color_t green = { 0.0f, 1.0f, 0.0f };
+static color_t blue = { 0.0f, 0.0f, 1.0f };
/*
* Common code across all tests, acting on data_t
@@ -211,6 +213,124 @@ test_plane_position(data_t *data, enum pipe pipe, enum
igt_plane plane,
flags);
}
+/*
+ * Plane panning test.
+ * - We start by grabbing reference CRCs of a full red and a full blue fb
+ * being scanned out on the primary plane
+ * - Then we create a big fb, sized (2 * hdisplay, 2 * vdisplay) and:
+ * - fill the top
Re: [Intel-gfx] WAs in init_clock_gating?
On Mon, 7 Jul 2014 22:50:08 +0200 Daniel Vetter dan...@ffwll.ch wrote: On Tue, Jul 01, 2014 at 04:51:07PM +, Mateo Lozano, Oscar wrote: Is there any reason why the WAs are applied in *_init_clock_gating? We are finding that some of them are lost during reset, and also the default context ends up with wrong values because the render context is restored saved before we get to gen8_init_clok_gating (at least with Execlists, I´m not sure this happens with MI_SET_CONTEXT because the context won´t be saved until the next switch). It's a historical accident since _very_ old hw only needed a bit of frobbing of the display clock gating bits. I believe this have been brought to the mailing list a couple of times, like: drm/i916: Init chv workarounds at render ring init My bsw is an unhappy camper if we delay the workaround init until init_clock_gating(). Move a bunch of it to the render ring init. FIXME: need to do this for all platforms since some of the registers also get clobbered at reset. Just need to figure out which registers those actually are. This patch is based on a slightly educated guess, but verifying on actual hw would be a good idea. Also should maybe move the init_clock_gating earlier too since we set up a bunch of clock gating stuff there that might be important for a properly working GT. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com And also: http://lists.freedesktop.org/archives/intel-gfx/2013-November/036482.html My concerns still apply. We need to move all work-arounds to the right places (a bunch of them also might need to get moved into the runtime pm code ...), and then we also need some test to make sure this all works. Since maintaining the full list of all w/a bits is currently out of the question (our code is too unstructured for this) I think we should have a per-platform list of w/a relevant registers + maybe bitmasks with stuff to ignore (e.g. the ring registers where the ring base addr might differ). I don't think it's unreasonable to use a macro that checks a global list for whether to apply a given WA. They'll be scattered all over, but at least it'll be easy to see: 1) whether we implement a given workaround and 2) which platforms steppings it applies to based on the table. -- Jesse Barnes, 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: Revert drm/i915: Reject the pin ioctl on gen6+
On Mon, 7 Jul 2014 23:04:55 +0200 Daniel Vetter dan...@ffwll.ch wrote: On Thu, Jul 03, 2014 at 08:12:35AM +0100, Damien Lespiau wrote: This reverts commit 02f6bcccf7c324115747aae2f0addd6af5d321cd. The OA buffer can contain global data (in particular, not linked to a context or a single batch execution) about GPU events (eg. hw context switches, rc6 transitions, frequency changes, ...) and needs to be mapped to GGTT. The pin ioctl provided a way to do that. Admittedly, this change broke what seems to be a valid use case of pinning a buffer in GGTT, even when PPGTT is used (which is the reason invoked in the commit message). Global OA buffers should be handled by the kernel and exposed through perf, imo. I think I'll go lalala on this a bit longer ... Why? Because allowing the pin ioctl as root is such a problem? You need to come up with an alternative proposal and we need to get it implemented in some reasonable amount of time if we're not going to just do the simple thing that's already been shown to work... IOW don't plug your ears and say lalala for too long. -- Jesse Barnes, 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 05/10] drm/i915: Implement MI decode for gen8
On Mon, Jun 30, 2014 at 09:53:39AM -0700, Rodrigo Vivi wrote:
Ipehr just carries Dword 0 and on Gen 8, offsets are located
on Dword 2 and 3 of MI_SEMAPHORE_WAIT.
This implementation was based on Ben's work and on Ville's suggestion for Ben
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 42
++---
1 file changed, 23 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 0217a41..66e2481 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2784,12 +2784,7 @@ static bool
ipehr_is_semaphore_wait(struct drm_device *dev, u32 ipehr)
{
if (INTEL_INFO(dev)-gen = 8) {
- /*
- * FIXME: gen8 semaphore support - currently we don't emit
- * semaphores on bdw anyway, but this needs to be addressed when
- * we merge that code.
- */
- return false;
+ return (ipehr 23) == 0x1c;
} else {
ipehr = ~MI_SEMAPHORE_SYNC_MASK;
return ipehr == (MI_SEMAPHORE_MBOX | MI_SEMAPHORE_COMPARE |
@@ -2798,19 +2793,20 @@ ipehr_is_semaphore_wait(struct drm_device *dev, u32
ipehr)
}
static struct intel_engine_cs *
-semaphore_wait_to_signaller_ring(struct intel_engine_cs *ring, u32 ipehr)
+semaphore_wait_to_signaller_ring(struct intel_engine_cs *ring, u32 ipehr,
u64 offset)
{
struct drm_i915_private *dev_priv = ring-dev-dev_private;
struct intel_engine_cs *signaller;
int i;
if (INTEL_INFO(dev_priv-dev)-gen = 8) {
- /*
- * FIXME: gen8 semaphore support - currently we don't emit
- * semaphores on bdw anyway, but this needs to be addressed when
- * we merge that code.
- */
- return NULL;
+ for_each_ring(signaller, dev_priv, i) {
+ if (ring == signaller)
+ continue;
+
+ if (offset ==
signaller-semaphore.signal_ggtt[ring-id])
+ return signaller;
+ }
} else {
u32 sync_bits = ipehr MI_SEMAPHORE_SYNC_MASK;
@@ -2823,8 +2819,8 @@ semaphore_wait_to_signaller_ring(struct intel_engine_cs
*ring, u32 ipehr)
}
}
- DRM_ERROR(No signaller ring found for ring %i, ipehr 0x%08x\n,
- ring-id, ipehr);
+ DRM_ERROR(No signaller ring found for ring %i, ipehr 0x%08x, offset
0x%0%016llx\n,
The format string has a spurious 0% here. I've dropped it.
-Daniel
+ ring-id, ipehr, offset);
return NULL;
}
@@ -2834,7 +2830,8 @@ semaphore_waits_for(struct intel_engine_cs *ring, u32
*seqno)
{
struct drm_i915_private *dev_priv = ring-dev-dev_private;
u32 cmd, ipehr, head;
- int i;
+ u64 offset = 0;
+ int i, backwards;
ipehr = I915_READ(RING_IPEHR(ring-mmio_base));
if (!ipehr_is_semaphore_wait(ring-dev, ipehr))
@@ -2843,13 +2840,15 @@ semaphore_waits_for(struct intel_engine_cs *ring, u32
*seqno)
/*
* HEAD is likely pointing to the dword after the actual command,
* so scan backwards until we find the MBOX. But limit it to just 3
- * dwords. Note that we don't care about ACTHD here since that might
+ * or 4 dwords depending on the semaphore wait command size.
+ * Note that we don't care about ACTHD here since that might
* point at at batch, and semaphores are always emitted into the
* ringbuffer itself.
*/
head = I915_READ_HEAD(ring) HEAD_ADDR;
+ backwards = (INTEL_INFO(ring-dev)-gen = 8) ? 5 : 4;
- for (i = 4; i; --i) {
+ for (i = backwards; i; --i) {
/*
* Be paranoid and presume the hw has gone off into the wild -
* our ring is smaller than what the hardware (and hence
@@ -2869,7 +2868,12 @@ semaphore_waits_for(struct intel_engine_cs *ring, u32
*seqno)
return NULL;
*seqno = ioread32(ring-buffer-virtual_start + head + 4) + 1;
- return semaphore_wait_to_signaller_ring(ring, ipehr);
+ if (INTEL_INFO(ring-dev)-gen = 8) {
+ offset = ioread32(ring-buffer-virtual_start + head + 12);
+ offset = 32;
+ offset = ioread32(ring-buffer-virtual_start + head + 8);
+ }
+ return semaphore_wait_to_signaller_ring(ring, ipehr, offset);
}
static int semaphore_passed(struct intel_engine_cs *ring)
--
1.9.3
___
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 -
Re: [Intel-gfx] [PATCH i-g-t] kms_universal_plane: Don't assert outside of fixtures/subtests
On Fri, Jul 04, 2014 at 12:01:24PM +0100, Damien Lespiau wrote:
Doing otherwise breaks listing the subtests. The test was throwing an
error out when universal planes were disabled as well because of that.
Cc: Matt Roper matthew.d.ro...@intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
tests/kms_universal_plane.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/kms_universal_plane.c b/tests/kms_universal_plane.c
index fe0fde3..c64cdbb 100644
--- a/tests/kms_universal_plane.c
+++ b/tests/kms_universal_plane.c
@@ -133,6 +133,7 @@ functional_test_pipe(data_t *data, enum pipe pipe,
igt_output_t *output)
int num_primary = 0, num_cursor = 0;
int i;
+ igt_assert(data-display.has_universal_planes);
Shouldn't this be an igt_require instead? In any case moving this into
subtests is correct.
-Daniel
igt_skip_on(pipe = display-n_pipes);
fprintf(stdout, Testing connector %s using pipe %c\n,
@@ -535,8 +536,6 @@ run_tests_for_pipe(data_t *data, enum pipe pipe)
{
igt_output_t *output;
- igt_assert(data-display.has_universal_planes);
-
igt_subtest_f(universal-plane-pipe-%c-functional, pipe_name(pipe))
for_each_connected_output(data-display, output)
functional_test_pipe(data, pipe, output);
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/i915: don't write powered down IRQ registers on Gen 8
On Fri, Jul 04, 2014 at 11:50:29AM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com If we enable unclaimed register reporting on Gen 8, we will discover that the IRQ registers for pipes B and C are also on the power well, so writes to them when the power well is disabled result in unclaimed register errors. Also, hsw_power_well_post_enable() already takes care of re-enabling them once the power well is enabled. Testcase: igt/pm_rpm/rte Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Hm, shouldn't we split this into only setting up pipe A here and the pipe B stuff once we fire up the power well? I just want to avoid duplicating logic all over the place ... -Daniel --- drivers/gpu/drm/i915/i915_irq.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 1c1ec22..2e116e9d 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3193,7 +3193,9 @@ static void gen8_irq_reset(struct drm_device *dev) gen8_gt_irq_reset(dev_priv); for_each_pipe(pipe) - GEN8_IRQ_RESET_NDX(DE_PIPE, pipe); + if (intel_display_power_enabled(dev_priv, + POWER_DOMAIN_PIPE(pipe))) + GEN8_IRQ_RESET_NDX(DE_PIPE, pipe); GEN5_IRQ_RESET(GEN8_DE_PORT_); GEN5_IRQ_RESET(GEN8_DE_MISC_); @@ -3526,8 +3528,11 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv) dev_priv-de_irq_mask[PIPE_C] = ~de_pipe_masked; for_each_pipe(pipe) - GEN8_IRQ_INIT_NDX(DE_PIPE, pipe, dev_priv-de_irq_mask[pipe], - de_pipe_enables); + if (intel_display_power_enabled(dev_priv, + POWER_DOMAIN_PIPE(pipe))) + GEN8_IRQ_INIT_NDX(DE_PIPE, pipe, + dev_priv-de_irq_mask[pipe], + de_pipe_enables); GEN5_IRQ_INIT(GEN8_DE_PORT_, ~GEN8_AUX_CHANNEL_A, GEN8_AUX_CHANNEL_A); } -- 2.0.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] how to build intel-gpu-tools without cairo
I will try to disable the cairo tests on esx. Thanks Ying -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Monday, July 07, 2014 1:53 PM To: Liu, Ying2 Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] how to build intel-gpu-tools without cairo On Thu, Jul 03, 2014 at 09:47:13PM +, Liu, Ying2 wrote: Damien, We run intel-gpu-tool in VMware esx console. We didn't port display part of intel gpu driver to esx, so we don't need any display tests at all. If you could provide us a solution to run intel gpu tools without cairo, that would be great. I guess do the same as the android guys and disable the cairo tests on esx. I don't want this for the linux autotools build since I _really_ want to make sure that people build all the tests, always. But if you can do it with an ESX platform check we could merge this upstream even. -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] WAs in init_clock_gating?
On Mon, Jul 7, 2014 at 11:16 PM, Jesse Barnes jbar...@virtuousgeek.org wrote: I don't think it's unreasonable to use a macro that checks a global list for whether to apply a given WA. They'll be scattered all over, but at least it'll be easy to see: 1) whether we implement a given workaround and 2) which platforms steppings it applies to based on the table. Oh, I agree it's not unreasonable. But I'm kinda begging for the simple solution since months (years?) and haven't gotten it, while still getting a steady stream of bug reports and issues. So I've readjusted my expectations ;-) If someone delivers the real deal I'll certainly won't reject it. -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: Implement MI decode for gen8
Ipehr just carries Dword 0 and on Gen 8, offsets are located
on Dword 2 and 3 of MI_SEMAPHORE_WAIT.
This implementation was based on Ben's work and on Ville's suggestion for Ben
v2: fix typo. Removing spurious 0% from debug msg 0x%0%0. (Daniel)
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 42 ++---
1 file changed, 23 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 0217a41..05264ad 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2784,12 +2784,7 @@ static bool
ipehr_is_semaphore_wait(struct drm_device *dev, u32 ipehr)
{
if (INTEL_INFO(dev)-gen = 8) {
- /*
-* FIXME: gen8 semaphore support - currently we don't emit
-* semaphores on bdw anyway, but this needs to be addressed when
-* we merge that code.
-*/
- return false;
+ return (ipehr 23) == 0x1c;
} else {
ipehr = ~MI_SEMAPHORE_SYNC_MASK;
return ipehr == (MI_SEMAPHORE_MBOX | MI_SEMAPHORE_COMPARE |
@@ -2798,19 +2793,20 @@ ipehr_is_semaphore_wait(struct drm_device *dev, u32
ipehr)
}
static struct intel_engine_cs *
-semaphore_wait_to_signaller_ring(struct intel_engine_cs *ring, u32 ipehr)
+semaphore_wait_to_signaller_ring(struct intel_engine_cs *ring, u32 ipehr, u64
offset)
{
struct drm_i915_private *dev_priv = ring-dev-dev_private;
struct intel_engine_cs *signaller;
int i;
if (INTEL_INFO(dev_priv-dev)-gen = 8) {
- /*
-* FIXME: gen8 semaphore support - currently we don't emit
-* semaphores on bdw anyway, but this needs to be addressed when
-* we merge that code.
-*/
- return NULL;
+ for_each_ring(signaller, dev_priv, i) {
+ if (ring == signaller)
+ continue;
+
+ if (offset ==
signaller-semaphore.signal_ggtt[ring-id])
+ return signaller;
+ }
} else {
u32 sync_bits = ipehr MI_SEMAPHORE_SYNC_MASK;
@@ -2823,8 +2819,8 @@ semaphore_wait_to_signaller_ring(struct intel_engine_cs
*ring, u32 ipehr)
}
}
- DRM_ERROR(No signaller ring found for ring %i, ipehr 0x%08x\n,
- ring-id, ipehr);
+ DRM_ERROR(No signaller ring found for ring %i, ipehr 0x%08x, offset
0x%016llx\n,
+ ring-id, ipehr, offset);
return NULL;
}
@@ -2834,7 +2830,8 @@ semaphore_waits_for(struct intel_engine_cs *ring, u32
*seqno)
{
struct drm_i915_private *dev_priv = ring-dev-dev_private;
u32 cmd, ipehr, head;
- int i;
+ u64 offset = 0;
+ int i, backwards;
ipehr = I915_READ(RING_IPEHR(ring-mmio_base));
if (!ipehr_is_semaphore_wait(ring-dev, ipehr))
@@ -2843,13 +2840,15 @@ semaphore_waits_for(struct intel_engine_cs *ring, u32
*seqno)
/*
* HEAD is likely pointing to the dword after the actual command,
* so scan backwards until we find the MBOX. But limit it to just 3
-* dwords. Note that we don't care about ACTHD here since that might
+* or 4 dwords depending on the semaphore wait command size.
+* Note that we don't care about ACTHD here since that might
* point at at batch, and semaphores are always emitted into the
* ringbuffer itself.
*/
head = I915_READ_HEAD(ring) HEAD_ADDR;
+ backwards = (INTEL_INFO(ring-dev)-gen = 8) ? 5 : 4;
- for (i = 4; i; --i) {
+ for (i = backwards; i; --i) {
/*
* Be paranoid and presume the hw has gone off into the wild -
* our ring is smaller than what the hardware (and hence
@@ -2869,7 +2868,12 @@ semaphore_waits_for(struct intel_engine_cs *ring, u32
*seqno)
return NULL;
*seqno = ioread32(ring-buffer-virtual_start + head + 4) + 1;
- return semaphore_wait_to_signaller_ring(ring, ipehr);
+ if (INTEL_INFO(ring-dev)-gen = 8) {
+ offset = ioread32(ring-buffer-virtual_start + head + 12);
+ offset = 32;
+ offset = ioread32(ring-buffer-virtual_start + head + 8);
+ }
+ return semaphore_wait_to_signaller_ring(ring, ipehr, offset);
}
static int semaphore_passed(struct intel_engine_cs *ring)
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] linux-next: Tree for Jun 19 (drm/i915)
On Monday, July 07, 2014 10:06:59 PM Daniel Vetter wrote:
On Mon, Jul 07, 2014 at 10:01:27PM +0200, Rafael J. Wysocki wrote:
On Monday, July 07, 2014 04:54:23 PM Daniel Vetter wrote:
On Wed, Jun 25, 2014 at 01:01:36AM +0200, Rafael J. Wysocki wrote:
On Tuesday, June 24, 2014 02:43:02 PM Jani Nikula wrote:
On Thu, 19 Jun 2014, Randy Dunlap rdun...@infradead.org wrote:
On 06/18/14 23:16, Stephen Rothwell wrote:
Hi all,
The powerpc allyesconfig is again broken more than usual.
Changes since 20140618:
on i386:
CONFIG_ACPI is not enabled.
CC drivers/gpu/drm/i915/i915_drv.o
../drivers/gpu/drm/i915/i915_drv.c: In function 'i915_drm_freeze':
../drivers/gpu/drm/i915/i915_drv.c:547:2: error: implicit
declaration of function 'acpi_target_system_state'
[-Werror=implicit-function-declaration]
../drivers/gpu/drm/i915/i915_drv.c:547:36: error: 'ACPI_STATE_S3'
undeclared (first use in this function)
../drivers/gpu/drm/i915/i915_drv.c:547:36: note: each undeclared
identifier is reported only once for each function it appears in
CC net/dccp/qpolicy.o
cc1: some warnings being treated as errors
make[5]: *** [drivers/gpu/drm/i915/i915_drv.o] Error 1
Thanks for the report, we'll fix it.
Can anyone explain why include/linux/acpi_bus.h has #ifdef
CONFIG_ACPI_SLEEP and conditional build for a dummy inline version of
acpi_target_system_state(), *but* that does not get included or used
if
CONFIG_ACPI=n? Additionally, the combination of CONFIG_ACPI=y and
CONFIG_ACPI_SLEEP=n does not seem to work at all.
These two things look like bugs to me. Most likely not tested
thoruoughly
enough.
So we'll really have to sprinkle #ifdef CONFIG_ACPI all over, instead
of
neatly using the dummy versions that someone has gone through the
trouble of adding?
No, we don't have to.
Back from my vacation and I didn't see a conclusion to this issue here.
Rafael, have you fixed this in your acpi tree or do I need to do something
in drm-intel?
I was on vacation too. :-)
Please have a look if i915 includes acpi/acpi_bus.h directly anywhere. If
so,
it should include linux/acpi.h instead. I'll fix up the rest in the ACPI
tree.
We seem to only use linux/acpi.h and acpi/(video|button).h, at least
according to a grep include.*acpi. So I think we're good in i915 land.
Thanks for taking care of this.
The patch below should fix this if I'm not mistaken.
Rafael
---
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Subject: ACPI / sleep: Fix up acpi_target_system_state() stub definition
The static inline stub for acpi_target_system_state() is defined in
include/acpi/acpi_bus.h, but that file is only included if CONFIG_ACPI
is set, so drivers that use acpi_target_system_state() will fail to
build for CONFIG_ACPI unset.
To prevent that from happening move the definition of the static
inline stub for acpi_target_system_state() to include/linux/acpi.h.
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
include/acpi/acpi_bus.h |6 --
include/linux/acpi.h|6 ++
2 files changed, 6 insertions(+), 6 deletions(-)
Index: linux-pm/include/acpi/acpi_bus.h
===
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -566,12 +566,6 @@ static inline int acpi_pm_device_sleep_w
}
#endif
-#ifdef CONFIG_ACPI_SLEEP
-u32 acpi_target_system_state(void);
-#else
-static inline u32 acpi_target_system_state(void) { return ACPI_STATE_S0; }
-#endif
-
static inline bool acpi_device_power_manageable(struct acpi_device *adev)
{
return adev-flags.power_manageable;
Index: linux-pm/include/linux/acpi.h
===
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -541,6 +541,12 @@ static inline void arch_reserve_mem_area
#define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
#endif
+#ifdef CONFIG_ACPI_SLEEP
+u32 acpi_target_system_state(void);
+#else
+static inline u32 acpi_target_system_state(void) { return ACPI_STATE_S0; }
+#endif
+
#if defined(CONFIG_ACPI) defined(CONFIG_PM_RUNTIME)
int acpi_dev_runtime_suspend(struct device *dev);
int acpi_dev_runtime_resume(struct device *dev);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/5] drm/i915: reorganize the unclaimed register detection code
On Fri, Jul 04, 2014 at 11:50:32AM -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
The current code only runs when we do an I915_WRITE operation. It
checks if the unclaimed register flag is set before we do the
operation, and then it checks it again after we do the operation. This
double check allows us to find out if the I915_WRITE operation in
question is the bad one, or if some previous code is the bad one. When
it finds a problem, our code uses DRM_ERROR to signal it.
The good thing about the current code is that it detects the problem,
so at least we can know we did something wrong. The problem is that
even though we find the problem, we don't really have much information
to actually debug it. So whenever I see one of these DRM_ERROR
messages on my systems, the first thing I do is apply a patch to
change the DRM_ERROR to a WARN and also check for unclaimed registers
on I915_READ operations. This local patch makes things even slower,
but it usually helps a lot in finding the bad code.
The first point here is that since the current code is only useful to
detect whether we have a problem or not, but it is not really good to
find the cause of the problem, I don't think we should be checking
both before and after every I915_WRITE operation: just doing the check
once should be enough for us to quickly detect problems. With this
change, the code that runs by default for every single user will only
do 1 read operation for every single I915_WRITE, instead of 2. This
patch does this change.
The second point is that the local patch I have should be upstream,
but since it makes things slower it should be disabled by default. So
I added the i915.mmio_debug option to enable it.
So after this patch, this is what will happen:
- By default, we will try to detect unclaimed registers once after
every I915_WRITE operation. Previously we tried twice for every
I915_WRITE.
- When we find an unclaimed register we will still print a DRM_ERROR
message, but we will now tell the user to try again with
i915.mmio_debug=1.
- When we use i915.mmio_debug=1 we will try to find unclaimed
registers both before and after every I915_READ and I915_WRITE
operation, and we will print stack traces in case we find them.
This should really help locating the exact point of the bad code
(or at least finding out that i915.ko is not the problem).
This commit also opens space for really-slow register debugging
operations on other platforms. In theory we can now add lots and lots
of debug code behind i915.mmio_debug, enable this option on our tests,
and catch more problems.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_params.c | 6
drivers/gpu/drm/i915/intel_uncore.c | 68
+
3 files changed, 68 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ac06c0f..51d867f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2092,6 +2092,7 @@ struct i915_params {
bool disable_display;
bool disable_vtd_wa;
int use_mmio_flip;
+ bool mmio_debug;
};
extern struct i915_params i915 __read_mostly;
diff --git a/drivers/gpu/drm/i915/i915_params.c
b/drivers/gpu/drm/i915/i915_params.c
index 8145729..7977872 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -49,6 +49,7 @@ struct i915_params i915 __read_mostly = {
.enable_cmd_parser = 1,
.disable_vtd_wa = 0,
.use_mmio_flip = 0,
+ .mmio_debug = 0,
};
module_param_named(modeset, i915.modeset, int, 0400);
@@ -161,3 +162,8 @@ MODULE_PARM_DESC(enable_cmd_parser,
module_param_named(use_mmio_flip, i915.use_mmio_flip, int, 0600);
MODULE_PARM_DESC(use_mmio_flip,
use MMIO flips (-1=never, 0=driver discretion [default],
1=always));
+
+module_param_named(mmio_debug, i915.mmio_debug, bool, 0600);
+MODULE_PARM_DESC(disable_vtd_wa,
+ Enable the MMIO debug code (default: false). This may negatively
+ affect performance.);
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 29145df..de5402f 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -513,21 +513,72 @@ ilk_dummy_write(struct drm_i915_private *dev_priv)
__raw_i915_write32(dev_priv, MI_MODE, 0);
}
+/**
+ * hsw_unclaimed_reg_debug - tries to find unclaimed registers
+ * @dev_priv: device private data
+ * @reg: register we're about to touch or just have touched
+ * @read: are we reading or writing a register now?
+ * @before: are we running this before or after we touch the register?
+ *
+ * This function tries to detect unclaimed registers and provide as much
+ * information as possible.
Re: [Intel-gfx] [PATCH 2/3] drm/i915: correct BLC vs PWM enable/disable ordering
On Thu, Jun 19, 2014 at 11:00:20AM -0700, Jesse Barnes wrote: Jani, can you review this one? It's still needed for us to conform to the eDP timing spec. Jani's already goofing off on vacation and I couldn't spot his r-b. Merged anyway, I guess people will scream fast enough if this breaks stuff ;-) -Daniel Thanks, Jesse On Mon, 31 Mar 2014 11:13:56 -0700 Jesse Barnes jbar...@virtuousgeek.org wrote: With the new checks in place, we can see we're doing things backwards, so fix them up per the spec. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_dp.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index b6f7087..d540fbe 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1273,8 +1273,6 @@ void intel_edp_panel_off(struct intel_dp *intel_dp) DRM_DEBUG_KMS(Turn eDP power off\n); - edp_wait_backlight_off(intel_dp); - WARN(!intel_dp-want_panel_vdd, Need VDD to turn off panel\n); /* By this time the PWM and BLC bits should be off already */ @@ -1316,6 +1314,9 @@ void intel_edp_backlight_on(struct intel_dp *intel_dp) return; DRM_DEBUG_KMS(\n); + + intel_panel_enable_backlight(intel_dp-attached_connector); + /* * If we enable the backlight right away following a panel power * on, we may see slight flicker as the panel syncs with the eDP @@ -1333,8 +1334,6 @@ void intel_edp_backlight_on(struct intel_dp *intel_dp) I915_WRITE(pp_ctrl_reg, pp); POSTING_READ(pp_ctrl_reg); - - intel_panel_enable_backlight(intel_dp-attached_connector); } void intel_edp_backlight_off(struct intel_dp *intel_dp) @@ -1350,8 +1349,6 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp) /* PWM must still be enabled here */ assert_pwm_enabled(intel_dp-attached_connector); - intel_panel_disable_backlight(intel_dp-attached_connector); - DRM_DEBUG_KMS(\n); pp = ironlake_get_pp_control(intel_dp); pp = ~EDP_BLC_ENABLE; @@ -1361,6 +1358,10 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp) I915_WRITE(pp_ctrl_reg, pp); POSTING_READ(pp_ctrl_reg); intel_dp-last_backlight_off = jiffies; + + edp_wait_backlight_off(intel_dp); + + intel_panel_disable_backlight(intel_dp-attached_connector); } static void ironlake_edp_pll_on(struct intel_dp *intel_dp) -- Jesse Barnes, Intel Open Source Technology Center ___ 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/4] drm/i915: don't warn if IRQs are disabled when shutting down display IRQs
(documenting what we discussed on IRC)
2014-06-20 13:29 GMT-03:00 Jesse Barnes jbar...@virtuousgeek.org:
This was always the case on our suspend path, but it was recently
exposed by the change to use our runtime IRQ disable routine rather than
the full DRM IRQ disable. Keep the warning on the enable side, as that
really would indicate a bug.
While I understand the patch and think it's a reasonable thing to do,
I feel the need to spend some time persuading you in replacing it with
something that doesn't involve removing WARNs from our driver. While
the driver is runtime suspended, no one should really be manipulating
IRQs, even if they're disabling stuff that is already disabled: this
reflects there's probably a problem somewhere. These WARNs are
extremely helpful for the runtime PM feature because almost nobody
actually uses runtime PM to notice any bugs with it, so the WARNs can
make QA report bugs and bisect things for us.
Also, it seems S3 suspend is currently a little disaster on our
driver. Your 6 patches just solve some of the WARNs, not all of them.
And last week I even solved another WARN on the S3 path. I just did
some investigation, and the current bad commits are:
8abdc17941c71b37311bb93876ac83dce58160c8,
e11aa362308f5de467ce355a2a2471321b15a35c and
85e90679335f56d162f4a0ff525573818e17ce44. If I just revert these 3
commits, S3 doesn't give me any WARNs.
Instead of the change you proposed, can't we think of another solution
that would maybe address all the 3 regressions we have? Since we're
always submitting patches to change the order we do things at S3
suspend/resume, shouldn't we add something like dev_priv-suspending
that could be used to avoid the strict ordering that is required
during runtime?
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/i915_irq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1c1ec22..fe3b309 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -151,7 +151,7 @@ ironlake_disable_display_irq(struct drm_i915_private
*dev_priv, u32 mask)
{
assert_spin_locked(dev_priv-irq_lock);
- if (WARN_ON(dev_priv-pm.irqs_disabled))
+ if (dev_priv-pm.irqs_disabled)
return;
if ((dev_priv-irq_mask mask) != mask) {
--
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] Linux 3.16-rc2
On Mon, 7 Jul 2014, Chris Wilson wrote: this patch on top of v3.16-rc3-62-gd92a333 makes the resume from ram regression go away on my machine: Hm, we could conditionalize this hack on IS_G4X ... Chris, thoughts? As different machines favour different w/a, I think the issue is mostly timing related. It could be sequence of register writes, but we tried different orders early on. The next experiment I guess would be to insert small delays between each write to see if that helps. Or to write each register twice. I actually tried to introduce rather large delays between individual I915_WRITE() calls in the ring initialization sequence a couple weeks ago already, but it resulted in complete machine lockup (which is worse than my usual symptoms) during resume. Therefore I probably lack the knowledge of internal workings of the HW that would allow me to guess what the reasonable timeout value should be. Willing to test any patches. -- Jiri Kosina SUSE Labs ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] kms_universal_plane: Don't assert outside of fixtures/subtests
On Mon, Jul 07, 2014 at 11:18:49PM +0200, Daniel Vetter wrote: On Fri, Jul 04, 2014 at 12:01:24PM +0100, Damien Lespiau wrote: Doing otherwise breaks listing the subtests. The test was throwing an error out when universal planes were disabled as well because of that. Cc: Matt Roper matthew.d.ro...@intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- tests/kms_universal_plane.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/kms_universal_plane.c b/tests/kms_universal_plane.c index fe0fde3..c64cdbb 100644 --- a/tests/kms_universal_plane.c +++ b/tests/kms_universal_plane.c @@ -133,6 +133,7 @@ functional_test_pipe(data_t *data, enum pipe pipe, igt_output_t *output) int num_primary = 0, num_cursor = 0; int i; + igt_assert(data-display.has_universal_planes); Shouldn't this be an igt_require instead? In any case moving this into subtests is correct. There's already a igt_require in the fixture block, I guess Matt wanted to be extra extra sure. Could have removed it as well. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: make system freeze support depend on CONFIG_ACPI_SLEEP
On 6/24/2014 5:12 PM, Imre Deak wrote: On Tue, 2014-06-24 at 17:53 +0300, Jani Nikula wrote: On Tue, 24 Jun 2014, Imre Deak imre.d...@intel.com wrote: On Tue, 2014-06-24 at 16:54 +0300, Jani Nikula wrote: On Mon, 23 Jun 2014, Imre Deak imre.d...@intel.com wrote: To achieve further power savings during system freeze (aka connected standby, or s0ix) we have to send a PCI_D1 opregion notification. As the information about the state we're entering (system freeze, suspend to ram or suspend to disk) is only available through the ACPI subsystem, make this support depend on the relevant kconfig option. Things will still work if this option isn't set, albeit with less than optimial power saving. This also fixes a compile breakage when the option is not set introduced in commit e5747e3adcd67ae27105003ec99fb58cba180105 Author: Jesse Barnes jbar...@virtuousgeek.org Date: Thu Jun 12 08:35:47 2014 -0700 drm/i915: send proper opregion notifications on suspend/resume Reported-by: Randy Dunlap rdun...@infradead.org Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 7ae4e2a..43dc8f7 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -544,10 +544,11 @@ static int i915_drm_freeze(struct drm_device *dev) i915_save_state(dev); - if (acpi_target_system_state() = ACPI_STATE_S3) - opregion_target_state = PCI_D3cold; - else + opregion_target_state = PCI_D3cold; +#if IS_ENABLED(CONFIG_ACPI_SLEEP) Maybe this should just check for CONFIG_ACPI? I wanted to send the PCI_D1 signal only if we are sure that the target sleep state is S0ix (or S1/2) and fall back to the old behavior to send PCI_D3cold in all other cases. But you are right, it would make much sense if CONFIG_ACPI_SLEEP=n the target state would be always S0ix. Rafael could you confirm this? The target state should be S0 for CONFIG_ACPI_SLEEP unset. intel_opregion_notify_adapter() is a NOP for CONFIG_ACPI=n anyway. Ok, but the question for me is what's the target sleep state in case of CONFIG_ACPI=y and CONFIG_ACPI_SLEEP=n. And AFAICT CONFIG_ACPI=y CONFIG_ACPI_SLEEP=n is broken. Broken how? But it seems like a valid configuration. So it needs to be fixed separately. It is a valid configuration. Rafael ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: make system freeze support depend on CONFIG_ACPI_SLEEP
On 7/8/2014 1:13 AM, Rafael J. Wysocki wrote: On 6/24/2014 5:12 PM, Imre Deak wrote: On Tue, 2014-06-24 at 17:53 +0300, Jani Nikula wrote: On Tue, 24 Jun 2014, Imre Deak imre.d...@intel.com wrote: On Tue, 2014-06-24 at 16:54 +0300, Jani Nikula wrote: On Mon, 23 Jun 2014, Imre Deak imre.d...@intel.com wrote: To achieve further power savings during system freeze (aka connected standby, or s0ix) we have to send a PCI_D1 opregion notification. As the information about the state we're entering (system freeze, suspend to ram or suspend to disk) is only available through the ACPI subsystem, make this support depend on the relevant kconfig option. Things will still work if this option isn't set, albeit with less than optimial power saving. This also fixes a compile breakage when the option is not set introduced in commit e5747e3adcd67ae27105003ec99fb58cba180105 Author: Jesse Barnes jbar...@virtuousgeek.org Date: Thu Jun 12 08:35:47 2014 -0700 drm/i915: send proper opregion notifications on suspend/resume Reported-by: Randy Dunlap rdun...@infradead.org Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 7ae4e2a..43dc8f7 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -544,10 +544,11 @@ static int i915_drm_freeze(struct drm_device *dev) i915_save_state(dev); -if (acpi_target_system_state() = ACPI_STATE_S3) -opregion_target_state = PCI_D3cold; -else +opregion_target_state = PCI_D3cold; +#if IS_ENABLED(CONFIG_ACPI_SLEEP) Maybe this should just check for CONFIG_ACPI? I wanted to send the PCI_D1 signal only if we are sure that the target sleep state is S0ix (or S1/2) and fall back to the old behavior to send PCI_D3cold in all other cases. But you are right, it would make much sense if CONFIG_ACPI_SLEEP=n the target state would be always S0ix. Rafael could you confirm this? The target state should be S0 for CONFIG_ACPI_SLEEP unset. Actually, no, it shouldn't. Or rather it depends on why CONFIG_ACPI_SLEEP is unset. If that's because CONFIG_ACPI is unset, the target sleep state is undefined and acpi_target_system_state() should not be called then. Rafael ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] linux-next: Tree for Jun 19 (drm/i915)
On Monday, July 07, 2014 11:49:22 PM Rafael J. Wysocki wrote: On Monday, July 07, 2014 10:06:59 PM Daniel Vetter wrote: On Mon, Jul 07, 2014 at 10:01:27PM +0200, Rafael J. Wysocki wrote: On Monday, July 07, 2014 04:54:23 PM Daniel Vetter wrote: On Wed, Jun 25, 2014 at 01:01:36AM +0200, Rafael J. Wysocki wrote: On Tuesday, June 24, 2014 02:43:02 PM Jani Nikula wrote: On Thu, 19 Jun 2014, Randy Dunlap rdun...@infradead.org wrote: On 06/18/14 23:16, Stephen Rothwell wrote: Hi all, The powerpc allyesconfig is again broken more than usual. Changes since 20140618: on i386: CONFIG_ACPI is not enabled. CC drivers/gpu/drm/i915/i915_drv.o ../drivers/gpu/drm/i915/i915_drv.c: In function 'i915_drm_freeze': ../drivers/gpu/drm/i915/i915_drv.c:547:2: error: implicit declaration of function 'acpi_target_system_state' [-Werror=implicit-function-declaration] ../drivers/gpu/drm/i915/i915_drv.c:547:36: error: 'ACPI_STATE_S3' undeclared (first use in this function) ../drivers/gpu/drm/i915/i915_drv.c:547:36: note: each undeclared identifier is reported only once for each function it appears in CC net/dccp/qpolicy.o cc1: some warnings being treated as errors make[5]: *** [drivers/gpu/drm/i915/i915_drv.o] Error 1 Thanks for the report, we'll fix it. Can anyone explain why include/linux/acpi_bus.h has #ifdef CONFIG_ACPI_SLEEP and conditional build for a dummy inline version of acpi_target_system_state(), *but* that does not get included or used if CONFIG_ACPI=n? Additionally, the combination of CONFIG_ACPI=y and CONFIG_ACPI_SLEEP=n does not seem to work at all. These two things look like bugs to me. Most likely not tested thoruoughly enough. So we'll really have to sprinkle #ifdef CONFIG_ACPI all over, instead of neatly using the dummy versions that someone has gone through the trouble of adding? No, we don't have to. Back from my vacation and I didn't see a conclusion to this issue here. Rafael, have you fixed this in your acpi tree or do I need to do something in drm-intel? I was on vacation too. :-) Please have a look if i915 includes acpi/acpi_bus.h directly anywhere. If so, it should include linux/acpi.h instead. I'll fix up the rest in the ACPI tree. We seem to only use linux/acpi.h and acpi/(video|button).h, at least according to a grep include.*acpi. So I think we're good in i915 land. Thanks for taking care of this. The patch below should fix this if I'm not mistaken. So I was mistaken, as it turns out. The problem is that ACPI sleep states are not defined for CONFIG_ACPI unset, quite obviously, so using acpi_target_system_state() in that case doesn't make sense at all. Rafael ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] pin OABUFFER to GGTT
On Mon, Jul 7, 2014 at 9:43 PM, Daniel Vetter dan...@ffwll.ch wrote: On Tue, Jul 01, 2014 at 08:54:27PM +0100, Chris Wilson wrote: On Tue, Jul 01, 2014 at 05:16:30PM +, Mateo Lozano, Oscar wrote: The issue is they need: A) A buffer object. B) Bound to GGTT. C) That userspace knows the GGTT offset of, so that they can program OABUFFER with it. D) That userspace can map so that they can read the reported counters. They used to create a bo, call bo_pin on it, use args-offset to program OABUFFER (via MI_LOAD_REGISTER_IMM, I imagine), map it and read the counter values. They cannot do this anymore. The answer might be that all of this needs to be done by the kernel itself, but then we need to provide an interface to userspace... Yes. If you need to pin a buffer for a register, then it needs to be handled by the kernel. Especially one that provides information about other users. Short-circuiting the entire discussion here. Afaik there's two OA modes: - inline with the batch with MI_REPORT_PERF - global with the ringbuffer setup with the OABUFFER registers There's one other; we can read the counters via mmio for HSW+ too. I've found that quite convenient for experimenting with capturing OA counters from userspace. A notable disadvantage with reading via mmio though is that there's no latch and hold mechanism to make sure the counters are frozen while they are read back-to-back. The later should indeed be fully controlled by the kernel as Chris suggested and exposed as an off-cpu performance monitoring unit through the perf subsystem. Chris has rfc patches floating somewhere to do this for other gpu perf data. Just to let folks know; I've recently been starting to play around with Chris' perf patch plus a couple of small patches on top and was planning on experimenting with exposing the oa counters this way soon. I was hoping to try and pick up the discussion of this patch soon too and give some comments, but I'd also like to collect some concrete data as a reference point first, to be more confident in my own understanding of how things are behaving. One fun thing here is the coordination between these two OA modes since iirc they both use the same setup registers for the performance counter configuration. No idea yet how to solve this. But really userspace shouldn't program ggtt offset, not even debug/performance measuring tools. I just wanted to pop my head up here just so others are aware that I'm another person looking at this area, aiming to understand how best to make this data available to both GL and tools; initially considering Mesa's performance query extensions (that don't always report reliable data currently) and tools like intel-gpu-top. -- Regards, Robert -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 mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Introduce intel_fb_obj() macro
Add an intel_fb_obj() macro that returns the GEM object associated with
a DRM framebuffer. This macro is safe to call on NULL framebuffers (a
NULL object pointer will be returned in this case).
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
drivers/gpu/drm/i915/intel_drv.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 45afd25..426c366 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -485,6 +485,7 @@ struct cxsr_latency {
#define to_intel_encoder(x) container_of(x, struct intel_encoder, base)
#define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base)
#define to_intel_plane(x) container_of(x, struct intel_plane, base)
+#define intel_fb_obj(x) (x ? to_intel_framebuffer(x)-obj : NULL)
struct intel_hdmi {
u32 hdmi_reg;
--
1.8.5.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
