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. -Chris I'm unclear why they need the offset. Can it not work like any other relocation, except with the requirement that it's in the GGTT? I'd also argue that they need to be able to map it (they need the contents, which may or may not be mapped). However, I think this is a very minor point. With the command validator this should be a pretty reasonable thing to accomplish. I think we can just give a flag for the reloc that it needs to be in the GGTT, and then pass a check to the command scanner that only that one offset is allowed, and only for OA. -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] pin OABUFFER to GGTT
On Tue, Jul 01, 2014 at 11:40:52PM -0700, Ben Widawsky 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. -Chris I'm unclear why they need the offset. Can it not work like any other relocation, except with the requirement that it's in the GGTT? I'd also argue that they need to be able to map it (they need the contents, which may or may not be mapped). However, I think this is a very minor point. With the command validator this should be a pretty reasonable thing to accomplish. I think we can just give a flag for the reloc that it needs to be in the GGTT, and then pass a check to the command scanner that only that one offset is allowed, and only for OA. If the intention was to only measure within a batch and the command parser ensured that the register was cleared before the end of the batch, fine. That's not an information leak nor do we keep the hardware pointing to an unpinned buffer afterwards. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type
On Thu, Jun 19, 2014 at 05:53:51PM +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
Making sure all relevant people are Cc'd.
I think we should wait for the hypervisor to settle
on what it wants to implement before making guest
changes. Otherwise we'll just add more guest configurations
that need to be supported.
---
drivers/gpu/drm/i915/i915_drv.c | 19 +++
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 651e65e..cb2526e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -417,18 +417,8 @@ void intel_detect_pch(struct drm_device *dev)
return;
}
- /*
- * 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.
- *
- * In some virtualized environments (e.g. XEN), there is irrelevant
- * ISA bridge in the system. To work reliably, we should scan trhough
- * all the ISA bridge devices and check for the first match, instead
- * of only checking the first one.
- */
- while ((pch = pci_get_class(PCI_CLASS_BRIDGE_ISA 8, pch))) {
+ pch = pci_get_bus_and_slot(0, PCI_DEVFN(0x1f, 0));
+ if (pch) {
if (pch-vendor == PCI_VENDOR_ID_INTEL) {
unsigned short id = pch-device
INTEL_PCH_DEVICE_ID_MASK;
dev_priv-pch_id = id;
@@ -462,10 +452,7 @@ void intel_detect_pch(struct drm_device *dev)
DRM_DEBUG_KMS(Found LynxPoint LP PCH\n);
WARN_ON(!IS_HASWELL(dev));
WARN_ON(!IS_ULT(dev));
- } else
- continue;
-
- break;
+ }
}
}
if (!pch)
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot.
On Tue, 03 Jun 2014, clinton.a.tay...@intel.com 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
Clint, the patch no longer applies cleanly. Please submit a rebased
version on top of current -nightly. Sorry for the extra trouble.
BR,
Jani.
---
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 5ca68aa9..cede9bc 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
@@ -302,6 +304,38 @@ 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 intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port-base.base.dev;
+ 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);
@@ -3344,6 +3378,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
mutex_lock(dev-mode_config.mutex);
edp_panel_vdd_off_sync(intel_dp);
mutex_unlock(dev-mode_config.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);
}
@@ -3782,6 +3820,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 328b1a7..6d0d96e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -510,6 +510,8 @@ struct intel_dp {
unsigned long last_power_on;
unsigned long last_backlight_off;
bool psr_setup_done;
+ 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
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type
On 2014/7/2 14:21, Michael S. Tsirkin wrote:
On Thu, Jun 19, 2014 at 05:53:51PM +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
Making sure all relevant people are Cc'd.
Are you saying you and Paolo? Or others I'm missing?
I think we should wait for the hypervisor to settle
on what it wants to implement before making guest
changes. Otherwise we'll just add more guest configurations
that need to be supported.
Agreed. Actually this is why I just send this by RFC.
Thanks
Tiejun
---
drivers/gpu/drm/i915/i915_drv.c | 19 +++
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 651e65e..cb2526e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -417,18 +417,8 @@ void intel_detect_pch(struct drm_device *dev)
return;
}
- /*
-* 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.
-*
-* In some virtualized environments (e.g. XEN), there is irrelevant
-* ISA bridge in the system. To work reliably, we should scan trhough
-* all the ISA bridge devices and check for the first match, instead
-* of only checking the first one.
-*/
- while ((pch = pci_get_class(PCI_CLASS_BRIDGE_ISA 8, pch))) {
+ pch = pci_get_bus_and_slot(0, PCI_DEVFN(0x1f, 0));
+ if (pch) {
if (pch-vendor == PCI_VENDOR_ID_INTEL) {
unsigned short id = pch-device
INTEL_PCH_DEVICE_ID_MASK;
dev_priv-pch_id = id;
@@ -462,10 +452,7 @@ void intel_detect_pch(struct drm_device *dev)
DRM_DEBUG_KMS(Found LynxPoint LP PCH\n);
WARN_ON(!IS_HASWELL(dev));
WARN_ON(!IS_ULT(dev));
- } else
- continue;
-
- break;
+ }
}
}
if (!pch)
--
1.9.1
___
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
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.
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;
--
2.0.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] pin OABUFFER to GGTT
- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Wednesday, July 02, 2014 7:56 AM To: Ben Widawsky Cc: Mateo Lozano, Oscar; Intel-gfx@lists.freedesktop.org; Madajczak, Tomasz; Rutkowski, Adam J; Jesse Barnes (jbar...@virtuousgeek.org) Subject: Re: [Intel-gfx] pin OABUFFER to GGTT On Tue, Jul 01, 2014 at 11:40:52PM -0700, Ben Widawsky 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. -Chris I'm unclear why they need the offset. Can it not work like any other relocation, except with the requirement that it's in the GGTT? I'd also argue that they need to be able to map it (they need the contents, which may or may not be mapped). However, I think this is a very minor point. With the command validator this should be a pretty reasonable thing to accomplish. I think we can just give a flag for the reloc that it needs to be in the GGTT, and then pass a check to the command scanner that only that one offset is allowed, and only for OA. If the intention was to only measure within a batch and the command parser ensured that the register was cleared before the end of the batch, fine. That's not an information leak nor do we keep the hardware pointing to an unpinned buffer afterwards. Chris Wilson, Intel Open Source Technology Centre Tomasz: is the intention to only measure within a batch? My impression is that you wanted to maintain the OABUFFER programmed and then collect performance reports for a period of time (probably for several batchbuffers). If that´s the case, the relocation approach is not possible. Chris: please notice that the bo_pin ioctl they were using until now required root, so the information leak about other users was not that bad. Still, I tend to agree that bo_pin for this is overkill and the programming of OABUFFER should be carried out by the kernel (with some interface to userspace so that they can retrieve the actual contents of the buffer... root only of course). -- Oscar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] Moving rotation_property to drm_plane
Hi, Did anybody get a chance to review these patches? Thanks, Sonika On 6/24/2014 5:38 PM, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com As suggested by Daniel and Damien, moved rotation_property to drm_plane. Also moved resetting of rotation_property to restore_fbdev_mode which will be used in switching VT use case along with the driver lastclose path. v2: Removing unused dev_priv from second patch instead of third (some mixup due to earlier reformatting). Sonika Jindal (2): drm/i915: Add 180 degree primary plane rotation support drm: Resetting rotation property Ville Syrjälä (1): drm/i915: Add rotation property for sprites drivers/gpu/drm/drm_fb_helper.c | 14 - drivers/gpu/drm/i915/i915_dma.c |5 -- drivers/gpu/drm/i915/i915_reg.h |1 + drivers/gpu/drm/i915/intel_display.c | 93 -- drivers/gpu/drm/i915/intel_pm.c |8 +++ drivers/gpu/drm/i915/intel_sprite.c | 40 ++- include/drm/drm_crtc.h |1 + 7 files changed, 150 insertions(+), 12 deletions(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] drm: i915: plane B assertion failure, should be off on pipe B but is still active
On Tue, 2014-07-01 at 12:17 +0300, Jani Nikula wrote: This does not ring any bells to me (but that doesn't prove anything). A bisect result would be awesome. Too bad. Unless someone else has a better idea I hope to start bisecting one of these days (that might take quite some time, especially since I can't yet narrow the bisect to changes in drivers/gpu/drm/i915/, can I?). Don't expect results before v3.16-rc4. Feel free to remind me if I stay silent on this subject for too long. Paul Bolle ___ 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
On 1 July 2014 19:49, Liu, Ying2 ying2@intel.com wrote: The latest intel-gpu-tools has dependency on cairo. We don’t use cairo. Is there any way to build intel-gpu-tools without cairo? Cairo has been a required dependency since version 1.2. It is used by various tests and tools and therefore there isn't a way to build intel-gpu-tools without it. Thank you so much for your help Ying ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] how to build intel-gpu-tools without cairo
-Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Thomas Wood Sent: Wednesday, July 02, 2014 11:04 AM To: Liu, Ying2 Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] how to build intel-gpu-tools without cairo On 1 July 2014 19:49, Liu, Ying2 ying2@intel.com wrote: The latest intel-gpu-tools has dependency on cairo. We don’t use cairo. Is there any way to build intel-gpu-tools without cairo? Cairo has been a required dependency since version 1.2. It is used by various tests and tools and therefore there isn't a way to build intel-gpu-tools without it. For Android (see tests/Android.mk) we skip the building of those tests that depend on Cairo (at least until we get Cairo fully compiled and working on the Android tree). Maybe you can do the same? -- Oscar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] pin OABUFFER to GGTT
AdTomasz: is the intention to only measure within a batch? My impression is that you wanted to maintain the OABUFFER programmed and then collect performance reports for a period of time (probably for several batchbuffers). If that´s the case, the relocation approach is not possible. I confirm that relocation approach is not possible. OA buffer collect performance data for many hw contexts, and thus many batch buffers, it is a hardware reporting mechanism not bound to any specific GPU command send within a BB. The report is triggered by hw when: - hw context switches, that action can be caused by any level GPU preemption, GUC direct context submission, and regular kernel exec list submission - GPU frequency change - that is trigger by power management - RC6 transition - time interval expiration (if programmed so for collect performance data at regular time intervals) Each instance of UMD maps then the OA buffer to examine it within the Performance Query measurement window. Window is learned with Query::Begin and Query::End timestamps and reported OABufferTail pointer on Query::End. As OA buffer is large (up to 16MB), any KMD assist in conveying that data to UMD on each Query::End is not applicable - queries might be done per each Draw. There isn't any secret or privacy in that OA buffer data - just results of performance counters, shown by tools such as GPA/ Vtune. Thanks, Tomasz -Original Message- From: Mateo Lozano, Oscar Sent: Wednesday, July 02, 2014 10:50 AM To: Chris Wilson; Ben Widawsky Cc: Intel-gfx@lists.freedesktop.org; Madajczak, Tomasz; Rutkowski, Adam J; Jesse Barnes (jbar...@virtuousgeek.org) Subject: RE: [Intel-gfx] pin OABUFFER to GGTT - Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Wednesday, July 02, 2014 7:56 AM To: Ben Widawsky Cc: Mateo Lozano, Oscar; Intel-gfx@lists.freedesktop.org; Madajczak, Tomasz; Rutkowski, Adam J; Jesse Barnes (jbar...@virtuousgeek.org) Subject: Re: [Intel-gfx] pin OABUFFER to GGTT On Tue, Jul 01, 2014 at 11:40:52PM -0700, Ben Widawsky 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. -Chris I'm unclear why they need the offset. Can it not work like any other relocation, except with the requirement that it's in the GGTT? I'd also argue that they need to be able to map it (they need the contents, which may or may not be mapped). However, I think this is a very minor point. With the command validator this should be a pretty reasonable thing to accomplish. I think we can just give a flag for the reloc that it needs to be in the GGTT, and then pass a check to the command scanner that only that one offset is allowed, and only for OA. If the intention was to only measure within a batch and the command parser ensured that the register was cleared before the end of the batch, fine. That's not an information leak nor do we keep the hardware pointing to an unpinned buffer afterwards. Chris Wilson, Intel Open Source Technology Centre Tomasz: is the intention to only measure within a batch? My impression is that you wanted to maintain the OABUFFER programmed and then collect performance reports for a period of time (probably for several batchbuffers). If that´s the case, the relocation approach is not possible. Chris: please notice that the bo_pin ioctl they were using until now required root, so the information leak about other users was not that bad. Still, I tend to agree that bo_pin for this is overkill and the programming of OABUFFER should be carried out by the kernel (with some interface to userspace so that they can retrieve the actual contents of the buffer... root only of course). -- Oscar smime.p7s Description: S/MIME cryptographic signature Intel Technology Poland sp. z o.o. ul. Slowackiego 173 | 80-298 Gdansk | Sad
Re: [Intel-gfx] pin OABUFFER to GGTT
On Wed, Jul 02, 2014 at 10:31:45AM +, Madajczak, Tomasz wrote: There isn't any secret or privacy in that OA buffer data - just results of performance counters, shown by tools such as GPA/ Vtune. Chris alludes to the fact that if there's a way for one application to gather data about other applications (whatever kind of data), that's an automatic security concern. One may think that's only very theoretical, but the side channel attacks are (surpringly) real and effective. It cannot be proven easily that exposing data about other contexes is totally secure. There are ways around that however. If only root can gather that data, I think we've contained the issue enough for the case at hand? -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] Documentation: drm: Removing placeholders for generic drm properties description
On Wed, Jun 25, 2014 at 11:08:05AM +0530, sonika.jin...@intel.com wrote:
From: Sagar Kamble sagar.a.kam...@intel.com
These property descriptions were kept as placeholder. Removing them for
simplicity.
Cc: damien.lesp...@intel.com
Cc: daniel.vet...@ffwll.ch
Cc: ville.syrj...@linux.intel.com
Cc: linux-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
Reviewed-by: Damien Lespiau damien.lesp...@intel.com
---
Documentation/DocBook/drm.tmpl | 64
+++---
1 file changed, 10 insertions(+), 54 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index b314a42..3adb6be 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2648,8 +2648,8 @@ void intel_crt_init(struct drm_device *dev)
td valign=top TBD/td
/tr
tr
- td rowspan=21 valign=top i915/td
- td rowspan=3 valign=top Generic/td
+ td rowspan=20 valign=top i915/td
+ td rowspan=2 valign=top Generic/td
td valign=top Broadcast RGB/td
td valign=top ENUM/td
td valign=top { Automatic, Full, Limited 16:235 }/td
@@ -2664,13 +2664,6 @@ void intel_crt_init(struct drm_device *dev)
td valign=top TBD/td
/tr
tr
- td valign=top Standard name as in DRM/td
- td valign=top Standard type as in DRM/td
- td valign=top Standard value as in DRM/td
- td valign=top Standard Object as in DRM/td
- td valign=top TBD/td
- /tr
- tr
td rowspan=17 valign=top SDVO-TV/td
td valign=top “mode”/td
td valign=top ENUM/td
@@ -2799,8 +2792,8 @@ void intel_crt_init(struct drm_device *dev)
td valign=top TBD/td
/tr
tr
- td rowspan=3 valign=top CDV gma-500/td
- td rowspan=3 valign=top Generic/td
+ td rowspan=2 valign=top CDV gma-500/td
+ td rowspan=2 valign=top Generic/td
td valign=top Broadcast RGB/td
td valign=top ENUM/td
td valign=top { “Full”, “Limited 16:235” }/td
@@ -2815,15 +2808,8 @@ void intel_crt_init(struct drm_device *dev)
td valign=top TBD/td
/tr
tr
- td valign=top Standard name as in DRM/td
- td valign=top Standard type as in DRM/td
- td valign=top Standard value as in DRM/td
- td valign=top Standard Object as in DRM/td
- td valign=top TBD/td
- /tr
- tr
- td rowspan=20 valign=top Poulsbo/td
- td rowspan=2 valign=top Generic/td
+ td rowspan=19 valign=top Poulsbo/td
+ td rowspan=1 valign=top Generic/td
td valign=top “backlight”/td
td valign=top RANGE/td
td valign=top Min=0, Max=100/td
@@ -2831,13 +2817,6 @@ void intel_crt_init(struct drm_device *dev)
td valign=top TBD/td
/tr
tr
- td valign=top Standard name as in DRM/td
- td valign=top Standard type as in DRM/td
- td valign=top Standard value as in DRM/td
- td valign=top Standard Object as in DRM/td
- td valign=top TBD/td
- /tr
- tr
td rowspan=17 valign=top SDVO-TV/td
td valign=top “mode”/td
td valign=top ENUM/td
@@ -3064,7 +3043,7 @@ void intel_crt_init(struct drm_device *dev)
td valign=top TBD/td
/tr
tr
- td rowspan=3 valign=top i2c/ch7006_drv/td
+ td rowspan=2 valign=top i2c/ch7006_drv/td
td valign=top Generic/td
td valign=top “scale”/td
td valign=top RANGE/td
@@ -3073,14 +3052,7 @@ void intel_crt_init(struct drm_device *dev)
td valign=top TBD/td
/tr
tr
- td rowspan=2 valign=top TV/td
- td valign=top Standard names as in DRM/td
- td valign=top Standard types as in DRM/td
- td valign=top Standard Values as in DRM/td
- td valign=top Standard object as in DRM/td
- td valign=top TBD/td
- /tr
- tr
+ td rowspan=1 valign=top TV/td
td valign=top “mode”/td
td valign=top ENUM/td
td valign=top { PAL, PAL-M,PAL-N}, ”PAL-Nc
@@ -3089,7 +3061,7 @@ void intel_crt_init(struct drm_device *dev)
td valign=top TBD/td
/tr
tr
- td rowspan=16 valign=top nouveau/td
+ td rowspan=15 valign=top nouveau/td
td rowspan=6 valign=top NV10 Overlay/td
td valign=top colorkey/td
td valign=top RANGE/td
@@ -3198,14 +3170,6 @@ void intel_crt_init(struct drm_device *dev)
td valign=top TBD/td
/tr
tr
- td valign=top Generic/td
- td valign=top Standard name as in DRM/td
- td valign=top Standard type as in DRM/td
- td valign=top Standard value as in DRM/td
- td valign=top Standard Object as in DRM/td
- td valign=top TBD/td
- /tr
- tr
td rowspan=2 valign=top omap/td
td rowspan=2 valign=top Generic/td
td valign=top “rotation”/td
@@ -3236,7 +3200,7 @@ void intel_crt_init(struct drm_device *dev)
td valign=top TBD/td
/tr
tr
- td
Re: [Intel-gfx] [PATCH 2/2] Documentation: drm: describing rotation property for i915
On Wed, Jun 25, 2014 at 11:08:06AM +0530, sonika.jin...@intel.com wrote:
From: Sagar Kamble sagar.a.kam...@intel.com
Cc: damien.lesp...@intel.com
Cc: daniel.vet...@ffwll.ch
Cc: ville.syrj...@linux.intel.com
Cc: linux-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
Reviewed-by: Damien Lespiau damien.lesp...@intel.com
--
Damien
---
Documentation/DocBook/drm.tmpl | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 3adb6be..3bd8ae8 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2648,7 +2648,7 @@ void intel_crt_init(struct drm_device *dev)
td valign=top TBD/td
/tr
tr
- td rowspan=20 valign=top i915/td
+ td rowspan=21 valign=top i915/td
td rowspan=2 valign=top Generic/td
td valign=top Broadcast RGB/td
td valign=top ENUM/td
@@ -2664,6 +2664,14 @@ void intel_crt_init(struct drm_device *dev)
td valign=top TBD/td
/tr
tr
+ td rowspan=1 valign=top Plane/td
+ td valign=top “rotation”/td
+ td valign=top BITMASK/td
+ td valign=top { 0, rotate-0 }, { 2, rotate-180 }/td
+ td valign=top Plane/td
+ td valign=top TBD/td
+ /tr
+ tr
td rowspan=17 valign=top SDVO-TV/td
td valign=top “mode”/td
td valign=top ENUM/td
--
1.8.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bdw: Use timeout mode for RC6 on bdw
On Tue, 01 Jul 2014, O'Rourke, Tom Tom.O'rou...@intel.com wrote: From: Ben Widawsky [mailto:b...@bwidawsk.net] Sent: Friday, June 20, 2014 9:43 AM On Wed, Apr 09, 2014 at 11:44:06AM -0700, Tom O'Rourke wrote: Higher RC6 residency is observed using timeout mode instead of EI mode. This applies to Broadwell only. The difference is particularly noticeable with video playback. Issue: VIZ-3778 Change-Id: I62bb12e21caf19651034826b45cde7f73a80938d Signed-off-by: Tom O'Rourke Tom.O'rou...@intel.com Now that CHV is out, does it apply there too? Reviewed-by: Ben Widawsky b...@bwidawsk.net [snip] -- Ben Widawsky, Intel Open Source Technology Center [TOR:] For CHV, we expect timeout mode will provide benefit on some pre- production steppings and no benefit on the production stepping. [TOR:] Can we get this patch merged now that Ben has reviewed? Pushed to dinq, thanks for the patch and review. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] pin OABUFFER to GGTT
bo_pin had so far root privileges - they were sufficient and it would be enough to get bo_pin back the same way. This also implies that root can only start the OA buffer measurements. -Tomasz -Original Message- From: Lespiau, Damien Sent: Wednesday, July 02, 2014 12:49 PM To: Madajczak, Tomasz Cc: Mateo Lozano, Oscar; Chris Wilson; Ben Widawsky; Intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] pin OABUFFER to GGTT On Wed, Jul 02, 2014 at 10:31:45AM +, Madajczak, Tomasz wrote: There isn't any secret or privacy in that OA buffer data - just results of performance counters, shown by tools such as GPA/ Vtune. Chris alludes to the fact that if there's a way for one application to gather data about other applications (whatever kind of data), that's an automatic security concern. One may think that's only very theoretical, but the side channel attacks are (surpringly) real and effective. It cannot be proven easily that exposing data about other contexes is totally secure. There are ways around that however. If only root can gather that data, I think we've contained the issue enough for the case at hand? -- Damien smime.p7s Description: S/MIME cryptographic signature Intel Technology Poland sp. z o.o. ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN. Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione. This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Drop WA to fix Voltage not getting dropped to Vmin when Gfx is power gated for latest VLV revision
On Fri, 27 Jun 2014, Ville Syrjälä ville.syrj...@linux.intel.com wrote:
On Sat, Jun 28, 2014 at 11:26:11AM +0530, deepa...@linux.intel.com wrote:
From: Deepak S deepa...@linux.intel.com
Workaround fixed in Latest VLV revision. Forcing Gfx clk up not needed, and
Requesting the
min freq should bring bring the voltage Vnn.
v2: Drop WA for Latest VLV revision (Ville)
Signed-off-by: Deepak S deepa...@linux.intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index a90fdbd..6b6cfd4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3212,6 +3212,14 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
*/
static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
{
+struct drm_device *dev = dev_priv-dev;
+
+/* Latest VLV doesn't need Vnn WA*/
Maybe this should say Latest VLV doesn't need to force the gfx clock
or something like that. We are still doing this to reduce Vnn after all.
Apart from that this matches my observations so:
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Pushed to -fixes with the comment changed and commit message massaged a
bit. Thanks for the patch and review.
BR,
Jani.
+if (dev-pdev-revision = 0xd) {
+valleyview_set_rps(dev_priv-dev,
dev_priv-rps.min_freq_softlimit);
+return;
+}
+
/*
* When we are idle. Drop to min voltage state.
*/
--
1.9.1
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Agnostic INTEL_INFO
On Wed, 02 Jul 2014, Chris Wilson ch...@chris-wilson.co.uk wrote:
Adapt the macro so that we can pass either the struct drm_device or the
struct drm_i915_private pointers and get the answer we want.
Polymorphism?! :o
textdata bss dec hex filename
8138307 1234176 679936 10052419 996343 vmlinux.before
8137591 1234176 679936 10051703 996077 vmlinux.after
700 bytes of code saving and peace of mind. Just don't look at the
macro.
I did. I think I need to get new glasses now.
I haven't yet decided whether this is really hideous or really
cool. Maybe both.
BR,
Jani.
---
drivers/gpu/drm/i915/i915_drv.h | 55
++-
drivers/gpu/drm/i915/i915_gpu_error.c | 4 +--
drivers/gpu/drm/i915/i915_irq.c | 16 +-
drivers/gpu/drm/i915/i915_sysfs.c | 11 ---
drivers/gpu/drm/i915/intel_display.c | 54 +-
drivers/gpu/drm/i915/intel_dp.c | 2 +-
drivers/gpu/drm/i915/intel_hdmi.c | 2 +-
drivers/gpu/drm/i915/intel_i2c.c | 8 ++---
drivers/gpu/drm/i915/intel_lvds.c | 2 +-
drivers/gpu/drm/i915/intel_pm.c | 10 +++
drivers/gpu/drm/i915/intel_uncore.c | 20 ++---
11 files changed, 93 insertions(+), 91 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7838b298048c..1781889e65f0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1937,51 +1937,54 @@ struct drm_i915_cmd_table {
int count;
};
-#define INTEL_INFO(dev) (to_i915(dev)-info)
+#define __I915__(ptr)((sizeof(*(ptr)) == sizeof(struct
drm_i915_private)) ? (struct drm_i915_private *)(ptr) : to_i915((struct
drm_device *)ptr))
+#define __DRM__(ptr) ((sizeof(*(ptr)) == sizeof(struct
drm_i915_private)) ? ((struct drm_i915_private *)(ptr))-dev : (struct
drm_device *)(ptr))
+#define INTEL_INFO(dev) (__I915__(dev)-info)
+#define INTEL_DEVICE(dev)(__DRM__(dev)-pdev-device)
-#define IS_I830(dev) ((dev)-pdev-device == 0x3577)
-#define IS_845G(dev) ((dev)-pdev-device == 0x2562)
+#define IS_I830(dev) (INTEL_DEVICE(dev) == 0x3577)
+#define IS_845G(dev) (INTEL_DEVICE(dev) == 0x2562)
#define IS_I85X(dev) (INTEL_INFO(dev)-is_i85x)
-#define IS_I865G(dev)((dev)-pdev-device == 0x2572)
+#define IS_I865G(dev)(INTEL_DEVICE(dev) == 0x2572)
#define IS_I915G(dev)(INTEL_INFO(dev)-is_i915g)
-#define IS_I915GM(dev) ((dev)-pdev-device == 0x2592)
-#define IS_I945G(dev)((dev)-pdev-device == 0x2772)
+#define IS_I915GM(dev) (INTEL_DEVICE(dev) == 0x2592)
+#define IS_I945G(dev)(INTEL_DEVICE(dev) == 0x2772)
#define IS_I945GM(dev) (INTEL_INFO(dev)-is_i945gm)
#define IS_BROADWATER(dev) (INTEL_INFO(dev)-is_broadwater)
#define IS_CRESTLINE(dev)(INTEL_INFO(dev)-is_crestline)
-#define IS_GM45(dev) ((dev)-pdev-device == 0x2A42)
+#define IS_GM45(dev) (INTEL_DEVICE(dev) == 0x2A42)
#define IS_G4X(dev) (INTEL_INFO(dev)-is_g4x)
-#define IS_PINEVIEW_G(dev) ((dev)-pdev-device == 0xa001)
-#define IS_PINEVIEW_M(dev) ((dev)-pdev-device == 0xa011)
+#define IS_PINEVIEW_G(dev) (INTEL_DEVICE(dev) == 0xa001)
+#define IS_PINEVIEW_M(dev) (INTEL_DEVICE(dev) == 0xa011)
#define IS_PINEVIEW(dev) (INTEL_INFO(dev)-is_pineview)
#define IS_G33(dev) (INTEL_INFO(dev)-is_g33)
-#define IS_IRONLAKE_M(dev) ((dev)-pdev-device == 0x0046)
+#define IS_IRONLAKE_M(dev) (INTEL_DEVICE(dev) == 0x0046)
#define IS_IVYBRIDGE(dev)(INTEL_INFO(dev)-is_ivybridge)
-#define IS_IVB_GT1(dev) ((dev)-pdev-device == 0x0156 || \
- (dev)-pdev-device == 0x0152 || \
- (dev)-pdev-device == 0x015a)
-#define IS_SNB_GT1(dev) ((dev)-pdev-device == 0x0102 || \
- (dev)-pdev-device == 0x0106 || \
- (dev)-pdev-device == 0x010A)
+#define IS_IVB_GT1(dev) (INTEL_DEVICE(dev) == 0x0156 || \
+ INTEL_DEVICE(dev) == 0x0152 || \
+ INTEL_DEVICE(dev) == 0x015a)
+#define IS_SNB_GT1(dev) (INTEL_DEVICE(dev) == 0x0102 || \
+ INTEL_DEVICE(dev) == 0x0106 || \
+ INTEL_DEVICE(dev) == 0x010A)
#define IS_VALLEYVIEW(dev) (INTEL_INFO(dev)-is_valleyview)
#define IS_CHERRYVIEW(dev) (INTEL_INFO(dev)-is_valleyview IS_GEN8(dev))
#define IS_HASWELL(dev) (INTEL_INFO(dev)-is_haswell)
#define IS_BROADWELL(dev)(!INTEL_INFO(dev)-is_valleyview
IS_GEN8(dev))
#define IS_MOBILE(dev) (INTEL_INFO(dev)-is_mobile)
#define IS_HSW_EARLY_SDV(dev)(IS_HASWELL(dev) \
-
Re: [Intel-gfx] [PATCH] drm/i915: Agnostic INTEL_INFO
On Wed, Jul 02, 2014 at 03:57:08PM +0300, Jani Nikula wrote: On Wed, 02 Jul 2014, Chris Wilson ch...@chris-wilson.co.uk wrote: Adapt the macro so that we can pass either the struct drm_device or the struct drm_i915_private pointers and get the answer we want. Polymorphism?! :o textdata bss dec hex filename 8138307 1234176 679936 10052419 996343 vmlinux.before 8137591 1234176 679936 10051703 996077 vmlinux.after 700 bytes of code saving and peace of mind. Just don't look at the macro. I did. I think I need to get new glasses now. I haven't yet decided whether this is really hideous or really cool. Maybe both. I think I like my version (or a variation of the theme) http://lists.freedesktop.org/archives/dri-devel/2014-January/051496.html better, but after Daniel comment I abandoned the effort to not conflict with his work. I don't believe this moved on at all since, oh well. I would think the explicit cast from __I915__(), __DRM__() are not needed? Maybe adding a BUILD_BUG_ON() checking the sizes of the 2 structures would let us sleep at night. drm_device is unlikey to catch up any time soon though. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] Moving rotation_property to drm_plane
On Wed, Jul 02, 2014 at 02:21:52PM +0530, Jindal, Sonika wrote: Hi, Did anybody get a chance to review these patches? Ok, now those patches are all over the place. It's pretty much impossible to have the big picture again with the latest changes (the rotation on drm_plane) nor the limit of the the series, esp. as the documentation patches have been split off. I'm not sure why, but it seems like the Cc: tags in your emails have never actually ended up in the Cc: header of your mails. Which means they never hit dri-devel. Can you make sure you fix that for the next resend. This means we haven't gathered their feedback during all this time... What you can do: Use --dry-run to make sure you're sending them the dri-devel. Maybe use the --cc command line option instead? Also, as already mentioned, don't send the patches to the LKML, noone will review them there. Could you resend the full series with the reviewed-by tags gathered so far? with dri-devel in copy? in a separarte mail thread? We usually version the big resend of series in the cover letter, explaning the changes. Thanks, -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Agnostic INTEL_INFO
On Wed, Jul 02, 2014 at 02:12:08PM +0100, Damien Lespiau wrote: On Wed, Jul 02, 2014 at 03:57:08PM +0300, Jani Nikula wrote: On Wed, 02 Jul 2014, Chris Wilson ch...@chris-wilson.co.uk wrote: Adapt the macro so that we can pass either the struct drm_device or the struct drm_i915_private pointers and get the answer we want. Polymorphism?! :o textdata bss dec hex filename 8138307 1234176 679936 10052419 996343 vmlinux.before 8137591 1234176 679936 10051703 996077 vmlinux.after 700 bytes of code saving and peace of mind. Just don't look at the macro. I did. I think I need to get new glasses now. I haven't yet decided whether this is really hideous or really cool. Maybe both. I think I like my version (or a variation of the theme) http://lists.freedesktop.org/archives/dri-devel/2014-January/051496.html better, but after Daniel comment I abandoned the effort to not conflict with his work. Subclassing is the way forward. I think this is merely a stepping stone to make the code neater in the short term. I don't believe this moved on at all since, oh well. I would think the explicit cast from __I915__(), __DRM__() are not needed? They are all there to keep the compiler happy, CPP is not a true macro system after all. Maybe adding a BUILD_BUG_ON() checking the sizes of the 2 structures would let us sleep at night. drm_device is unlikey to catch up any time soon though. Indeed. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] pin OABUFFER to GGTT
Hi Chris alludes to the fact that if there's a way for one application to gather data about other applications (whatever kind of data), that's an automatic security concern. We have a few use cases for OA buffer and one application to gather data about other applications is basically a definition of at least some of them. I'm unclear why they need the offset. Can it not work like any other relocation, except with the requirement that it's in the GGTT? We read OA_TAIL_PTR register via batch buffer (SRM) and we need to subtract OA buffer offset from it to get the relative tail. Currently in most cases we write to OACONTROL, OABUFFER and other OA registers via MMIO in user mode. I suppose your answer is to move OA ownership to kernel and provide new API for this. This is fine for us as long as multiple non-root applications are able to freely access contents of OA buffer in an efficient manner. We are ok with limiting other functionalities (like setting up / programming OA) to root only. One more thing. Assuming usermode has OA buffer mapped I don't see how to avoid exposing of OA buffer offset to user mode in real life scenarios. To profile selected commands usermode driver must read OA_TAIL_PTR via SRM (so that it is written to memory together with perf counters). And it needs to know how the value read translates to offset in OA buffer. This implies usermode needs to know OA buffer offset (since the address in OA_BUFFER_PTR is not relative to the buffer boundary). Having said all this, how about restoring the pin_ioctl? At least for some time? We do have a use case and moving to other - better - solution would take time. I think backward compatibility is something that you take into consideration as well. Thanks Adam -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Madajczak, Tomasz Sent: Wednesday, July 2, 2014 1:12 PM To: Lespiau, Damien Cc: Ben Widawsky; Intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] pin OABUFFER to GGTT bo_pin had so far root privileges - they were sufficient and it would be enough to get bo_pin back the same way. This also implies that root can only start the OA buffer measurements. -Tomasz -Original Message- From: Lespiau, Damien Sent: Wednesday, July 02, 2014 12:49 PM To: Madajczak, Tomasz Cc: Mateo Lozano, Oscar; Chris Wilson; Ben Widawsky; Intel- g...@lists.freedesktop.org Subject: Re: [Intel-gfx] pin OABUFFER to GGTT On Wed, Jul 02, 2014 at 10:31:45AM +, Madajczak, Tomasz wrote: There isn't any secret or privacy in that OA buffer data - just results of performance counters, shown by tools such as GPA/ Vtune. Chris alludes to the fact that if there's a way for one application to gather data about other applications (whatever kind of data), that's an automatic security concern. One may think that's only very theoretical, but the side channel attacks are (surpringly) real and effective. It cannot be proven easily that exposing data about other contexes is totally secure. There are ways around that however. If only root can gather that data, I think we've contained the issue enough for the case at hand? -- Damien Intel Technology Poland sp. z o.o. ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN. Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione. This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot
2014-07-02 5:35 GMT-03:00 Jani Nikula jani.nik...@intel.com:
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.
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)
What if someone does a power off and _very quickly_ starts the system again? =P
insert same statement for the other code possibilities
Also, depending based on the suggestions below, you may not need the
is_edp() check (or you may want to convert it to a WARN).
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
This check is not really needed. It could also be an early return or a WARN.
+ 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));
Or pp_div = I915_READ(pp_div_reg);, since we just defined it :)
+ 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);
So this is basically turning the panel off and turning the force VDD
bit off too, and we're not putting any power domain references back.
Even though this might not be a big problem since we're shutting down
the machine anyway, did we attach a serial cable and check if this
won't print any WARNs while shutting down? Shouldn't we try to make
this function call the other functions that shut down stuff instead of
forcing the panel down here?
+ 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);
Why not put this inside intel_edp_init_connector? If you keep it here,
you also have to undo the notifier at the point
intel_dp_init_connector returns false (a few lines below). If you move
to the
Re: [Intel-gfx] [Xen-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support
On Wed, Jul 02, 2014 at 05:08:43PM +0300, Michael S. Tsirkin wrote: On Wed, Jul 02, 2014 at 10:00:33AM -0400, Konrad Rzeszutek Wilk wrote: On Wed, Jul 02, 2014 at 01:33:09PM +0200, Paolo Bonzini wrote: Il 01/07/2014 19:39, Ross Philipson ha scritto: We do IGD pass-through in our project (XenClient). The patches originally came from our project. We surface the same ISA bridge and have never had activation issues on any version of Widows from XP to Win8. We do not normally run server platforms so I can't say for sure there. The problem is not activation, the problem is that the patches are making assumptions on the driver and the firmware that might work today but are IMHO just not sane. I would have no problem with a clean patchset that adds a new machine type and doesn't touch code in -M pc, but it looks like mst disagrees. Ultimately, if a patchset is too hacky for upstream, you can include it in your downstream XenClient (and XenServer) QEMU branch. It happens. And then this discussion will come back again in a year when folks rebase and ask: Why hasn't this been done upstream. Then the discussion resumes .. With this long thread I lost a bit context about the challenges that exists. But let me try summarizing it here - which will hopefully get some consensus. Before I answer could you clarify please: by Southbridge do you mean the PCH at slot 1f or the MCH at slot 0 or both? MCH slot. We read/write from this (see intel_setup_mchbar) from couple of registers (0x44 and 0x48 if gen = 4, otherwise 0x54). It is hard-coded in the i915_get_bridge_dev (see ec2a4c3fdc8e82fe82a25d800e85c1ea06b74372) as 0:0.0 BDF. The PCH (does not matter where it sits) we only use the model:vendor id to figure out the pch_type (see intel_detect_pch). I don't see why that model:vendor_id can't be exposed via checking the type of device:vendor_id of the IGD itself. CC-ing some Intel i915 authors. So for the discussion here, when I say Southbridge I mean MCH. 1). Fix IGD hardware to not use Southbridge magic addresses. We can moan and moan but I doubt it is going to change. 2). Since we need the Southbridge magic addresses, we can expose an bridge. [I think everybody agrees that we need to do that since 1) is no go). 3). What kind of bridge. We can do: a) Two bridges - one 'passthrough' and the legacy ISA bridge that QEMU emulates. Both Linux and Windows are OK with two bridges (even thought it is pretty weird). b) One bridge - the one that QEMU emulates - and lets emulate more of the registers (by emulate - I mean for some get the data from the real hardware). b1). We can't use the legacy because the registers are above 256 (is that correct? Did I miss something?) b2) We would need to use the Q35. b2a). If we need Q35, that needs to be exposed in for Xen guests. That means exposing the MMCONFIG and restructing the E820 to fit that in. Problem: - Migration is not working with Q35. (But for v1 you wouldn't migrate, however later hardware will surely have SR-IOV so we will need to migrate). - There are no developers who have an OK from their management to focus on this. (Potential solution: Poke Intel management to see if they can get more developers on it) 4). Code does a bit of sysfs that could use some refacturing with the KVM code. Problem: More time needed to do the code restructing. Is that about correct? What are folks timezones and the best days next week to talk about this on either Google Hangout or the phone? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Linux 3.16-rc2
Am 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:
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 ring1\n,
ring-name);
+// return false;
+// }
+// }
+
+ /* Stop the ring if it's running. */
I915_WRITE_CTL(ring, 0);
I915_WRITE_HEAD(ring, 0);
ring-write_tail(ring, 0);
+ if
Re: [Intel-gfx] ResettRe: [Xen-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support
On Wed, Jul 02, 2014 at 04:50:15PM +0200, Paolo Bonzini wrote:
Il 02/07/2014 16:00, Konrad Rzeszutek Wilk ha scritto:
With this long thread I lost a bit context about the challenges
that exists. But let me try summarizing it here - which will hopefully
get some consensus.
1). Fix IGD hardware to not use Southbridge magic addresses.
We can moan and moan but I doubt it is going to change.
There are two problems:
- Northbridge (i.e. MCH i.e. PCI host bridge) configuration space addresses
Right. So in drivers/gpu/drm/i915/i915_dma.c:
1135 #define MCHBAR_I915 0x44
1136 #define MCHBAR_I965 0x48
1147 int reg = INTEL_INFO(dev)-gen = 4 ? MCHBAR_I965 : MCHBAR_I915;
1152 if (INTEL_INFO(dev)-gen = 4)
1153 pci_read_config_dword(dev_priv-bridge_dev, reg + 4,
temp_hi);
1154 pci_read_config_dword(dev_priv-bridge_dev, reg, temp_lo);
1155 mchbar_addr = ((u64)temp_hi 32) | temp_lo;
and
1139 #define DEVEN_REG 0x54
1193 int mchbar_reg = INTEL_INFO(dev)-gen = 4 ? MCHBAR_I965 :
MCHBAR_I915;
1202 if (IS_I915G(dev) || IS_I915GM(dev)) {
1203 pci_read_config_dword(dev_priv-bridge_dev, DEVEN_REG,
temp);
1204 enabled = !!(temp DEVEN_MCHBAR_EN);
1205 } else {
1206 pci_read_config_dword(dev_priv-bridge_dev, mchbar_reg,
temp);
1207 enabled = temp 1;
1208 }
- Southbridge (i.e. PCH i.e. ISA bridge) vendor/device ID; some versions of
the driver identify it by class, some versions identify it by slot (1f.0).
Right, So in drivers/gpu/drm/i915/i915_drv.c the giant intel_detect_pch
which sets the pch_type based on :
432 if (pch-vendor == PCI_VENDOR_ID_INTEL) {
433 unsigned short id = pch-device
INTEL_PCH_DEVICE_ID_MASK;
434 dev_priv-pch_id = id;
435
436 if (id == INTEL_PCH_IBX_DEVICE_ID_TYPE) {
It checks for 0x3b00, 0x1c00, 0x1e00, 0x8c00 and 0x9c00.
The INTEL_PCH_DEVICE_ID_MASK is 0xff00
To solve the first, make a new machine type, PIIX4-based, and pass through
the registers you need. The patch must document _exactly_ why the registers
are safe to pass. If they are not reserved on PIIX4, the patch must
document what the same offsets mean on PIIX4, and why it's sensible to
assume that firmware for virtual machine will not read/write them. Bonus
point for also documenting the same for Q35.
OK. They look to be related to setting up an MBAR , but I don't understand
why it is needed. Hopefully some of the i915 folks CC-ed here can answer.
Regarding the second, fixing IGD hardware to not rely on chipset magic is a
no-go, I agree. I disagree that it's a no-go to define a backdoor that
lets a hypervisor pass the right information to the driver without hacking
the chipset device model.
The hardware folks would have to give us a place for a pair of registers
(something like data/address), and a bit somewhere else that would be always
0 on hardware and always 1 if the hypervisor is implementing the pair of
registers. This is similar to CPUID, which has the HYPERVISOR bit +
hypervisor-defined leaves at 0x4000.
The data/address pair could be in a BAR, in configuration space, in the low
VGA ports at 0x3c0-0x3df, wherever. The hypervisor bit can be in the same
place or somewhere else---again, whatever is convenient for the hardware
folks. We just need *one bit* that is known-zero on all hardware, and 8
bytes in a reserved area. I don't think it's too hard to find this space,
and I really, really would like Intel to follow up on a paravirtualized
backdoor.
That said, we have the problem of existing guests, so I agree something else
is needed.
a) Two bridges - one 'passthrough' and the legacy ISA bridge
that QEMU emulates. Both Linux and Windows are OK with
two bridges (even thought it is pretty weird).
This is pretty much the only solution for existing Linux guests that look up
the southbridge by class.
Right.
The proposed solution here is to define a new pci stub device in QEMU that
lets you define a do-nothing device with your desired vendor ID, device ID,
class and optionally subsystem IDs.
nods
The new machine type (the one that instantiates the special
IGD-passthrough-enabled northbridge) can then instantiate this
Re: [Intel-gfx] ResettRe: [Xen-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support
Il 02/07/2014 18:23, Konrad Rzeszutek Wilk ha scritto:
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 651e65e..03f2829 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -433,6 +433,8 @@ void intel_detect_pch(struct drm_device *dev)
unsigned short id = pch-device
INTEL_PCH_DEVICE_ID_MASK;
dev_priv-pch_id = id;
+ if (pch-subsystem_vendor == PCI_VENDOR_ID_XEN)
+ id = pch-device INTEL_PCH_DEVICE_ID_MASK;
Actually you could look at *dev*'s subsystem IDs and skip the pch lookup
completely.
Paolo
if (id == INTEL_PCH_IBX_DEVICE_ID_TYPE) {
dev_priv-pch_type = PCH_IBX;
DRM_DEBUG_KMS(Found Ibex Peak PCH\n);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] igt/gem_partial_pwrite_pread: Add set-cache subtest to validate JIRA VIZ-3721
Hi Daniel,
Is it possible to get this patch on the review board? I've been on vacation a
couple of weeks and haven't been around to push it.
Thanks,
Armin
-Original Message-
From: Reese, Armin C
Sent: Thursday, June 12, 2014 2:19 PM
To: intel-gfx@lists.freedesktop.org
Cc: Reese, Armin C
Subject: [PATCH] igt/gem_partial_pwrite_pread: Add set-cache subtest to
validate JIRA VIZ-3721
From: Armin Reese armin.c.re...@intel.com
This subtest forces the driver down the code path in
i915_gem_object_set_cache_level() which unbinds VMAs and triggers the kernel
panic described in VIZ-3721. This test will pass if the bug fix is in place.
Bugzilla: https://bugs.fredesktop.org/show_bug.cgi?id=76384
Signed-off-by: Armin Reese armin.c.re...@intel.com
---
tests/gem_partial_pwrite_pread.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/tests/gem_partial_pwrite_pread.c b/tests/gem_partial_pwrite_pread.c
index 7945ba7..3e3ecc9 100644
--- a/tests/gem_partial_pwrite_pread.c
+++ b/tests/gem_partial_pwrite_pread.c
@@ -131,6 +131,14 @@ static void test_partial_reads(void)
}
+static void test_set_cache(void)
+{
+ igt_info(checking set-cache\n);
+
+ blt_bo_fill(staging_bo, scratch_bo, 0);
+
+}
+
static void test_partial_writes(void)
{
int i, j;
@@ -244,6 +252,9 @@ static void do_tests(int cache_level, const char *suffix)
gem_set_caching(fd, scratch_bo-handle, cache_level);
}
+ igt_subtest_f(set-cache%s, suffix)
+ test_set_cache();
+
igt_subtest_f(reads%s, suffix)
test_partial_reads();
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Xen-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support
On Wed, Jul 02, 2014 at 06:29:23PM +0200, Paolo Bonzini wrote: Il 02/07/2014 17:27, Michael S. Tsirkin ha scritto: At some level, maybe Paolo is right. Ignore existing drivers and ask intel developers to update their drivers to do something sane on hypervisors, even if they do ugly things on real hardware. A simple proposal since what I wrote earlier though apparently wasn't very clear: Detect Xen subsystem vendor id on vga card. If there, avoid poking at chipset. Instead - use subsystem device # for card type You mean for PCH type (aka PCH device id). - use second half of BAR0 of device - instead of access to pci host hypervisors will simply take BAR0 and double it in size, make second part map to what would be the pci host. Nice. Detecting the backdoor via the subsystem vendor id is clever. I'm not sure if it's possible to just double the size of BAR0 or not, but my laptop has: Region 0: Memory at d000 (64-bit, non-prefetchable) [size=4M] Region 2: Memory at c000 (64-bit, prefetchable) [size=256M] Region 4: I/O ports at 5000 [size=64] and I hope we can reserve a few KB for hypervisors within those 4M, or 8 bytes for an address/data pair (like cf8/cfc) within BAR4's 64 bytes (or grow BAR4 to 128 bytes, or something like that). Xen can still add the hacky machine type if they want for existing hosts, but this would be a nice way forward. It would be good to understand first why i915 in the first place needs to setup the bridge MBAR if it has not been set. As in, why is this region needed? Is it needed to flush the pipeline (as in you need to write there?) or .. Perhaps it is not needed anymore with the current hardware and what can be done is put a stake in the ground saying that only genX or later will be supported. The commit ids allude to power managament and the earlier commits did poke there - but I don't see it on the latest tree. Paolo ___ Xen-devel mailing list xen-de...@lists.xen.org http://lists.xen.org/xen-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ResettRe: [Xen-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support
On Wed, Jul 02, 2014 at 12:23:37PM -0400, Konrad Rzeszutek Wilk wrote:
On Wed, Jul 02, 2014 at 04:50:15PM +0200, Paolo Bonzini wrote:
Il 02/07/2014 16:00, Konrad Rzeszutek Wilk ha scritto:
With this long thread I lost a bit context about the challenges
that exists. But let me try summarizing it here - which will hopefully
get some consensus.
1). Fix IGD hardware to not use Southbridge magic addresses.
We can moan and moan but I doubt it is going to change.
There are two problems:
- Northbridge (i.e. MCH i.e. PCI host bridge) configuration space addresses
Right. So in drivers/gpu/drm/i915/i915_dma.c:
1135 #define MCHBAR_I915 0x44
1136 #define MCHBAR_I965 0x48
1147 int reg = INTEL_INFO(dev)-gen = 4 ? MCHBAR_I965 : MCHBAR_I915;
1152 if (INTEL_INFO(dev)-gen = 4)
1153 pci_read_config_dword(dev_priv-bridge_dev, reg + 4,
temp_hi);
1154 pci_read_config_dword(dev_priv-bridge_dev, reg, temp_lo);
1155 mchbar_addr = ((u64)temp_hi 32) | temp_lo;
and
1139 #define DEVEN_REG 0x54
1193 int mchbar_reg = INTEL_INFO(dev)-gen = 4 ? MCHBAR_I965 :
MCHBAR_I915;
1202 if (IS_I915G(dev) || IS_I915GM(dev)) {
1203 pci_read_config_dword(dev_priv-bridge_dev, DEVEN_REG,
temp);
1204 enabled = !!(temp DEVEN_MCHBAR_EN);
1205 } else {
1206 pci_read_config_dword(dev_priv-bridge_dev, mchbar_reg,
temp);
1207 enabled = temp 1;
1208 }
- Southbridge (i.e. PCH i.e. ISA bridge) vendor/device ID; some versions of
the driver identify it by class, some versions identify it by slot (1f.0).
Right, So in drivers/gpu/drm/i915/i915_drv.c the giant intel_detect_pch
which sets the pch_type based on :
432 if (pch-vendor == PCI_VENDOR_ID_INTEL) {
433 unsigned short id = pch-device
INTEL_PCH_DEVICE_ID_MASK;
434 dev_priv-pch_id = id;
435
436 if (id == INTEL_PCH_IBX_DEVICE_ID_TYPE) {
It checks for 0x3b00, 0x1c00, 0x1e00, 0x8c00 and 0x9c00.
The INTEL_PCH_DEVICE_ID_MASK is 0xff00
To solve the first, make a new machine type, PIIX4-based, and pass through
the registers you need. The patch must document _exactly_ why the registers
are safe to pass. If they are not reserved on PIIX4, the patch must
document what the same offsets mean on PIIX4, and why it's sensible to
assume that firmware for virtual machine will not read/write them. Bonus
point for also documenting the same for Q35.
OK. They look to be related to setting up an MBAR , but I don't understand
why it is needed. Hopefully some of the i915 folks CC-ed here can answer.
Regarding the second, fixing IGD hardware to not rely on chipset magic is a
no-go, I agree. I disagree that it's a no-go to define a backdoor that
lets a hypervisor pass the right information to the driver without hacking
the chipset device model.
The hardware folks would have to give us a place for a pair of registers
(something like data/address), and a bit somewhere else that would be always
0 on hardware and always 1 if the hypervisor is implementing the pair of
registers. This is similar to CPUID, which has the HYPERVISOR bit +
hypervisor-defined leaves at 0x4000.
The data/address pair could be in a BAR, in configuration space, in the low
VGA ports at 0x3c0-0x3df, wherever. The hypervisor bit can be in the same
place or somewhere else---again, whatever is convenient for the hardware
folks. We just need *one bit* that is known-zero on all hardware, and 8
bytes in a reserved area. I don't think it's too hard to find this space,
and I really, really would like Intel to follow up on a paravirtualized
backdoor.
That said, we have the problem of existing guests, so I agree something else
is needed.
a) Two bridges - one 'passthrough' and the legacy ISA bridge
that QEMU emulates. Both Linux and Windows are OK with
two bridges (even thought it is pretty weird).
This is pretty much the only solution for existing Linux guests that look up
the southbridge by class.
Right.
The proposed solution here is to define a new pci stub device in QEMU that
lets you define a do-nothing device with your desired vendor ID,
Re: [Intel-gfx] [PATCH 11/19] drm/i915: Precompute static ddi_pll_sel values in encoders
2014-06-25 16:01 GMT-03:00 Imre Deak imre.d...@intel.com:
From: Daniel Vetter daniel.vet...@ffwll.ch
This way only the dynamic WRPLL selection for hdmi ddi mode is
done in intel_ddi_pll_select.
v2: Don't clobber the precomputed values when selecting clocks fro
hdmi encoders.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_crt.c | 4 +++-
drivers/gpu/drm/i915/intel_ddi.c | 34 +++---
drivers/gpu/drm/i915/intel_dp.c | 23 ---
3 files changed, 26 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_crt.c
b/drivers/gpu/drm/i915/intel_crt.c
index 76ffa2c..88db4b6 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -315,8 +315,10 @@ static bool intel_crt_compute_config(struct
intel_encoder *encoder,
pipe_config-pipe_bpp = 24;
/* FDI must always be 2.7 GHz */
- if (HAS_DDI(dev))
+ if (HAS_DDI(dev)) {
+ pipe_config-ddi_pll_sel = PORT_CLK_SEL_SPLL;
pipe_config-port_clock = 135000 * 2;
+ }
return true;
}
diff --git a/drivers/gpu/drm/i915/intel_ddi.c
b/drivers/gpu/drm/i915/intel_ddi.c
index 5356e3e..6e976ba 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -403,6 +403,7 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
I915_WRITE(WRPLL_CTL1, val ~WRPLL_PLL_ENABLE);
POSTING_READ(WRPLL_CTL1);
}
+ intel_crtc-config.ddi_pll_sel = PORT_CLK_SEL_NONE;
break;
case PORT_CLK_SEL_WRPLL2:
plls-wrpll2_refcount--;
@@ -413,13 +414,12 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
I915_WRITE(WRPLL_CTL2, val ~WRPLL_PLL_ENABLE);
POSTING_READ(WRPLL_CTL2);
}
+ intel_crtc-config.ddi_pll_sel = PORT_CLK_SEL_NONE;
break;
}
WARN(plls-wrpll1_refcount 0, Invalid WRPLL1 refcount\n);
WARN(plls-wrpll2_refcount 0, Invalid WRPLL2 refcount\n);
-
- intel_crtc-config.ddi_pll_sel = PORT_CLK_SEL_NONE;
}
#define LC_FREQ 2700
@@ -739,7 +739,6 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
{
struct drm_crtc *crtc = intel_crtc-base;
struct intel_encoder *intel_encoder =
intel_ddi_get_crtc_encoder(crtc);
- struct drm_encoder *encoder = intel_encoder-base;
struct drm_i915_private *dev_priv = crtc-dev-dev_private;
struct intel_ddi_plls *plls = dev_priv-ddi_plls;
int type = intel_encoder-type;
@@ -748,26 +747,7 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
intel_ddi_put_crtc_pll(crtc);
- if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
- struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
-
- switch (intel_dp-link_bw) {
- case DP_LINK_BW_1_62:
- intel_crtc-config.ddi_pll_sel =
PORT_CLK_SEL_LCPLL_810;
- break;
- case DP_LINK_BW_2_7:
- intel_crtc-config.ddi_pll_sel =
PORT_CLK_SEL_LCPLL_1350;
- break;
- case DP_LINK_BW_5_4:
- intel_crtc-config.ddi_pll_sel =
PORT_CLK_SEL_LCPLL_2700;
- break;
- default:
- DRM_ERROR(Link bandwidth %d unsupported\n,
- intel_dp-link_bw);
- return false;
- }
-
- } else if (type == INTEL_OUTPUT_HDMI) {
+ if (type == INTEL_OUTPUT_HDMI) {
uint32_t reg, val;
unsigned p, n2, r2;
@@ -808,14 +788,6 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
plls-wrpll2_refcount++;
intel_crtc-config.ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
}
-
- } else if (type == INTEL_OUTPUT_ANALOG) {
- DRM_DEBUG_KMS(Using SPLL on pipe %c\n,
- pipe_name(pipe));
- intel_crtc-config.ddi_pll_sel = PORT_CLK_SEL_SPLL;
- } else {
- WARN(1, Invalid DDI encoder type %d\n, type);
- return false;
}
return true;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b5ec489..9a2ede0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -746,6 +746,22 @@ intel_dp_connector_unregister(struct intel_connector
*intel_connector)
}
static void
+hsw_dp_set_ddi_pll_sel(struct intel_crtc_config *pipe_config, int link_bw)
+{
+ switch (link_bw) {
+ case DP_LINK_BW_1_62:
+ pipe_config-ddi_pll_sel = PORT_CLK_SEL_LCPLL_810;
+
Re: [Intel-gfx] pin OABUFFER to GGTT
On Wed, Jul 02, 2014 at 10:31:45AM +, Madajczak, Tomasz wrote: AdTomasz: is the intention to only measure within a batch? My impression is that you wanted to maintain the OABUFFER programmed and then collect performance reports for a period of time (probably for several batchbuffers). If that´s the case, the relocation approach is not possible. I confirm that relocation approach is not possible. I'm not sure it's not possible. As long as you only need to start and stop the OA counters from one context, only one context needs to emit the reloc (with the GGTT pin flag). Nobody else needs to know the flag is present, and indeed it prevents other contexts from snooping the data. OTOH, later in the thread you say root + PIN is okay for you. That is probably easiest. OA buffer collect performance data for many hw contexts, and thus many batch buffers, it is a hardware reporting mechanism not bound to any specific GPU command send within a BB. The report is triggered by hw when: - hw context switches, that action can be caused by any level GPU preemption, GUC direct context submission, and regular kernel exec list submission - GPU frequency change - that is trigger by power management - RC6 transition - time interval expiration (if programmed so for collect performance data at regular time intervals) Each instance of UMD maps then the OA buffer to examine it within the Performance Query measurement window. Window is learned with Query::Begin and Query::End timestamps and reported OABufferTail pointer on Query::End. As OA buffer is large (up to 16MB), any KMD assist in conveying that data to UMD on each Query::End is not applicable - queries might be done per each Draw. There isn't any secret or privacy in that OA buffer data - just results of performance counters, shown by tools such as GPA/ Vtune. Thanks, Tomasz -Original Message- From: Mateo Lozano, Oscar Sent: Wednesday, July 02, 2014 10:50 AM To: Chris Wilson; Ben Widawsky Cc: Intel-gfx@lists.freedesktop.org; Madajczak, Tomasz; Rutkowski, Adam J; Jesse Barnes (jbar...@virtuousgeek.org) Subject: RE: [Intel-gfx] pin OABUFFER to GGTT - Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Wednesday, July 02, 2014 7:56 AM To: Ben Widawsky Cc: Mateo Lozano, Oscar; Intel-gfx@lists.freedesktop.org; Madajczak, Tomasz; Rutkowski, Adam J; Jesse Barnes (jbar...@virtuousgeek.org) Subject: Re: [Intel-gfx] pin OABUFFER to GGTT On Tue, Jul 01, 2014 at 11:40:52PM -0700, Ben Widawsky 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. -Chris I'm unclear why they need the offset. Can it not work like any other relocation, except with the requirement that it's in the GGTT? I'd also argue that they need to be able to map it (they need the contents, which may or may not be mapped). However, I think this is a very minor point. With the command validator this should be a pretty reasonable thing to accomplish. I think we can just give a flag for the reloc that it needs to be in the GGTT, and then pass a check to the command scanner that only that one offset is allowed, and only for OA. If the intention was to only measure within a batch and the command parser ensured that the register was cleared before the end of the batch, fine. That's not an information leak nor do we keep the hardware pointing to an unpinned buffer afterwards. Chris Wilson, Intel Open Source Technology Centre Tomasz: is the intention to only measure within a batch? My impression is that you wanted to maintain the OABUFFER programmed and then collect performance reports for a period of time (probably for several batchbuffers). If that´s the case, the relocation approach is not possible. Chris: please notice that the bo_pin ioctl they were
Re: [Intel-gfx] [RFC 05/44] drm/i915: Updating assorted register and status page definitions
On Thu, 26 Jun 2014 18:23:56 +0100 john.c.harri...@intel.com wrote: + * Premption-related registers + */ +#define RING_UHPTR(base) ((base)+0x134) +#define UHPTR_GFX_ADDR_ALIGN (0x7) +#define UHPTR_VALID(0x1) +#define RING_PREEMPT_ADDR0x0214c +#define PREEMPT_BATCH_LEVEL_MASK (0x3) +#define BB_PREEMPT_ADDR 0x02148 +#define SBB_PREEMPT_ADDR 0x0213c +#define RS_PREEMPT_STATUS0x0215c I couldn't find these easily, and the GFX_ADDR_ALIGN is just page alignment right? So you might not need that one. But overall looks fine. 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
Re: [Intel-gfx] [RFC 06/44] drm/i915: Fixes for FIFO space queries
On Thu, 26 Jun 2014 18:23:57 +0100
john.c.harri...@intel.com wrote:
From: John Harrison john.c.harri...@intel.com
The previous code was not correctly masking the value of the GTFIFOCTL
register,
leading to overruns and the message MMIO read or write has been dropped. In
addition, the checks were repeated in several different places. This commit
replaces these various checks with a simple (inline) function to encapsulate
the
read-and-mask operation. In addition, it adds a custom wait-for-fifo function
for VLV, as the timing parameters are somewhat different from those on earlier
chips.
---
drivers/gpu/drm/i915/intel_uncore.c | 49
++-
1 file changed, 42 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 871c284..6a3dddf 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -47,6 +47,12 @@ assert_device_not_suspended(struct drm_i915_private
*dev_priv)
Device suspended\n);
}
+static inline u32 fifo_free_entries(struct drm_i915_private *dev_priv)
+{
+ u32 count = __raw_i915_read32(dev_priv, GTFIFOCTL);
+ return count GT_FIFO_FREE_ENTRIES_MASK;
+}
+
static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
{
u32 gt_thread_status_mask;
@@ -154,6 +160,28 @@ static void __gen7_gt_force_wake_mt_put(struct
drm_i915_private *dev_priv,
gen6_gt_check_fifodbg(dev_priv);
}
+static int __vlv_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
+{
+ u32 free = fifo_free_entries(dev_priv);
+ int loop1, loop2;
+
+ for (loop1 = 0; loop1 5000 free GT_FIFO_NUM_RESERVED_ENTRIES; ) {
+ for (loop2 = 0; loop2 1000 free
GT_FIFO_NUM_RESERVED_ENTRIES; loop2 += 10) {
+ udelay(10);
+ free = fifo_free_entries(dev_priv);
+ }
+ loop1 += loop2;
+ if (loop1 1000 || free 48)
+ DRM_DEBUG(after %d us, the FIFO has %d slots, loop1,
free);
+ }
+
+ dev_priv-uncore.fifo_count = free;
+ if (WARN(free GT_FIFO_NUM_RESERVED_ENTRIES,
+ FIFO has insufficient space (%d slots), free))
+ return -1;
+ return 0;
+}
+
static int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
{
int ret = 0;
@@ -161,16 +189,15 @@ static int __gen6_gt_wait_for_fifo(struct
drm_i915_private *dev_priv)
/* On VLV, FIFO will be shared by both SW and HW.
* So, we need to read the FREE_ENTRIES everytime */
if (IS_VALLEYVIEW(dev_priv-dev))
- dev_priv-uncore.fifo_count =
- __raw_i915_read32(dev_priv, GTFIFOCTL)
- GT_FIFO_FREE_ENTRIES_MASK;
+ return __vlv_gt_wait_for_fifo(dev_priv);
if (dev_priv-uncore.fifo_count GT_FIFO_NUM_RESERVED_ENTRIES) {
int loop = 500;
- u32 fifo = __raw_i915_read32(dev_priv, GTFIFOCTL)
GT_FIFO_FREE_ENTRIES_MASK;
+ u32 fifo = fifo_free_entries(dev_priv);
+
while (fifo = GT_FIFO_NUM_RESERVED_ENTRIES loop--) {
udelay(10);
- fifo = __raw_i915_read32(dev_priv, GTFIFOCTL)
GT_FIFO_FREE_ENTRIES_MASK;
+ fifo = fifo_free_entries(dev_priv);
}
if (WARN_ON(loop 0 fifo = GT_FIFO_NUM_RESERVED_ENTRIES))
++ret;
@@ -194,6 +221,11 @@ static void vlv_force_wake_reset(struct drm_i915_private
*dev_priv)
static void __vlv_force_wake_get(struct drm_i915_private *dev_priv,
int fw_engine)
{
+#if 1
+ if (__gen6_gt_wait_for_fifo(dev_priv))
+ gen6_gt_check_fifodbg(dev_priv);
+#endif
+
/* Check for Render Engine */
if (FORCEWAKE_RENDER fw_engine) {
if (wait_for_atomic((__raw_i915_read32(dev_priv,
@@ -238,6 +270,10 @@ static void __vlv_force_wake_get(struct drm_i915_private
*dev_priv,
static void __vlv_force_wake_put(struct drm_i915_private *dev_priv,
int fw_engine)
{
+#if 1
+ if (__gen6_gt_wait_for_fifo(dev_priv))
+ gen6_gt_check_fifodbg(dev_priv);
+#endif
/* Check for Render Engine */
if (FORCEWAKE_RENDER fw_engine)
@@ -355,8 +391,7 @@ static void intel_uncore_forcewake_reset(struct
drm_device *dev, bool restore)
if (IS_GEN6(dev) || IS_GEN7(dev))
dev_priv-uncore.fifo_count =
- __raw_i915_read32(dev_priv, GTFIFOCTL)
- GT_FIFO_FREE_ENTRIES_MASK;
+ fifo_free_entries(dev_priv);
} else {
dev_priv-uncore.forcewake_count = 0;
dev_priv-uncore.fw_rendercount = 0;
It
Re: [Intel-gfx] [RFC 07/44] drm/i915: Disable 'get seqno' workaround for VLV
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 -- 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] [RFC 09/44] drm/i915: Start of GPU scheduler
On Thu, 26 Jun 2014 18:24:00 +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.
---
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 = 1;
+
+ dev_priv-scheduler = scheduler;
+
+ return 0;
+}
+
+#else /* CONFIG_DRM_I915_SCHEDULER */
+
+int i915_scheduler_init(struct drm_device *dev)
+{
+ return 0;
+}
+
+#endif /* CONFIG_DRM_I915_SCHEDULER */
Usually these bits are hidden in a header, and the source file isn't
compiled in if the config isn't set. But I think once we get it in,
we might just want a runtime option rather than a config option anyway,
so I'd say you could just drop the config option.
--
Jesse Barnes,
Re: [Intel-gfx] [Xen-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support
On Wed, Jul 02, 2014 at 12:05:27PM -0400, Konrad Rzeszutek Wilk wrote: On Wed, Jul 02, 2014 at 05:08:43PM +0300, Michael S. Tsirkin wrote: On Wed, Jul 02, 2014 at 10:00:33AM -0400, Konrad Rzeszutek Wilk wrote: On Wed, Jul 02, 2014 at 01:33:09PM +0200, Paolo Bonzini wrote: Il 01/07/2014 19:39, Ross Philipson ha scritto: We do IGD pass-through in our project (XenClient). The patches originally came from our project. We surface the same ISA bridge and have never had activation issues on any version of Widows from XP to Win8. We do not normally run server platforms so I can't say for sure there. The problem is not activation, the problem is that the patches are making assumptions on the driver and the firmware that might work today but are IMHO just not sane. I would have no problem with a clean patchset that adds a new machine type and doesn't touch code in -M pc, but it looks like mst disagrees. Ultimately, if a patchset is too hacky for upstream, you can include it in your downstream XenClient (and XenServer) QEMU branch. It happens. And then this discussion will come back again in a year when folks rebase and ask: Why hasn't this been done upstream. Then the discussion resumes .. With this long thread I lost a bit context about the challenges that exists. But let me try summarizing it here - which will hopefully get some consensus. Before I answer could you clarify please: by Southbridge do you mean the PCH at slot 1f or the MCH at slot 0 or both? MCH slot. We read/write from this (see intel_setup_mchbar) from couple of registers (0x44 and 0x48 if gen = 4, otherwise 0x54). It is hard-coded in the i915_get_bridge_dev (see ec2a4c3fdc8e82fe82a25d800e85c1ea06b74372) as 0:0.0 BDF. The PCH (does not matter where it sits) we only use the model:vendor id to figure out the pch_type (see intel_detect_pch). I don't see why that model:vendor_id can't be exposed via checking the type of device:vendor_id of the IGD itself. CC-ing some Intel i915 authors. So for the discussion here, when I say Southbridge I mean MCH. OK so PIIX spec says: 0x10-4F reserved. So far so good, it is likely harmless to stick something at 0x44 and 0x48 most guests will very likely just keep ticking. 0x54-0x57 deal with RAM though. Maybe we can just stick to emulating gen = 4 for now: detect it on host and fail assignment. How old is gen 4? 1). Fix IGD hardware to not use Southbridge magic addresses. We can moan and moan but I doubt it is going to change. 2). Since we need the Southbridge magic addresses, we can expose an bridge. [I think everybody agrees that we need to do that since 1) is no go). 3). What kind of bridge. We can do: a) Two bridges - one 'passthrough' and the legacy ISA bridge that QEMU emulates. Both Linux and Windows are OK with two bridges (even thought it is pretty weird). b) One bridge - the one that QEMU emulates - and lets emulate more of the registers (by emulate - I mean for some get the data from the real hardware). b1). We can't use the legacy because the registers are above 256 (is that correct? Did I miss something?) b2) We would need to use the Q35. b2a). If we need Q35, that needs to be exposed in for Xen guests. That means exposing the MMCONFIG and restructing the E820 to fit that in. Problem: - Migration is not working with Q35. (But for v1 you wouldn't migrate, however later hardware will surely have SR-IOV so we will need to migrate). - There are no developers who have an OK from their management to focus on this. (Potential solution: Poke Intel management to see if they can get more developers on it) 4). Code does a bit of sysfs that could use some refacturing with the KVM code. Problem: More time needed to do the code restructing. Is that about correct? What are folks timezones and the best days next week to talk about this on either Google Hangout or the phone? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 10/44] drm/i915: Prepare retire_requests to handle out-of-order seqnos
On Thu, 26 Jun 2014 18:24:01 +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.
---
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 @@ i915_gem_retire_requests_ring(struct intel_engine_cs
*ring)
ring-trace_irq_seqno = 0;
}
+ /* Finish processing active list before freeing request */
+ while (!list_empty(deferred_request_free)) {
+ req = list_first_entry(deferred_request_free,
+struct drm_i915_gem_request,
+list);
+
+ i915_gem_free_request(req);
+ }
+
WARN_ON(i915_verify_lists(ring-dev));
}
I think this looks ok, but I don't look at this code
Re: [Intel-gfx] [RFC 11/44] drm/i915: Added scheduler hook into i915_seqno_passed()
On Thu, 26 Jun 2014 18:24:02 +0100 john.c.harri...@intel.com wrote: +bool i915_scheduler_is_seqno_in_flight(struct intel_engine_cs *ring, + uint32_t seqno, bool *completed); + In what cases might the return value not match the completed value? I guess I'll see in a later patch... Same comment about the ifdef applies here; looks like you have some runtime checking in place too, which seems sufficient to me. -- 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] [RFC 12/44] drm/i915: Disable hardware semaphores when GPU scheduler is enabled
On Thu, 26 Jun 2014 18:24:03 +0100
john.c.harri...@intel.com wrote:
From: John Harrison john.c.harri...@intel.com
Hardware sempahores require seqno values to be continuously incrementing.
However, the scheduler's reordering of batch buffers means that the seqno
values
going through the hardware could be out of order. Thus semaphores can not be
used.
On the other hand, the scheduler superceeds the need for hardware semaphores
anyway. Having one ring stall waiting for something to complete on another
ring
is inefficient if that ring could be working on some other, independent task.
This is what the scheduler is meant to do - keep the hardware as busy as
possible by reordering batch buffers to avoid dependency stalls.
---
drivers/gpu/drm/i915/i915_drv.c |9 +
drivers/gpu/drm/i915/i915_scheduler.c |9 +
drivers/gpu/drm/i915/i915_scheduler.h |1 +
drivers/gpu/drm/i915/intel_ringbuffer.c |4
4 files changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e2bfdda..748b13a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -33,6 +33,7 @@
#include i915_drv.h
#include i915_trace.h
#include intel_drv.h
+#include i915_scheduler.h
#include linux/console.h
#include linux/module.h
@@ -468,6 +469,14 @@ void intel_detect_pch(struct drm_device *dev)
bool i915_semaphore_is_enabled(struct drm_device *dev)
{
+ /* Hardware semaphores are not compatible with the scheduler due to the
+ * seqno values being potentially out of order. However, semaphores are
+ * also not required as the scheduler will handle interring dependencies
+ * and try do so in a way that does not cause dead time on the hardware.
+ */
+ if (i915_scheduler_is_enabled(dev))
+ return 0;
+
if (INTEL_INFO(dev)-gen 6)
return false;
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c
b/drivers/gpu/drm/i915/i915_scheduler.c
index e9aa566..d9c1879 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -26,6 +26,15 @@
#include intel_drv.h
#include i915_scheduler.h
+bool i915_scheduler_is_enabled(struct drm_device *dev)
+{
+#ifdef CONFIG_DRM_I915_SCHEDULER
+ return true;
+#else
+ return false;
+#endif
+}
I think this should be:
if (dev_priv-scheduler)
return true;
return false;
instead?
Otherwise looks fine.
--
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] how to build intel-gpu-tools without cairo
Thank you so much for your reply Ying -Original Message- From: Thomas Wood [mailto:thomas.w...@intel.com] Sent: Wednesday, July 02, 2014 3:04 AM To: Liu, Ying2 Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] how to build intel-gpu-tools without cairo On 1 July 2014 19:49, Liu, Ying2 ying2@intel.com wrote: The latest intel-gpu-tools has dependency on cairo. We don’t use cairo. Is there any way to build intel-gpu-tools without cairo? Cairo has been a required dependency since version 1.2. It is used by various tests and tools and therefore there isn't a way to build intel-gpu-tools without it. Thank you so much for your help Ying ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 13/44] drm/i915: Added scheduler hook when closing DRM file handles
On Thu, 26 Jun 2014 18:24:04 +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
submission of batch buffers to the hardware. Thus it is possible for an
application to submit work, then close the DRM handle and free up all the
resources that piece of work wishes to use before the work has even been
submitted to the hardware. To prevent this, the scheduler needs to be informed
of the DRM close event so that it can force through any outstanding work
attributed to that file handle.
---
drivers/gpu/drm/i915/i915_dma.c |3 +++
drivers/gpu/drm/i915/i915_scheduler.c | 18 ++
drivers/gpu/drm/i915/i915_scheduler.h |2 ++
3 files changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 494b156..6c9ce82 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -42,6 +42,7 @@
#include linux/vga_switcheroo.h
#include linux/slab.h
#include acpi/video.h
+#include i915_scheduler.h
#include linux/pm.h
#include linux/pm_runtime.h
#include linux/oom.h
@@ -1930,6 +1931,8 @@ void i915_driver_lastclose(struct drm_device * dev)
void i915_driver_preclose(struct drm_device *dev, struct drm_file *file)
{
+ i915_scheduler_closefile(dev, file);
+
mutex_lock(dev-struct_mutex);
i915_gem_context_close(dev, file);
i915_gem_release(dev, file);
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c
b/drivers/gpu/drm/i915/i915_scheduler.c
index d9c1879..66a6568 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -78,6 +78,19 @@ bool i915_scheduler_is_seqno_in_flight(struct
intel_engine_cs *ring,
return found;
}
+int i915_scheduler_closefile(struct drm_device *dev, struct drm_file *file)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct i915_scheduler *scheduler = dev_priv-scheduler;
+
+ if (!scheduler)
+ return 0;
+
+ /* Do stuff... */
+
+ return 0;
+}
+
#else /* CONFIG_DRM_I915_SCHEDULER */
int i915_scheduler_init(struct drm_device *dev)
@@ -85,4 +98,9 @@ int i915_scheduler_init(struct drm_device *dev)
return 0;
}
+int i915_scheduler_closefile(struct drm_device *dev, struct drm_file *file)
+{
+ return 0;
+}
+
#endif /* CONFIG_DRM_I915_SCHEDULER */
diff --git a/drivers/gpu/drm/i915/i915_scheduler.h
b/drivers/gpu/drm/i915/i915_scheduler.h
index 4044b6e..95641f6 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.h
+++ b/drivers/gpu/drm/i915/i915_scheduler.h
@@ -27,6 +27,8 @@
booli915_scheduler_is_enabled(struct drm_device *dev);
int i915_scheduler_init(struct drm_device *dev);
+int i915_scheduler_closefile(struct drm_device *dev,
+ struct drm_file *file);
#ifdef CONFIG_DRM_I915_SCHEDULER
Yeah I guess the client could have passed a ref to some other process
for tracking the outstanding work, so we need to complete it.
But shouldn't that happen as part of the clearing of the outstanding
requests in i915_gem_suspend() which is called from lastclose()? We do
a gpu_idle() and retire_requests() in there already...
--
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] [RFC 14/44] drm/i915: Added getparam for GPU scheduler
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_SYNC30
+#define I915_PARAM_HAS_GPU_SCHEDULER 31
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.
Either way though:
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
Re: [Intel-gfx] [RFC 16/44] drm/i915: Alloc early seqno
On Thu, 26 Jun 2014 18:24:07 +0100
john.c.harri...@intel.com wrote:
From: John Harrison john.c.harri...@intel.com
The scheduler needs to explicitly allocate a seqno to track each submitted
batch
buffer. This must happen a long time before any commands are actually written
to
the ring.
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c |5 +
drivers/gpu/drm/i915/intel_ringbuffer.c|2 +-
drivers/gpu/drm/i915/intel_ringbuffer.h|1 +
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index ee836a6..ec274ef 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1317,6 +1317,11 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
vma-bind_vma(vma, batch_obj-cache_level, GLOBAL_BIND);
}
+ /* Allocate a seqno for this batch buffer nice and early. */
+ ret = intel_ring_alloc_seqno(ring);
+ if (ret)
+ goto err;
+
if (flags I915_DISPATCH_SECURE)
exec_start += i915_gem_obj_ggtt_offset(batch_obj);
else
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 34d6d6e..737c41b 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1662,7 +1662,7 @@ int intel_ring_idle(struct intel_engine_cs *ring)
return i915_wait_seqno(ring, seqno);
}
-static int
+int
intel_ring_alloc_seqno(struct intel_engine_cs *ring)
{
if (ring-outstanding_lazy_seqno)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 30841ea..cc92de2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -347,6 +347,7 @@ void intel_cleanup_ring_buffer(struct intel_engine_cs
*ring);
int __must_check intel_ring_begin(struct intel_engine_cs *ring, int n);
int __must_check intel_ring_cacheline_align(struct intel_engine_cs *ring);
+int __must_check intel_ring_alloc_seqno(struct intel_engine_cs *ring);
static inline void intel_ring_emit(struct intel_engine_cs *ring,
u32 data)
{
This ought to be ok even w/o the scheduler, we'll just pick up the
lazy_seqno later on rather than allocating a new one at ring_begin
right?
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
Re: [Intel-gfx] [RFC 17/44] drm/i915: Prelude to splitting i915_gem_do_execbuffer in two
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 -- 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] [RFC 18/44] drm/i915: Added scheduler debug macro
On Thu, 26 Jun 2014 18:24:09 +0100
john.c.harri...@intel.com wrote:
From: John Harrison john.c.harri...@intel.com
Added a DRM debug facility for use by the scheduler.
---
include/drm/drmP.h |7 +++
1 file changed, 7 insertions(+)
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 76ccaab..2f477c9 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -120,6 +120,7 @@ struct videomode;
#define DRM_UT_DRIVER0x02
#define DRM_UT_KMS 0x04
#define DRM_UT_PRIME 0x08
+#define DRM_UT_SCHED 0x40
What's wrong with 0x10? We should probably define these in terms of
shifts anyway, since this is just a bitmask really.
extern __printf(2, 3)
void drm_ut_debug_printk(const char *function_name,
@@ -221,10 +222,16 @@ int drm_err(const char *func, const char *format, ...);
if (unlikely(drm_debug DRM_UT_PRIME)) \
drm_ut_debug_printk(__func__, fmt, ##args); \
} while (0)
+#define DRM_DEBUG_SCHED(fmt, args...)
\
+ do {\
+ if (unlikely(drm_debug DRM_UT_SCHED)) \
+ drm_ut_debug_printk(__func__, fmt, ##args); \
+ } while (0)
#else
#define DRM_DEBUG_DRIVER(fmt, args...) do { } while (0)
#define DRM_DEBUG_KMS(fmt, args...) do { } while (0)
#define DRM_DEBUG_PRIME(fmt, args...)do { } while (0)
+#define DRM_DEBUG_SCHED(fmt, args...)do { } while (0)
#define DRM_DEBUG(fmt, arg...)do { } while (0)
#endif
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
[Intel-gfx] [PATCH 1/1] drm/i915: replace ALIGN(PAGE_SIZE) by PAGE_ALIGN
use mm.h definition
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Jani Nikula jani.nik...@linux.intel.com
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Fabian Frederick f...@skynet.be
---
drivers/gpu/drm/i915/intel_display.c | 10 +-
drivers/gpu/drm/i915/intel_fbdev.c | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5f285fb..f07cb83 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6116,8 +6116,8 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
aligned_height = intel_align_height(dev, crtc-base.primary-fb-height,
plane_config-tiled);
- plane_config-size = ALIGN(crtc-base.primary-fb-pitches[0] *
- aligned_height, PAGE_SIZE);
+ plane_config-size = PAGE_ALIGN(crtc-base.primary-fb-pitches[0] *
+ aligned_height);
DRM_DEBUG_KMS(pipe/plane %d/%d with fb: size=%dx%d@%d, offset=%x,
pitch %d, size 0x%x\n,
pipe, plane, crtc-base.primary-fb-width,
@@ -7136,8 +7136,8 @@ static void ironlake_get_plane_config(struct intel_crtc
*crtc,
aligned_height = intel_align_height(dev, crtc-base.primary-fb-height,
plane_config-tiled);
- plane_config-size = ALIGN(crtc-base.primary-fb-pitches[0] *
- aligned_height, PAGE_SIZE);
+ plane_config-size = PAGE_ALIGN(crtc-base.primary-fb-pitches[0] *
+ aligned_height);
DRM_DEBUG_KMS(pipe/plane %d/%d with fb: size=%dx%d@%d, offset=%x,
pitch %d, size 0x%x\n,
pipe, plane, crtc-base.primary-fb-width,
@@ -8233,7 +8233,7 @@ static u32
intel_framebuffer_size_for_mode(struct drm_display_mode *mode, int bpp)
{
u32 pitch = intel_framebuffer_pitch_for_width(mode-hdisplay, bpp);
- return ALIGN(pitch * mode-vdisplay, PAGE_SIZE);
+ return PAGE_ALIGN(pitch * mode-vdisplay);
}
static struct drm_framebuffer *
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index 088fe93..182fbc2 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -81,7 +81,7 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
sizes-surface_depth);
size = mode_cmd.pitches[0] * mode_cmd.height;
- size = ALIGN(size, PAGE_SIZE);
+ size = PAGE_ALIGN(size);
obj = i915_gem_object_create_stolen(dev, size);
if (obj == NULL)
obj = i915_gem_alloc_object(dev, size);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/2] drm/i915: provide interface for audio driver to query cdclk
From: Jani Nikula jani.nik...@intel.com
Signed-off-by: Jani Nikula jani.nik...@intel.com
Signed-off-by: Mengdong Lin mengdong@intel.com
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a90fdbd..21170e5 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6256,6 +6256,27 @@ int i915_release_power_well(void)
}
EXPORT_SYMBOL_GPL(i915_release_power_well);
+/*
+ * Private interface for the audio driver to get CDCLK in kHz.
+ *
+ * Caller must request power well using i915_request_power_well() prior to
+ * making the call.
+ */
+int i915_get_cdclk_freq(void)
+{
+ struct drm_i915_private *dev_priv;
+
+ if (!hsw_pwr)
+ return -ENODEV;
+
+ dev_priv = container_of(hsw_pwr, struct drm_i915_private,
+ power_domains);
+
+ return intel_ddi_get_cdclk_freq(dev_priv);
+}
+EXPORT_SYMBOL_GPL(i915_get_cdclk_freq);
+
+
#define POWER_DOMAIN_MASK (BIT(POWER_DOMAIN_NUM) - 1)
#define HSW_ALWAYS_ON_POWER_DOMAINS ( \
diff --git a/include/drm/i915_powerwell.h b/include/drm/i915_powerwell.h
index 2baba99..baa6f11 100644
--- a/include/drm/i915_powerwell.h
+++ b/include/drm/i915_powerwell.h
@@ -32,5 +32,6 @@
/* For use by hda_i915 driver */
extern int i915_request_power_well(void);
extern int i915_release_power_well(void);
+extern int i915_get_cdclk_freq(void);
#endif /* _I915_POWERWELL_H_ */
--
1.8.1.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
