[Intel-gfx] [PATCH v2 2/8] drm/i915: Adds graphic address space ballooning logic
In XenGT, the global graphic memory space is partitioned by multiple
vgpu instances in different VMs. The ballooning code is added in
i915_gem_setup_global_gtt(), utilizing the drm mm allocator APIs to
mark the graphic address space which are partitioned out to other
vgpus as reserved.
Signed-off-by: Yu Zhang
Signed-off-by: Jike Song
Signed-off-by: Zhi Wang
Signed-off-by: Eddie Dong
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 144 +++-
1 file changed, 141 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 39c2d13..90757ab 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -31,6 +31,134 @@
#include "i915_trace.h"
#include "intel_drv.h"
+struct _balloon_info_ {
+ /*
+* There are up to 2 regions per low/high GM that
+* might be ballooned. Here, index 0/1 is for low
+* GM, 2/3 for high GM.
+*/
+ struct drm_mm_node space[4];
+} bl_info;
+
+void intel_vgt_deballoon(void)
+{
+ int i;
+
+ DRM_INFO("VGT deballoon.\n");
+
+ for (i = 0; i < 4; i++) {
+ if (bl_info.space[i].allocated)
+ drm_mm_remove_node(&bl_info.space[i]);
+ }
+
+ memset(&bl_info, 0, sizeof(bl_info));
+}
+
+static int vgt_balloon_space(struct drm_mm *mm,
+struct drm_mm_node *node,
+unsigned long start, unsigned long end)
+{
+ unsigned long size = end - start;
+
+ if (start == end)
+ return -EINVAL;
+
+ DRM_INFO("balloon space: range [ 0x%lx - 0x%lx ] %lu KB.\n",
+start, end, size / 1024);
+
+ node->start = start;
+ node->size = size;
+
+ return drm_mm_reserve_node(mm, node);
+}
+
+static int intel_vgt_balloon(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct i915_address_space *ggtt_vm = &dev_priv->gtt.base;
+ unsigned long ggtt_vm_end = ggtt_vm->start + ggtt_vm->total;
+
+ unsigned long low_gm_base, low_gm_size, low_gm_end;
+ unsigned long high_gm_base, high_gm_size, high_gm_end;
+ int ret;
+
+ low_gm_base = I915_READ(vgtif_reg(avail_rs.low_gmadr.my_base));
+ low_gm_size = I915_READ(vgtif_reg(avail_rs.low_gmadr.my_size));
+ high_gm_base = I915_READ(vgtif_reg(avail_rs.high_gmadr.my_base));
+ high_gm_size = I915_READ(vgtif_reg(avail_rs.high_gmadr.my_size));
+
+ low_gm_end = low_gm_base + low_gm_size;
+ high_gm_end = high_gm_base + high_gm_size;
+
+ DRM_INFO("VGT ballooning configuration:\n");
+ DRM_INFO("Low GM: base 0x%lx size %ldKB\n",
+low_gm_base, low_gm_size / 1024);
+ DRM_INFO("High GM: base 0x%lx size %ldKB\n",
+high_gm_base, high_gm_size / 1024);
+
+ if (low_gm_base < ggtt_vm->start
+ || low_gm_end > dev_priv->gtt.mappable_end
+ || high_gm_base < dev_priv->gtt.mappable_end
+ || high_gm_end > ggtt_vm_end) {
+ DRM_ERROR("Invalid ballooning configuration!\n");
+ return -EINVAL;
+ }
+
+ memset(&bl_info, 0, sizeof(bl_info));
+
+ /* High GM ballooning */
+ if (high_gm_base > dev_priv->gtt.mappable_end) {
+ ret = vgt_balloon_space(&ggtt_vm->mm,
+ &bl_info.space[2],
+ dev_priv->gtt.mappable_end,
+ high_gm_base);
+
+ if (ret)
+ goto err;
+ }
+
+ /*
+* No need to partition out the last physical page,
+* because it is reserved to the guard page.
+*/
+ if (high_gm_end < ggtt_vm_end - PAGE_SIZE) {
+ ret = vgt_balloon_space(&ggtt_vm->mm,
+ &bl_info.space[3],
+ high_gm_end,
+ ggtt_vm_end - PAGE_SIZE);
+ if (ret)
+ goto err;
+ }
+
+ /* Low GM ballooning */
+ if (low_gm_base > ggtt_vm->start) {
+ ret = vgt_balloon_space(&ggtt_vm->mm,
+ &bl_info.space[0],
+ ggtt_vm->start, low_gm_base);
+
+ if (ret)
+ goto err;
+ }
+
+ if (low_gm_end < dev_priv->gtt.mappable_end) {
+ ret = vgt_balloon_space(&ggtt_vm->mm,
+ &bl_info.space[1],
+ low_gm_end,
+ dev_priv->gtt.mappable_end);
+
+ if (ret)
+ goto err;
+ }
+
+ DRM_INFO("VGT balloon successfully\n");
+ return 0;
+
+err:
+ DRM_ERROR("VGT balloon fail\n");
+ intel_vgt_deballoon();
+ re
[Intel-gfx] [PATCH v2 5/8] drm/i915: Add the display switch logic for vgpu in i915 driver
Display switch logic is added to notify the vgt mediator that
current vgpu have a valid surface to show. It does so by writing
the display_ready field in PV INFO page, and then will be handled
in vgt mediator. This is useful to avoid trickiness when the VM's
framebuffer is being accessed in the middle of VM modesetting,
e.g. compositing the framebuffer in the host side.
Signed-off-by: Yu Zhang
Signed-off-by: Jike Song
Signed-off-by: Zhiyuan Lv
---
drivers/gpu/drm/i915/i915_dma.c| 11 +++
drivers/gpu/drm/i915/i915_vgt_if.h | 8
2 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 85d14e1..3a42b11 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -35,6 +35,7 @@
#include
#include "intel_drv.h"
#include
+#include "i915_vgt_if.h"
#include "i915_drv.h"
#include "i915_trace.h"
#include
@@ -1780,6 +1781,16 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
dev_priv->ums.mm_suspended = 1;
}
+ if (intel_vgpu_active(dev)) {
+ /*
+* Notify a valid surface after modesetting,
+* when running inside a VM.
+*/
+ struct drm_i915_private *dev_priv = to_i915(dev);
+
+ I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY);
+ }
+
i915_setup_sysfs(dev);
if (INTEL_INFO(dev)->num_pipes) {
diff --git a/drivers/gpu/drm/i915/i915_vgt_if.h
b/drivers/gpu/drm/i915/i915_vgt_if.h
index fa45d28..633f98a 100644
--- a/drivers/gpu/drm/i915/i915_vgt_if.h
+++ b/drivers/gpu/drm/i915/i915_vgt_if.h
@@ -82,4 +82,12 @@ struct vgt_if {
#define vgtif_reg(x) \
(VGT_PVINFO_PAGE + (long)&((struct vgt_if *) NULL)->x)
+/*
+ * The information set by the guest gfx driver, through the display_ready field
+ */
+enum vgt_display_status {
+ VGT_DRV_DISPLAY_NOT_READY = 0,
+ VGT_DRV_DISPLAY_READY, /* ready for display switch */
+};
+
#endif /* _I915_VGT_IF_H_ */
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 4/8] drm/i915: Disable framebuffer compression for i915 driver in VM
Framebuffer compression is disabled when driver detects it's running in XenGT VM, because XenGT does not provide emulations for FBC related operations, and we do not expose stolen memory to the VM. Signed-off-by: Yu Zhang Signed-off-by: Jike Song Signed-off-by: Zhiyuan Lv --- drivers/gpu/drm/i915/intel_pm.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index daa99e7..50cf96b 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -544,6 +544,10 @@ void intel_update_fbc(struct drm_device *dev) return; } + /* disable framebuffer compression in vgt */ + if (intel_vgpu_active(dev)) + i915.enable_fbc = 0; + /* * If FBC is already on, we just have to verify that we can * keep it that way... -- 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/8] drm/i915: Introduce a PV INFO page structure for Intel GVT-g.
Introduce a PV INFO structure, to facilitate the Intel GVT-g
technology, which is a GPU virtualization solution with mediated
pass-through(previously known as XenGT). This page contains the
shared information between i915 driver and the mediator. For now,
this structure utilizes an area of 4K bypes on HSW GPU's unused
MMIO space to support existing production. Future hardware will
have the reserved window architecturally defined, and layout of
the page will be added in future BSpec.
The i915 driver load routine detects if it is running in a VM by
reading the contents of this PV INFO page. If true, the pointer,
vgpu.vgt_info is initialized, and intel_vgpu_active() is used by
checking this pointer to conclude if gpu is virtualized with Intel
GVT-g. By now, it will return true only when the driver is running
in the XenGT environment on HSW.
Signed-off-by: Yu Zhang
Signed-off-by: Jike Song
Signed-off-by: Eddie Dong
---
drivers/gpu/drm/i915/i915_drv.h | 12 ++
drivers/gpu/drm/i915/i915_gem_gtt.c | 26
drivers/gpu/drm/i915/i915_vgt_if.h | 85 +
drivers/gpu/drm/i915/intel_uncore.c | 2 +
4 files changed, 125 insertions(+)
create mode 100644 drivers/gpu/drm/i915/i915_vgt_if.h
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ac6232b..ef6dadd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1448,6 +1448,10 @@ struct i915_frontbuffer_tracking {
unsigned flip_bits;
};
+struct i915_virtual_gpu {
+ bool active;
+};
+
struct drm_i915_private {
struct drm_device *dev;
struct kmem_cache *slab;
@@ -1460,6 +1464,8 @@ struct drm_i915_private {
struct intel_uncore uncore;
+ struct i915_virtual_gpu vgpu;
+
struct intel_gmbus gmbus[GMBUS_NUM_PORTS];
@@ -2303,6 +2309,12 @@ extern void intel_uncore_check_errors(struct drm_device
*dev);
extern void intel_uncore_fini(struct drm_device *dev);
extern void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore);
+extern void i915_check_vgpu(struct drm_device *dev);
+static inline bool intel_vgpu_active(struct drm_device *dev)
+{
+ return to_i915(dev)->vgpu.active;
+}
+
void
i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
u32 status_mask);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index e0bcba0..39c2d13 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -26,10 +26,36 @@
#include
#include
#include
+#include "i915_vgt_if.h"
#include "i915_drv.h"
#include "i915_trace.h"
#include "intel_drv.h"
+void i915_check_vgpu(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ uint64_t magic;
+ uint32_t version;
+
+ BUILD_BUG_ON(sizeof(struct vgt_if) != VGT_PVINFO_SIZE);
+
+ if (!IS_HASWELL(dev))
+ return;
+
+ magic = readq(dev_priv->regs + vgtif_reg(magic));
+ if (magic != VGT_MAGIC)
+ return;
+
+ version = INTEL_VGT_IF_VERSION_ENCODE(
+ readw(dev_priv->regs + vgtif_reg(version_major)),
+ readw(dev_priv->regs + vgtif_reg(version_minor)));
+ if (version != INTEL_VGT_IF_VERSION)
+ return;
+
+ dev_priv->vgpu.active = true;
+ DRM_INFO("Virtual GPU for Intel GVT-g detected.\n");
+}
+
static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv);
static void chv_setup_private_ppat(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_vgt_if.h
b/drivers/gpu/drm/i915/i915_vgt_if.h
new file mode 100644
index 000..fa45d28
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_vgt_if.h
@@ -0,0 +1,85 @@
+/*
+ * Interface between Gfx driver and vgt mediator for Intel GVT-g
+ *
+ * Copyright(c) 2011-2014 Intel Corporation. All rights reserved.
+ *
+ * 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,
[Intel-gfx] [PATCH v2 8/8] drm/i915: Support alias ppgtt in VM if ppgtt is enabled
The current XenGT only supports alias ppgtt. And the emulation
is done in XenGT host by first trapping PP_DIR_BASE mmio
accesses. Updating PP_DIR_BASE by using instructions such as
MI_LOAD_REGISTER_IMM are hard to detect and are not supported
in current XenGT. Therefore this patch also added a new callback
routine - vgpu_mm_switch() to set the PP_DIR_BASE by mmio writes.
Signed-off-by: Yu Zhang
Signed-off-by: Jike Song
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 90757ab..a731896 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -197,6 +197,9 @@ static int sanitize_enable_ppgtt(struct drm_device *dev,
int enable_ppgtt)
if (IS_GEN8(dev))
has_full_ppgtt = false; /* XXX why? */
+ if (intel_vgpu_active(dev))
+ has_full_ppgtt = false; /* emulation is too hard */
+
if (enable_ppgtt == 0 || !has_aliasing_ppgtt)
return 0;
@@ -886,6 +889,16 @@ static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt,
return 0;
}
+static int vgpu_mm_switch(struct i915_hw_ppgtt *ppgtt,
+struct intel_engine_cs *ring)
+{
+ struct drm_i915_private *dev_priv = to_i915(ppgtt->base.dev);
+
+ I915_WRITE(RING_PP_DIR_DCLV(ring), PP_DIR_DCLV_2G);
+ I915_WRITE(RING_PP_DIR_BASE(ring), get_pd_offset(ppgtt));
+ return 0;
+}
+
static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt,
struct intel_engine_cs *ring)
{
@@ -1212,6 +1225,9 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
} else
BUG();
+ if (intel_vgpu_active(dev))
+ ppgtt->switch_mm = vgpu_mm_switch;
+
ret = gen6_ppgtt_alloc(ppgtt);
if (ret)
return ret;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/8] drm/i915: Partition the fence registers for vgpu in i915 driver
In XenGT, the fence registers are partitioned by multiple vgpu instances in different VMs. Routine i915_gem_load() is modified to reset the num_fence_regs, when the driver detects it's runing in a VM. And the allocated fence numbers is provided in PV INFO page structure. Signed-off-by: Yu Zhang Signed-off-by: Jike Song Signed-off-by: Eddie Dong --- drivers/gpu/drm/i915/i915_gem.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index e9c783d..a0eec59 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -28,6 +28,7 @@ #include #include #include +#include "i915_vgt_if.h" #include "i915_drv.h" #include "i915_trace.h" #include "intel_drv.h" @@ -4988,6 +4989,10 @@ i915_gem_load(struct drm_device *dev) else dev_priv->num_fence_regs = 8; + if (intel_vgpu_active(dev)) + dev_priv->num_fence_regs = + I915_READ(vgtif_reg(avail_rs.fence_num)); + /* Initialize fence registers to zero */ INIT_LIST_HEAD(&dev_priv->mm.fence_list); i915_gem_restore_fences(dev); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 7/8] drm/i915: Create vgpu specific write MMIO to reduce traps
In the virtualized environment, forcewake operations are not
necessory for the driver, because mmio accesses will be trapped
and emulated by the host side, and real forcewake operations are
also done in the host. New mmio write handlers are added to directly
call the __raw_i915_write, therefore will reduce many traps and
increase the overall performance for drivers runing in the VM
with Intel GVT-g enhancement.
Signed-off-by: Yu Zhang
Signed-off-by: Jike Song
Signed-off-by: Kevin Tian
---
drivers/gpu/drm/i915/intel_uncore.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index d5f39f3..ec6d5ce 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -719,6 +719,14 @@ hsw_write##x(struct drm_i915_private *dev_priv, off_t reg,
u##x val, bool trace)
REG_WRITE_FOOTER; \
}
+#define __vgpu_write(x) \
+static void \
+vgpu_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool
trace) { \
+ REG_WRITE_HEADER; \
+ __raw_i915_write##x(dev_priv, reg, val); \
+ REG_WRITE_FOOTER; \
+}
+
static const u32 gen8_shadowed_regs[] = {
FORCEWAKE_MT,
GEN6_RPNSWREQ,
@@ -813,6 +821,10 @@ __gen4_write(8)
__gen4_write(16)
__gen4_write(32)
__gen4_write(64)
+__vgpu_write(8)
+__vgpu_write(16)
+__vgpu_write(32)
+__vgpu_write(64)
#undef __chv_write
#undef __gen8_write
@@ -820,6 +832,7 @@ __gen4_write(64)
#undef __gen6_write
#undef __gen5_write
#undef __gen4_write
+#undef __vgpu_write
#undef REG_WRITE_FOOTER
#undef REG_WRITE_HEADER
@@ -950,6 +963,13 @@ void intel_uncore_init(struct drm_device *dev)
dev_priv->uncore.funcs.mmio_readq = gen4_read64;
break;
}
+
+ if (intel_vgpu_active(dev)) {
+ dev_priv->uncore.funcs.mmio_writeb = vgpu_write8;
+ dev_priv->uncore.funcs.mmio_writew = vgpu_write16;
+ dev_priv->uncore.funcs.mmio_writel = vgpu_write32;
+ dev_priv->uncore.funcs.mmio_writeq = vgpu_write64;
+ }
}
void intel_uncore_fini(struct drm_device *dev)
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 0/8] Add enlightenments for vGPU
Intel GVT-g (previously known as XenGT), is a complete GPU virtualization solution with mediated pass-through for 4th generation Intel Core processors - Haswell platform. This technology presents a virtual full-fledged GPU to each Virtual Machine (VM). VMs can directly access performance-critical resources, without intervention from the hypervisor in most cases, while privileged operations from VMs are trap-and-emulated at minimal cost. For detail, please refer to https://01.org/xen/blogs/wangbo85/2014/intel-gvt-gxengt-pubic-release This patch set includes necessary code changes when i915 driver runs inside a VM. Though ideally we can run an unmodified i915 driver in VM, adding such enlightenments can greatly reduce the virtualization complexity in orders of magnitude. Code changes for the host side, which includes the actual Intel GVT-g implementation, were sent out in another patchset. The primary change introduced here is to implement so-called "address space ballooning" technique. XenGT partitions global graphics memory among multiple VMs, so each VM can directly access a portion of the memory w/o hypervisor's intervention, e.g. filling textures and queuing commands. However w/ the partitioning an unmodified i915 driver would assume a smaller graphics memory starting from address ZERO, so requires XenGT core module (vgt) to translate the graphics address between 'guest view' and 'host view', for all registers and command opcodes which contain a graphics memory address. To reduce the complexity, XenGT introduces "address space ballooning", by telling the exact partitioning knowledge to each guest i915 driver, which then reserves and prevents non-allocated portions from allocation. Then vgt module only needs to scan and validate graphics addresses w/o complexity of translation. Note: The partitioning of global graphics memory may break some applications, with large objects in the aperture, because current userspace assumes half of the aperture usable. That would need separate fix either in user space (e.g. remove assumption in mesa) or in kernel (w/ some faulting mechanism). The partitioning knowledge is conveyed through a reserved MMIO range, called PVINFO, which will be architecturally reserved in future hardware generations. Another information carried through PVINFO is about the number of fence registers. As a global resource XenGT also partitions them among VMs. Other changes are trivial as optimizations, to either reduce the trap overhead or disable power management features which don't make sense in a virtualized environment. Yu Zhang (8): drm/i915: Introduce a PV INFO page structure for Intel GVT-g. drm/i915: Adds graphic address space ballooning logic drm/i915: Partition the fence registers for vgpu in i915 driver drm/i915: Disable framebuffer compression for i915 driver in VM drm/i915: Add the display switch logic for vgpu in i915 driver drm/i915: Disable power management for i915 driver in VM drm/i915: Create vgpu specific write MMIO to reduce traps drm/i915: Support alias ppgtt in VM if ppgtt is enabled drivers/gpu/drm/i915/i915_dma.c | 11 +++ drivers/gpu/drm/i915/i915_drv.h | 12 +++ drivers/gpu/drm/i915/i915_gem.c | 5 + drivers/gpu/drm/i915/i915_gem_gtt.c | 186 +++- drivers/gpu/drm/i915/i915_vgt_if.h | 93 ++ drivers/gpu/drm/i915/intel_pm.c | 8 ++ drivers/gpu/drm/i915/intel_uncore.c | 22 + 7 files changed, 334 insertions(+), 3 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_vgt_if.h -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] tests/gem_exec_parse: fix batch_len setting for cmd-crossing-page
From: Brad Volkin
The size of the batch buffer passed to the kernel is significantly
larger than the size of the batch buffer passed to the function. A
proposed optimization as part of the batch copy kernel series is to
use batch_len for the copy and parse operations, which leads to a
false "batch without MI_BATCH_BUFFER_END" failure for this test.
Signed-off-by: Brad Volkin
---
tests/gem_exec_parse.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index 05f271c..568bd4a 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -144,9 +144,10 @@ static void exec_split_batch(int fd, uint32_t *cmds,
struct drm_i915_gem_exec_object2 objs[1];
uint32_t cmd_bo;
uint32_t noop[1024] = { 0 };
+ const int alloc_size = 4096 * 2;
// Allocate and fill a 2-page batch with noops
- cmd_bo = gem_create(fd, 4096 * 2);
+ cmd_bo = gem_create(fd, alloc_size);
gem_write(fd, cmd_bo, 0, noop, sizeof(noop));
gem_write(fd, cmd_bo, 4096, noop, sizeof(noop));
@@ -167,7 +168,7 @@ static void exec_split_batch(int fd, uint32_t *cmds,
execbuf.buffers_ptr = (uintptr_t)objs;
execbuf.buffer_count = 1;
execbuf.batch_start_offset = 0;
- execbuf.batch_len = size;
+ execbuf.batch_len = alloc_size;
execbuf.cliprects_ptr = 0;
execbuf.num_cliprects = 0;
execbuf.DR1 = 0;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] tests/gem_exec_parse: test for chained batch buffers
From: Brad Volkin
libva makes extensive use of chained batch buffers. The batch
buffer copy portion of the command parser has the potential to
break chained batches, so add a simple test to make sure that
doesn't happen.
Signed-off-by: Brad Volkin
---
lib/intel_reg.h| 1 +
tests/gem_exec_parse.c | 105 +
2 files changed, 106 insertions(+)
diff --git a/lib/intel_reg.h b/lib/intel_reg.h
index f0fc5fd..fcc9d7c 100644
--- a/lib/intel_reg.h
+++ b/lib/intel_reg.h
@@ -2571,6 +2571,7 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
#define MI_BATCH_BUFFER_END(0xA << 23)
#define MI_BATCH_NON_SECURE(1)
#define MI_BATCH_NON_SECURE_I965 (1 << 8)
+#define MI_BATCH_NON_SECURE_HSW(1<<13) /* Additional bit for
RCS */
#define MAX_DISPLAY_PIPES 2
diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index 568bd4a..3ff6a66 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -183,6 +183,96 @@ static void exec_split_batch(int fd, uint32_t *cmds,
gem_close(fd, cmd_bo);
}
+static void exec_batch_chained(int fd, uint32_t cmd_bo, uint32_t *cmds,
+ int size, int patch_offset,
+ uint64_t expected_value)
+{
+ struct drm_i915_gem_execbuffer2 execbuf;
+ struct drm_i915_gem_exec_object2 objs[3];
+ struct drm_i915_gem_relocation_entry reloc;
+ struct drm_i915_gem_relocation_entry first_level_reloc;
+
+ uint32_t target_bo = gem_create(fd, 4096);
+ uint32_t first_level_bo = gem_create(fd, 4096);
+ uint64_t actual_value = 0;
+
+ static uint32_t first_level_cmds[] = {
+ MI_BATCH_BUFFER_START | MI_BATCH_NON_SECURE_I965,
+ 0,
+ MI_BATCH_BUFFER_END,
+ 0,
+ };
+
+ if (IS_HASWELL(intel_get_drm_devid(fd)))
+ first_level_cmds[0] |= MI_BATCH_NON_SECURE_HSW;
+
+ gem_write(fd, first_level_bo, 0,
+ first_level_cmds, sizeof(first_level_cmds));
+ gem_write(fd, cmd_bo, 0, cmds, size);
+
+ reloc.offset = patch_offset;
+ reloc.delta = 0;
+ reloc.target_handle = target_bo;
+ reloc.read_domains = I915_GEM_DOMAIN_RENDER;
+ reloc.write_domain = I915_GEM_DOMAIN_RENDER;
+ reloc.presumed_offset = 0;
+
+ first_level_reloc.offset = 4;
+ first_level_reloc.delta = 0;
+ first_level_reloc.target_handle = cmd_bo;
+ first_level_reloc.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
+ first_level_reloc.write_domain = 0;
+ first_level_reloc.presumed_offset = 0;
+
+ objs[0].handle = target_bo;
+ objs[0].relocation_count = 0;
+ objs[0].relocs_ptr = 0;
+ objs[0].alignment = 0;
+ objs[0].offset = 0;
+ objs[0].flags = 0;
+ objs[0].rsvd1 = 0;
+ objs[0].rsvd2 = 0;
+
+ objs[1].handle = cmd_bo;
+ objs[1].relocation_count = 1;
+ objs[1].relocs_ptr = (uintptr_t)&reloc;
+ objs[1].alignment = 0;
+ objs[1].offset = 0;
+ objs[1].flags = 0;
+ objs[1].rsvd1 = 0;
+ objs[1].rsvd2 = 0;
+
+ objs[2].handle = first_level_bo;
+ objs[2].relocation_count = 1;
+ objs[2].relocs_ptr = (uintptr_t)&first_level_reloc;
+ objs[2].alignment = 0;
+ objs[2].offset = 0;
+ objs[2].flags = 0;
+ objs[2].rsvd1 = 0;
+ objs[2].rsvd2 = 0;
+
+ execbuf.buffers_ptr = (uintptr_t)objs;
+ execbuf.buffer_count = 3;
+ execbuf.batch_start_offset = 0;
+ execbuf.batch_len = sizeof(first_level_cmds);
+ execbuf.cliprects_ptr = 0;
+ execbuf.num_cliprects = 0;
+ execbuf.DR1 = 0;
+ execbuf.DR4 = 0;
+ execbuf.flags = I915_EXEC_RENDER;
+ i915_execbuffer2_set_context_id(execbuf, 0);
+ execbuf.rsvd2 = 0;
+
+ gem_execbuf(fd, &execbuf);
+ gem_sync(fd, cmd_bo);
+
+ gem_read(fd,target_bo, 0, &actual_value, sizeof(actual_value));
+ igt_assert_eq(expected_value, actual_value);
+
+ gem_close(fd, first_level_bo);
+ gem_close(fd, target_bo);
+}
+
uint32_t handle;
int fd;
@@ -366,6 +456,21 @@ igt_main
-EINVAL);
}
+ igt_subtest("chained-batch") {
+ uint32_t pc[] = {
+ GFX_OP_PIPE_CONTROL,
+ PIPE_CONTROL_QW_WRITE,
+ 0, // To be patched
+ 0x1200,
+ 0,
+ MI_BATCH_BUFFER_END,
+ };
+ exec_batch_chained(fd, handle,
+ pc, sizeof(pc),
+ 8, // patch offset,
+ 0x1200);
+ }
+
igt_fixture {
gem_close(fd, handle);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://
Re: [Intel-gfx] [PATCH] drm/i915: run intel_uncore_early_sanitize earlier on resume
2014-10-10 8:25 GMT-03:00 Imre Deak :
> On Thu, 2014-10-09 at 14:46 -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni
>>
>> As far as I understand, intel_uncore_early_sanitize() was supposed to
>> be ran before any register access, but currently
>> intel_resume_prepare() is ran earlier, and it does register
>> access. I don't think it should be safe to be calling
>> I915_{READ,WRITE} without calling intel_uncore_early_sanitize() first.
>>
>> One of the problems we currently have is that when we suspend/resume
>> BDW, the FPGA_DBG_RM_NOCLAIM bit becomes 1, so we end up printing an
>> "unclaimed register" message on resume, but this message doesn't
>> really seem to have been triggered by our driver or user space, since
>> the bit was not there before suspending, and gets there just after
>> resuming, before any of our own register accesses. So calling
>> intel_uncore_early_sanitize() as a first thing will allow us to stop
>> printing the error message, fixing the "bug".
>
> One issue is that intel_uncore_early_sanitize() uses forcewake, which is
> enabled only in prepare resume for VLV (vlv_allow_gt_wake()). Maybe
> FPGA_DBG could be reset separately before the rest?
It doesn't "use" forcewake, it calls intel_uncore_forcewake_reset().
Shouldn't we move the vlv code (e.g., vlv_allow_gt_wake()) to inside
intel_uncore_forcewake_reset() instead?
To my understanding, it doesn't make sense to call anything at all
before intel_uncore_early_sanitize()...
>
>>
>> Cc: Chris Wilson
>> Cc: Imre Deak
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83094
>> Signed-off-by: Paulo Zanoni
>> ---
>> drivers/gpu/drm/i915/i915_drv.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Maybe we need to move even more code up?
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>> b/drivers/gpu/drm/i915/i915_drv.c
>> index a05a1d0..dffb173 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -665,11 +665,11 @@ static int i915_drm_thaw_early(struct drm_device *dev)
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> int ret;
>>
>> + intel_uncore_early_sanitize(dev, true);
>> ret = intel_resume_prepare(dev_priv, false);
>> if (ret)
>> DRM_ERROR("Resume prepare failed: %d,Continuing resume\n",
>> ret);
>>
>> - intel_uncore_early_sanitize(dev, true);
>> intel_uncore_sanitize(dev);
>> intel_power_domains_init_hw(dev_priv);
>>
>
>
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915/skl: Stage the pipe DDB allocation
To correctly flush the new DDB allocation we need to know about the pipe
allocation layout inside the DDB in order to sequence the re-allocation
to not cause a newly allocated pipe to fetch from a space that was
previously allocated to another pipe.
This patch preserves the per-pipe (start,end) allocation to be used in
the flush.
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 14 +++---
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2e800db..075be77 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1436,6 +1436,7 @@ static inline bool skl_ddb_entry_equal(const struct
skl_ddb_entry *e1,
}
struct skl_ddb_allocation {
+ struct skl_ddb_entry pipe[I915_MAX_PIPES];
struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES];
struct skl_ddb_entry cursor[I915_MAX_PIPES];
};
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5129d6b..118eb95 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3117,13 +3117,13 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
struct drm_device *dev = crtc->dev;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
enum pipe pipe = intel_crtc->pipe;
- struct skl_ddb_entry alloc;
+ struct skl_ddb_entry *alloc = &ddb->pipe[pipe];
uint16_t alloc_size, start, cursor_blocks;
unsigned int total_data_rate;
int plane;
- skl_ddb_get_pipe_allocation_limits(dev, crtc, config, params, &alloc);
- alloc_size = skl_ddb_entry_size(&alloc);
+ skl_ddb_get_pipe_allocation_limits(dev, crtc, config, params, alloc);
+ alloc_size = skl_ddb_entry_size(alloc);
if (alloc_size == 0) {
memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
memset(&ddb->cursor[pipe], 0, sizeof(ddb->cursor[pipe]));
@@ -3131,11 +3131,11 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
}
cursor_blocks = skl_cursor_allocation(config);
- ddb->cursor[pipe].start = alloc.end - cursor_blocks;
- ddb->cursor[pipe].end = alloc.end;
+ ddb->cursor[pipe].start = alloc->end - cursor_blocks;
+ ddb->cursor[pipe].end = alloc->end;
alloc_size -= cursor_blocks;
- alloc.end -= cursor_blocks;
+ alloc->end -= cursor_blocks;
/*
* Each active plane get a portion of the remaining space, in
@@ -3145,7 +3145,7 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
*/
total_data_rate = skl_get_total_relative_data_rate(intel_crtc, params);
- start = alloc.start;
+ start = alloc->start;
for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) {
const struct intel_plane_wm_parameters *p;
unsigned int data_rate;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915/skl: Log the order in which we flush the pipes in the WM code
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/intel_pm.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 99f7c40..178b35f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3531,11 +3531,14 @@ static void skl_write_wm_values(struct drm_i915_private
*dev_priv,
* We need to sequence the re-allocation: C, B, A (and not B, C, A).
*/
-static void skl_wm_flush_pipe(struct drm_i915_private *dev_priv, enum pipe
pipe)
+static void
+skl_wm_flush_pipe(struct drm_i915_private *dev_priv, enum pipe pipe, int pass)
{
struct drm_device *dev = dev_priv->dev;
int plane;
+ DRM_DEBUG_KMS("flush pipe %c (pass %d)\n", pipe_name(pipe), pass);
+
for_each_plane(pipe, plane) {
I915_WRITE(PLANE_SURF(pipe, plane),
I915_READ(PLANE_SURF(pipe, plane)));
@@ -3586,7 +3589,7 @@ static void skl_flush_wm_values(struct drm_i915_private
*dev_priv,
if (!skl_ddb_allocation_included(cur_ddb, new_ddb, pipe))
continue;
- skl_wm_flush_pipe(dev_priv, pipe);
+ skl_wm_flush_pipe(dev_priv, pipe, 1);
intel_wait_for_vblank(dev, pipe);
reallocated[pipe] = true;
@@ -3611,7 +3614,7 @@ static void skl_flush_wm_values(struct drm_i915_private
*dev_priv,
if (skl_ddb_entry_size(&new_ddb->pipe[pipe]) <
skl_ddb_entry_size(&cur_ddb->pipe[pipe])) {
- skl_wm_flush_pipe(dev_priv, pipe);
+ skl_wm_flush_pipe(dev_priv, pipe, 2);
intel_wait_for_vblank(dev, pipe);
}
@@ -3637,7 +3640,7 @@ static void skl_flush_wm_values(struct drm_i915_private
*dev_priv,
if (reallocated[pipe])
continue;
- skl_wm_flush_pipe(dev_priv, pipe);
+ skl_wm_flush_pipe(dev_priv, pipe, 3);
}
}
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915/skl: Flush the WM configuration
When we write new values for the DDB allocation and WM parameters, we now
need to trigger the double buffer update for the pipe to take the new
configuration into account.
As the DDB is a global resource shared between planes, enabling or
disabling one plane will result in changes for all planes that are
currently in use, thus the need write PLANE_SURF/CUR_BASE for more than
the plane we're touching.
v2: Don't wait for pipes that are off
v3: Split the staging results structure to not exceed the 1Kb stack
allocation in skl_update_wm()
v4: Rework and document the algorithm after Ville found that it was all
wrong.
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/intel_pm.c | 135
1 file changed, 135 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 118eb95..99f7c40 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3507,6 +3507,140 @@ static void skl_write_wm_values(struct drm_i915_private
*dev_priv,
}
}
+/*
+ * When setting up a new DDB allocation arrangement, we need to correctly
+ * sequence the times at which the new allocations for the pipes are taken into
+ * account or we'll have pipes fetching from space previously allocated to
+ * another pipe.
+ *
+ * Roughly the sequence looks like:
+ * 1. re-allocate the pipe(s) with the allocation being reduced and not
+ * overlapping with a previous light-up pipe (another way to put it is:
+ * pipes with their new allocation strickly included into their old ones).
+ * 2. re-allocate the other pipes that get their allocation reduced
+ * 3. allocate the pipes having their allocation increased
+ *
+ * Steps 1. and 2. are here to take care of the following case:
+ * - Initially DDB looks like this:
+ * | B| C|
+ * - enable pipe A.
+ * - pipe B has a reduced DDB allocation that overlaps with the old pipe C
+ * allocation
+ * | A | B | C |
+ *
+ * We need to sequence the re-allocation: C, B, A (and not B, C, A).
+ */
+
+static void skl_wm_flush_pipe(struct drm_i915_private *dev_priv, enum pipe
pipe)
+{
+ struct drm_device *dev = dev_priv->dev;
+ int plane;
+
+ for_each_plane(pipe, plane) {
+ I915_WRITE(PLANE_SURF(pipe, plane),
+ I915_READ(PLANE_SURF(pipe, plane)));
+ }
+ I915_WRITE(CURBASE(pipe), I915_READ(CURBASE(pipe)));
+}
+
+static bool
+skl_ddb_allocation_included(const struct skl_ddb_allocation *old,
+ const struct skl_ddb_allocation *new,
+ enum pipe pipe)
+{
+ uint16_t old_size, new_size;
+
+ old_size = skl_ddb_entry_size(&old->pipe[pipe]);
+ new_size = skl_ddb_entry_size(&new->pipe[pipe]);
+
+ return old_size != new_size &&
+ new->pipe[pipe].start >= old->pipe[pipe].start &&
+ new->pipe[pipe].end <= old->pipe[pipe].end;
+}
+
+static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
+ struct skl_wm_values *new_values)
+{
+ struct drm_device *dev = dev_priv->dev;
+ struct skl_ddb_allocation *cur_ddb, *new_ddb;
+ bool reallocated[I915_MAX_PIPES] = {false, false, false};
+ struct intel_crtc *crtc;
+ enum pipe pipe;
+
+ new_ddb = &new_values->ddb;
+ cur_ddb = &dev_priv->wm.skl_hw.ddb;
+
+ /*
+* First pass: flush the pipes with the new allocation contained into
+* the old space.
+*
+* We'll wait for the vblank on those pipes to ensure we can safely
+* re-allocate the freed space without this pipe fetching from it.
+*/
+ for_each_intel_crtc(dev, crtc) {
+ if (!crtc->active)
+ continue;
+
+ pipe = crtc->pipe;
+
+ if (!skl_ddb_allocation_included(cur_ddb, new_ddb, pipe))
+ continue;
+
+ skl_wm_flush_pipe(dev_priv, pipe);
+ intel_wait_for_vblank(dev, pipe);
+
+ reallocated[pipe] = true;
+ }
+
+
+ /*
+* Second pass: flush the pipes that are having their allocation
+* reduced, but overlapping with a previous allocation.
+*
+* Here as well we need to wait for the vblank to make sure the freed
+* space is not used anymore.
+*/
+ for_each_intel_crtc(dev, crtc) {
+ if (!crtc->active)
+ continue;
+
+ pipe = crtc->pipe;
+
+ if (reallocated[pipe])
+ continue;
+
+ if (skl_ddb_entry_size(&new_ddb->pipe[pipe]) <
+ skl_ddb_entry_size(&cur_ddb->pipe[pipe])) {
+ skl_wm_flush_pipe(dev_priv, pipe);
+ intel_wait_for_vblank(dev, pipe);
+ }
+
+ reallocated[pipe] = true;
+ }
+
+ /*
+* Third
Re: [Intel-gfx] [PATCH] tests/kms_cursor_crc: HSW/BDW only have square cursors
2014-10-14 16:50 GMT-03:00 Chris Wilson : > On Tue, Oct 14, 2014 at 02:05:42PM -0300, Paulo Zanoni wrote: >> From: Paulo Zanoni >> >> When I look at BSpec, and at cursor_size_ok() (from the Kernel's >> intel_display.c), I see that only 845g and i865g support non-square >> displays, so SKIP the tests on HSW/BDW instead of failing them. > > Why don't we just use the information provided by the kernel as to what > it supports? (just documenting what we discussed on IRC yesterday) Apparently, there's no way to check if IVB+ has support for variable-size cursors with the current capability checks: the only way would be to check the return code from the cursor IOCTLs... > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: call drm_vblank_cleanup() earlier at unload
From: Paulo Zanoni
In its current place, it just segfaults while trying to access the
CRTC structures:
[ 9132.421681] Call Trace:
[ 9132.421707] [] i915_get_crtc_scanoutpos+0x1e8/0x220 [i915]
[ 9132.421727] []
drm_calc_vbltimestamp_from_scanoutpos+0x94/0x330 [drm]
[ 9132.421744] [] ?vblank_disable_and_save+0x40/0x1e0 [drm]
[ 9132.421769] [] i915_get_vblank_timestamp+0x68/0xb0 [i915]
[ 9132.421786] [] drm_get_last_vbltimestamp+0x44/0x80 [drm]
[ 9132.421801] [] vblank_disable_and_save+0x1a6/0x1e0 [drm]
[ 9132.421817] [] drm_vblank_cleanup+0x61/0xa0 [drm]
[ 9132.421849] [] i915_driver_unload+0xde/0x290 [i915]
[ 9132.421867] [] drm_dev_unregister+0x24/0xb0 [drm]
[ 9132.421884] [] drm_put_dev+0x1e/0x70 [drm]
[ 9132.421901] [] i915_pci_remove+0x10/0x20 [i915]
[ 9132.421910] [] pci_device_remove+0x36/0xb0
[ 9132.421920] [] __device_release_driver+0x7a/0xf0
[ 9132.421928] [] driver_detach+0xb8/0xc0
[ 9132.421936] [] bus_remove_driver+0x4a/0xb0
[ 9132.421944] [] driver_unregister+0x27/0x50
[ 9132.421953] [] pci_unregister_driver+0x25/0x70
[ 9132.421971] [] drm_pci_exit+0x78/0xa0 [drm]
[ 9132.422000] [] i915_exit+0x20/0x94e [i915]
[ 9132.422009] [] SyS_delete_module+0x13c/0x1f0
[ 9132.422019] [] ?
trace_hardirqs_on_thunk+0x3a/0x3f
[ 9132.422028] [] system_call_fastpath+0x16/0x1b
This means it has to be before intel_modeset_cleanup, which cleans the
CRTC structures. But if we move it to before intel_fbdev_fini(), we
get WARNs because intel_fbdev_fini() still tries to use the vblanks,
so the only acceptable point for drm_vblank_cleanup() seems to be this
place.
Related commit:
commit cbb47d179fb345c579cd8cd884693903fceed26a
Author: Chris Wilson
Date: Mon Sep 23 17:33:20 2013 -0300
drm/i915: Add some missing steps to i915_driver_load error path
Testsuite: igt/drv_module_reload
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77511
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83484
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/i915_dma.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 85d14e1..1b39807 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1853,8 +1853,12 @@ int i915_driver_unload(struct drm_device *dev)
acpi_video_unregister();
- if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+ if (drm_core_check_feature(dev, DRIVER_MODESET))
intel_fbdev_fini(dev);
+
+ drm_vblank_cleanup(dev);
+
+ if (drm_core_check_feature(dev, DRIVER_MODESET)) {
intel_modeset_cleanup(dev);
/*
@@ -1895,8 +1899,6 @@ int i915_driver_unload(struct drm_device *dev)
i915_free_hws(dev);
}
- drm_vblank_cleanup(dev);
-
intel_teardown_gmbus(dev);
intel_teardown_mchbar(dev);
--
2.1.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] lib: fix warnings on ignoring return values
On 15 October 2014 12:43, Mika Kuoppala wrote:
> Signed-off-by: Mika Kuoppala
> ---
> lib/igt_core.c| 21 -
> lib/igt_debugfs.c | 6 +++---
> lib/igt_kms.c | 22 +-
> lib/intel_os.c| 2 +-
> 4 files changed, 33 insertions(+), 18 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index e3d5fb0..287e345 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -336,13 +336,18 @@ static void low_mem_killer_disable(bool disable)
> /* writing to this module parameter effectively diables
> the
> * low memory killer. This is not a real file, so we dont
> need to
> * seek to the start or truncate it */
> - write(fd, no_lowmem_killer, sizeof(no_lowmem_killer));
> + igt_assert(
> + write(fd, no_lowmem_killer, sizeof(no_lowmem_killer))
> + == sizeof(no_lowmem_killer));
> close(fd);
> } else {
> /* just re-enstate the original settings */
> fd = open(adj_fname, O_WRONLY);
> igt_assert(fd != -1);
> - write(fd, prev_adj_scores, adj_scores_len);
> + igt_assert(
> + write(fd, prev_adj_scores, adj_scores_len)
> + == adj_scores_len);
> +
These warnings appear on Ubuntu because _FORTIFY_SOURCE is set by
default, which is why they don't always appear in other environments.
The warnings can be disabled by adding -U_FORITIFY_SOURCE to the
compiler flags, or if left enabled it might be worth adding wrappers
for common functions, which can then also be added to igt.cocci.
> close(fd);
> }
>
> @@ -775,7 +780,10 @@ void __igt_skip_check(const char *file, const int line,
> char *err_str = NULL;
>
> if (err)
> - asprintf(&err_str, "Last errno: %i, %s\n", err,
> strerror(err));
> + igt_assert(
> + asprintf(&err_str, "Last errno: %i, %s\n",
> +err, strerror(err))
> + > 0);
>
> if (f) {
> static char *buf;
> @@ -785,7 +793,7 @@ void __igt_skip_check(const char *file, const int line,
> free(buf);
>
> va_start(args, f);
> - vasprintf(&buf, f, args);
> + igt_assert(vasprintf(&buf, f, args) >= 0);
> va_end(args);
>
> igt_skip("Test requirement not met in function %s, file
> %s:%i:\n"
> @@ -878,7 +886,10 @@ void __igt_fail_assert(int exitcode, const char *file,
> char *err_str = NULL;
>
> if (err)
> - asprintf(&err_str, "Last errno: %i, %s\n", err,
> strerror(err));
> + igt_assert(
> + asprintf(&err_str, "Last errno: %i, %s\n",
> +err, strerror(err))
> + > 0);
>
> printf("Test assertion failure function %s, file %s:%i:\n"
>"Failed assertion: %s\n"
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index b30f5e4..f4ff61a 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -297,7 +297,7 @@ static bool igt_pipe_crc_do_start(igt_pipe_crc_t
> *pipe_crc)
> sprintf(buf, "pipe %s %s", kmstest_pipe_name(pipe_crc->pipe),
> pipe_crc_source_name(pipe_crc->source));
> errno = 0;
> - write(pipe_crc->ctl_fd, buf, strlen(buf));
> + igt_assert(write(pipe_crc->ctl_fd, buf, strlen(buf)) == strlen(buf));
> if (errno != 0)
> return false;
>
> @@ -309,7 +309,7 @@ static void igt_pipe_crc_pipe_off(int fd, enum pipe pipe)
> char buf[32];
>
> sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe));
> - write(fd, buf, strlen(buf));
> + igt_assert(write(fd, buf, strlen(buf) == strlen(buf)));
> }
>
> static void igt_pipe_crc_reset(void)
> @@ -446,7 +446,7 @@ void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
> char buf[32];
>
> sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe_crc->pipe));
> - write(pipe_crc->ctl_fd, buf, strlen(buf));
> + igt_assert(write(pipe_crc->ctl_fd, buf, strlen(buf)) == strlen(buf));
> }
>
> static bool pipe_crc_init_from_string(igt_crc_t *crc, const char *line)
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 0547147..49bc6a9 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -323,11 +323,13 @@ static char* get_debugfs_connector_path(int drm_fd,
> drmModeConnector *connector,
> {
> char *path;
>
> - asprintf(&path, "/sys/kernel/debug/dri/%d/%s-%d/%s",
> -get_card_number(drm_fd),
> -kmstest_connector_type_str(connector->connector_type),
> -connector->connector_type_id,
> -file);
> + igt_assert(
> + asprintf(&path, "/sys/kernel/debug/dri/%d/%s-%d/%s",
>
[Intel-gfx] [PATCH] lib: fix warnings on ignoring return values
Signed-off-by: Mika Kuoppala
---
lib/igt_core.c| 21 -
lib/igt_debugfs.c | 6 +++---
lib/igt_kms.c | 22 +-
lib/intel_os.c| 2 +-
4 files changed, 33 insertions(+), 18 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index e3d5fb0..287e345 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -336,13 +336,18 @@ static void low_mem_killer_disable(bool disable)
/* writing to this module parameter effectively diables the
* low memory killer. This is not a real file, so we dont need
to
* seek to the start or truncate it */
- write(fd, no_lowmem_killer, sizeof(no_lowmem_killer));
+ igt_assert(
+ write(fd, no_lowmem_killer, sizeof(no_lowmem_killer))
+ == sizeof(no_lowmem_killer));
close(fd);
} else {
/* just re-enstate the original settings */
fd = open(adj_fname, O_WRONLY);
igt_assert(fd != -1);
- write(fd, prev_adj_scores, adj_scores_len);
+ igt_assert(
+ write(fd, prev_adj_scores, adj_scores_len)
+ == adj_scores_len);
+
close(fd);
}
@@ -775,7 +780,10 @@ void __igt_skip_check(const char *file, const int line,
char *err_str = NULL;
if (err)
- asprintf(&err_str, "Last errno: %i, %s\n", err, strerror(err));
+ igt_assert(
+ asprintf(&err_str, "Last errno: %i, %s\n",
+err, strerror(err))
+ > 0);
if (f) {
static char *buf;
@@ -785,7 +793,7 @@ void __igt_skip_check(const char *file, const int line,
free(buf);
va_start(args, f);
- vasprintf(&buf, f, args);
+ igt_assert(vasprintf(&buf, f, args) >= 0);
va_end(args);
igt_skip("Test requirement not met in function %s, file
%s:%i:\n"
@@ -878,7 +886,10 @@ void __igt_fail_assert(int exitcode, const char *file,
char *err_str = NULL;
if (err)
- asprintf(&err_str, "Last errno: %i, %s\n", err, strerror(err));
+ igt_assert(
+ asprintf(&err_str, "Last errno: %i, %s\n",
+err, strerror(err))
+ > 0);
printf("Test assertion failure function %s, file %s:%i:\n"
"Failed assertion: %s\n"
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index b30f5e4..f4ff61a 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -297,7 +297,7 @@ static bool igt_pipe_crc_do_start(igt_pipe_crc_t *pipe_crc)
sprintf(buf, "pipe %s %s", kmstest_pipe_name(pipe_crc->pipe),
pipe_crc_source_name(pipe_crc->source));
errno = 0;
- write(pipe_crc->ctl_fd, buf, strlen(buf));
+ igt_assert(write(pipe_crc->ctl_fd, buf, strlen(buf)) == strlen(buf));
if (errno != 0)
return false;
@@ -309,7 +309,7 @@ static void igt_pipe_crc_pipe_off(int fd, enum pipe pipe)
char buf[32];
sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe));
- write(fd, buf, strlen(buf));
+ igt_assert(write(fd, buf, strlen(buf) == strlen(buf)));
}
static void igt_pipe_crc_reset(void)
@@ -446,7 +446,7 @@ void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
char buf[32];
sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe_crc->pipe));
- write(pipe_crc->ctl_fd, buf, strlen(buf));
+ igt_assert(write(pipe_crc->ctl_fd, buf, strlen(buf)) == strlen(buf));
}
static bool pipe_crc_init_from_string(igt_crc_t *crc, const char *line)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 0547147..49bc6a9 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -323,11 +323,13 @@ static char* get_debugfs_connector_path(int drm_fd,
drmModeConnector *connector,
{
char *path;
- asprintf(&path, "/sys/kernel/debug/dri/%d/%s-%d/%s",
-get_card_number(drm_fd),
-kmstest_connector_type_str(connector->connector_type),
-connector->connector_type_id,
-file);
+ igt_assert(
+ asprintf(&path, "/sys/kernel/debug/dri/%d/%s-%d/%s",
+get_card_number(drm_fd),
+kmstest_connector_type_str(connector->connector_type),
+connector->connector_type_id,
+file)
+ > 0);
return path;
}
@@ -851,9 +853,11 @@ static void igt_output_refresh(igt_output_t *output)
if (!output->name) {
drmModeConnector *c = output->config.connector;
- asprintf(&output->name, "%s-%d",
-kmstest_connector_type_str(c->connector_type),
-c->connector_type_id);
[Intel-gfx] [RFC] drm/i915: Remove FIXME_lrc_ctx backpointer
The first pass implementation of execlists required a backpointer to the
context to be held
in the intel_ringbuffer. However the context pointer is available higher in the
call stack.
Remove the backpointer from the ring buffer structure and instead pass it down
through the
call stack.
Signed-off-by: Nick Hoath
CC: Chris Harris
---
drivers/gpu/drm/i915/i915_gem.c | 7 ++--
drivers/gpu/drm/i915/intel_lrc.c| 65 -
drivers/gpu/drm/i915/intel_lrc.h| 10 +++--
drivers/gpu/drm/i915/intel_ringbuffer.h | 14 +++
4 files changed, 56 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e9c783d..724bbdf 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2322,6 +2322,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
struct drm_i915_gem_request *request;
struct intel_ringbuffer *ringbuf;
u32 request_ring_position, request_start;
+ struct intel_context *ctx = NULL;
int ret;
request = ring->preallocated_lazy_request;
@@ -2329,7 +2330,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
return -ENOMEM;
if (i915.enable_execlists) {
- struct intel_context *ctx = request->ctx;
+ ctx = request->ctx;
ringbuf = ctx->engine[ring->id].ringbuf;
} else
ringbuf = ring->buffer;
@@ -2343,7 +2344,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
* what.
*/
if (i915.enable_execlists) {
- ret = logical_ring_flush_all_caches(ringbuf);
+ ret = logical_ring_flush_all_caches(ringbuf, ctx);
if (ret)
return ret;
} else {
@@ -2360,7 +2361,7 @@ int __i915_add_request(struct intel_engine_cs *ring,
request_ring_position = intel_ring_get_tail(ringbuf);
if (i915.enable_execlists) {
- ret = ring->emit_request(ringbuf);
+ ret = ring->emit_request(ringbuf, ctx);
if (ret)
return ret;
} else {
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 803fc38..1be836a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -578,7 +578,8 @@ static int execlists_context_queue(struct intel_engine_cs
*ring,
return 0;
}
-static int logical_ring_invalidate_all_caches(struct intel_ringbuffer *ringbuf)
+static int logical_ring_invalidate_all_caches(struct intel_ringbuffer *ringbuf,
+ struct intel_context *ctx)
{
struct intel_engine_cs *ring = ringbuf->ring;
uint32_t flush_domains;
@@ -588,7 +589,8 @@ static int logical_ring_invalidate_all_caches(struct
intel_ringbuffer *ringbuf)
if (ring->gpu_caches_dirty)
flush_domains = I915_GEM_GPU_DOMAINS;
- ret = ring->emit_flush(ringbuf, I915_GEM_GPU_DOMAINS, flush_domains);
+ ret = ring->emit_flush(ringbuf, ctx,
+ I915_GEM_GPU_DOMAINS, flush_domains);
if (ret)
return ret;
@@ -597,6 +599,7 @@ static int logical_ring_invalidate_all_caches(struct
intel_ringbuffer *ringbuf)
}
static int execlists_move_to_gpu(struct intel_ringbuffer *ringbuf,
+struct intel_context *ctx,
struct list_head *vmas)
{
struct intel_engine_cs *ring = ringbuf->ring;
@@ -624,7 +627,7 @@ static int execlists_move_to_gpu(struct intel_ringbuffer
*ringbuf,
/* Unconditionally invalidate gpu caches and ensure that we do flush
* any residual writes from the previous batch.
*/
- return logical_ring_invalidate_all_caches(ringbuf);
+ return logical_ring_invalidate_all_caches(ringbuf, ctx);
}
/**
@@ -704,13 +707,13 @@ int intel_execlists_submission(struct drm_device *dev,
struct drm_file *file,
return -EINVAL;
}
- ret = execlists_move_to_gpu(ringbuf, vmas);
+ ret = execlists_move_to_gpu(ringbuf, ctx, vmas);
if (ret)
return ret;
if (ring == &dev_priv->ring[RCS] &&
instp_mode != dev_priv->relative_constants_mode) {
- ret = intel_logical_ring_begin(ringbuf, 4);
+ ret = intel_logical_ring_begin(ringbuf, ctx, 4);
if (ret)
return ret;
@@ -723,7 +726,7 @@ int intel_execlists_submission(struct drm_device *dev,
struct drm_file *file,
dev_priv->relative_constants_mode = instp_mode;
}
- ret = ring->emit_bb_start(ringbuf, exec_start, flags);
+ ret = ring->emit_bb_start(ringbuf, ctx, exec_start, flags);
if (ret)
return ret;
@@ -755,7 +758,8 @@ void intel_logical_ring_stop(struct intel_engine_cs *ring)
Re: [Intel-gfx] [RFC PATCH 0/3] drm driver for baytrail's vxd392
On Wed, Oct 15, 2014 at 01:18:02AM -0700, Stéphane Marchesin wrote: > On Wed, Oct 15, 2014 at 1:13 AM, Thierry Reding > wrote: > > > On Tue, Oct 14, 2014 at 08:50:35AM -0700, Sean V Kelley wrote: > > > On Tue, Oct 14, 2014 at 4:53 AM, Thierry Reding > > > wrote: > > > > On Mon, Oct 13, 2014 at 08:15:00PM +0800, Yao Cheng wrote: > > > >> drm/ipvr is a new GEM driver for baytrail's vxd392, which accelerates > > VP8 video decoding. > > > >> The driver name "ipvr" means the PowerVR's IP wrapped by Intel. In > > the future, ipvr may support other platforms such as Merrifield. > > > >> Code is placed at drivers/gpu/drm/ipvr and the following two new > > Kconfig are added: > > > >> CONFIG_DRM_IPVR: Build option for ipvr module > > > >> CONFIG_DRM_IPVR_EC: Experimental feature of error concealment > > > >> > > > >> User mode drm helper "libdrm_ipvr.so" and simple test are also > > included. > > > >> > > > >> Yao Cheng (3): > > > >> [1/3] drm/i915: add vxd392 bridge in i915 on baytrail > > > >> [2/3] drm/ipvr: ipvr drm driver for vxd392 > > > > > > > > If this is Intel-specific, why doesn't it live under the i915 driver? > > > > > > It is an entirely unrelated HW IP block, VXD392. Nothing to do with > > > GEN aside from DRM based. > > > > With GEN you're referring to the Intel integrated GPU? And VXD392 I take > > it is the IP block licensed by Imagination? Baytrail and others then > > wrap some additional logic around this as it is integrated into the SoC? > > > > How much wrapping actually happens here? I worry that this is going to > > lead to a lot of duplication if we ever want to support another SoC that > > uses the VXD392 IP. Could the code be split into a VXD392 "library" and > > some driver that implements the Intel-specific glue? > > > > Finally, if this IP block is a VP8 video decoding engine only, I'm not > > sure DRM is the best subsystem for it. Traditionally video decoding has > > been done primarily in V4L2. I'm not sure that's the best fit given that > > it was originally designed for video capturing, but they've evolved some > > infrastructure to deal with encoding/decoding, whereas we have nothing > > like that at all in DRM. > > > > That isn't true. i915, nouveau and radeon drm drivers all support video > decoding user space in some form. Well yes, because they are GPUs that happen to have video decoding engines on the same board and use the same method of command-stream submission that they use for 2D or 3D work. For standalone video decoding hardware, the only drivers that I know of are in drivers/media. Thierry pgpus0EoziqjL.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 0/3] drm driver for baytrail's vxd392
On Wed, Oct 15, 2014 at 1:13 AM, Thierry Reding wrote: > On Tue, Oct 14, 2014 at 08:50:35AM -0700, Sean V Kelley wrote: > > On Tue, Oct 14, 2014 at 4:53 AM, Thierry Reding > > wrote: > > > On Mon, Oct 13, 2014 at 08:15:00PM +0800, Yao Cheng wrote: > > >> drm/ipvr is a new GEM driver for baytrail's vxd392, which accelerates > VP8 video decoding. > > >> The driver name "ipvr" means the PowerVR's IP wrapped by Intel. In > the future, ipvr may support other platforms such as Merrifield. > > >> Code is placed at drivers/gpu/drm/ipvr and the following two new > Kconfig are added: > > >> CONFIG_DRM_IPVR: Build option for ipvr module > > >> CONFIG_DRM_IPVR_EC: Experimental feature of error concealment > > >> > > >> User mode drm helper "libdrm_ipvr.so" and simple test are also > included. > > >> > > >> Yao Cheng (3): > > >> [1/3] drm/i915: add vxd392 bridge in i915 on baytrail > > >> [2/3] drm/ipvr: ipvr drm driver for vxd392 > > > > > > If this is Intel-specific, why doesn't it live under the i915 driver? > > > > It is an entirely unrelated HW IP block, VXD392. Nothing to do with > > GEN aside from DRM based. > > With GEN you're referring to the Intel integrated GPU? And VXD392 I take > it is the IP block licensed by Imagination? Baytrail and others then > wrap some additional logic around this as it is integrated into the SoC? > > How much wrapping actually happens here? I worry that this is going to > lead to a lot of duplication if we ever want to support another SoC that > uses the VXD392 IP. Could the code be split into a VXD392 "library" and > some driver that implements the Intel-specific glue? > > Finally, if this IP block is a VP8 video decoding engine only, I'm not > sure DRM is the best subsystem for it. Traditionally video decoding has > been done primarily in V4L2. I'm not sure that's the best fit given that > it was originally designed for video capturing, but they've evolved some > infrastructure to deal with encoding/decoding, whereas we have nothing > like that at all in DRM. > That isn't true. i915, nouveau and radeon drm drivers all support video decoding user space in some form. Stéphane ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix short vs. long hpd detection
On Wed, 15 Oct 2014, Jani Nikula wrote:
> On Wed, 15 Oct 2014, Ville Syrjälä wrote:
>> On Thu, Oct 02, 2014 at 10:26:58AM +0200, Daniel Vetter wrote:
>>> On Thu, Oct 02, 2014 at 11:16:32AM +0300, Jani Nikula wrote:
>>> > Fix short vs. long hpd detection for non-g4x and non-pch split
>>> > platforms.
>>> >
>>> > Broken since introduction in
>>> > commit 13cf550448b58abf8f44f5d6a560f2d20871c965
>>> > Author: Dave Airlie
>>> > Date: Wed Jun 18 11:29:35 2014 +1000
>>> >
>>> > drm/i915: rework digital port IRQ handling (v2)
>>> >
>>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83175
>>> > Signed-off-by: Jani Nikula
>>> > ---
>>> > drivers/gpu/drm/i915/i915_irq.c | 14 +++---
>>> > 1 file changed, 7 insertions(+), 7 deletions(-)
>>> >
>>> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
>>> > b/drivers/gpu/drm/i915/i915_irq.c
>>> > index 72cb9397ecc5..3ff6f1bb594a 100644
>>> > --- a/drivers/gpu/drm/i915/i915_irq.c
>>> > +++ b/drivers/gpu/drm/i915/i915_irq.c
>>> > @@ -1708,7 +1708,7 @@ static irqreturn_t gen8_gt_irq_handler(struct
>>> > drm_device *dev,
>>> > #define HPD_STORM_DETECT_PERIOD 1000
>>> > #define HPD_STORM_THRESHOLD 5
>>> >
>>> > -static int ilk_port_to_hotplug_shift(enum port port)
>>> > +static int pch_port_to_hotplug_shift(enum port port)
>>> > {
>>> > switch (port) {
>>> > case PORT_A:
>>> > @@ -1724,7 +1724,7 @@ static int ilk_port_to_hotplug_shift(enum port port)
>>> > }
>>> > }
>>> >
>>> > -static int g4x_port_to_hotplug_shift(enum port port)
>>> > +static int i915_port_to_hotplug_shift(enum port port)
>>> > {
>>> > switch (port) {
>>> > case PORT_A:
>>> > @@ -1782,12 +1782,12 @@ static inline void intel_hpd_irq_handler(struct
>>> > drm_device *dev,
>>> > if (port && dev_priv->hpd_irq_port[port]) {
>>> > bool long_hpd;
>>> >
>>> > - if (IS_G4X(dev)) {
>>> > - dig_shift = g4x_port_to_hotplug_shift(port);
>>> > - long_hpd = (hotplug_trigger >> dig_shift) &
>>> > PORTB_HOTPLUG_LONG_DETECT;
>>> > - } else {
>>> > - dig_shift = ilk_port_to_hotplug_shift(port);
>>> > + if (HAS_PCH_SPLIT(dev)) {
>>> > + dig_shift = pch_port_to_hotplug_shift(port);
>>> > long_hpd = (dig_hotplug_reg >> dig_shift) &
>>> > PORTB_HOTPLUG_LONG_DETECT;
>>>
>>> Using the new HAS_GMCH_DISPLAY will probably survive longer (i.e. skl).
>>
>> Did we have a concenses on this? I want hpd on my BSW.
>
> Sorry, we agreed on IRC that the ifs were okay as-is, but we're still
> missing review.
Even a tested-by would be welcome!
Jani.
>
> BR,
> Jani.
>
>
>
>
>>
>>> -Daniel
>>>
>>> > + } else {
>>> > + dig_shift = i915_port_to_hotplug_shift(port);
>>> > + long_hpd = (hotplug_trigger >> dig_shift) &
>>> > PORTB_HOTPLUG_LONG_DETECT;
>>> > }
>>> >
>>> > DRM_DEBUG_DRIVER("digital hpd port %c - %s\n",
>>> > --
>>> > 1.9.1
>>> >
>>> > ___
>>> > Intel-gfx mailing list
>>> > Intel-gfx@lists.freedesktop.org
>>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>>> --
>>> Daniel Vetter
>>> Software Engineer, Intel Corporation
>>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>>> ___
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> --
>> Ville Syrjälä
>> Intel OTC
>
> --
> Jani Nikula, Intel Open Source Technology Center
> ___
> 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: fix short vs. long hpd detection
On Wed, Oct 15, 2014 at 10:51:54AM +0300, Jani Nikula wrote:
> On Wed, 15 Oct 2014, Ville Syrjälä wrote:
> > On Thu, Oct 02, 2014 at 10:26:58AM +0200, Daniel Vetter wrote:
> >> On Thu, Oct 02, 2014 at 11:16:32AM +0300, Jani Nikula wrote:
> >> > Fix short vs. long hpd detection for non-g4x and non-pch split
> >> > platforms.
> >> >
> >> > Broken since introduction in
> >> > commit 13cf550448b58abf8f44f5d6a560f2d20871c965
> >> > Author: Dave Airlie
> >> > Date: Wed Jun 18 11:29:35 2014 +1000
> >> >
> >> > drm/i915: rework digital port IRQ handling (v2)
> >> >
> >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83175
> >> > Signed-off-by: Jani Nikula
> >> > ---
> >> > drivers/gpu/drm/i915/i915_irq.c | 14 +++---
> >> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> >> > b/drivers/gpu/drm/i915/i915_irq.c
> >> > index 72cb9397ecc5..3ff6f1bb594a 100644
> >> > --- a/drivers/gpu/drm/i915/i915_irq.c
> >> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> > @@ -1708,7 +1708,7 @@ static irqreturn_t gen8_gt_irq_handler(struct
> >> > drm_device *dev,
> >> > #define HPD_STORM_DETECT_PERIOD 1000
> >> > #define HPD_STORM_THRESHOLD 5
> >> >
> >> > -static int ilk_port_to_hotplug_shift(enum port port)
> >> > +static int pch_port_to_hotplug_shift(enum port port)
> >> > {
> >> > switch (port) {
> >> > case PORT_A:
> >> > @@ -1724,7 +1724,7 @@ static int ilk_port_to_hotplug_shift(enum port
> >> > port)
> >> > }
> >> > }
> >> >
> >> > -static int g4x_port_to_hotplug_shift(enum port port)
> >> > +static int i915_port_to_hotplug_shift(enum port port)
> >> > {
> >> > switch (port) {
> >> > case PORT_A:
> >> > @@ -1782,12 +1782,12 @@ static inline void intel_hpd_irq_handler(struct
> >> > drm_device *dev,
> >> > if (port && dev_priv->hpd_irq_port[port]) {
> >> > bool long_hpd;
> >> >
> >> > -if (IS_G4X(dev)) {
> >> > -dig_shift =
> >> > g4x_port_to_hotplug_shift(port);
> >> > -long_hpd = (hotplug_trigger >>
> >> > dig_shift) & PORTB_HOTPLUG_LONG_DETECT;
> >> > -} else {
> >> > -dig_shift =
> >> > ilk_port_to_hotplug_shift(port);
> >> > +if (HAS_PCH_SPLIT(dev)) {
> >> > +dig_shift =
> >> > pch_port_to_hotplug_shift(port);
> >> > long_hpd = (dig_hotplug_reg >>
> >> > dig_shift) & PORTB_HOTPLUG_LONG_DETECT;
> >>
> >> Using the new HAS_GMCH_DISPLAY will probably survive longer (i.e. skl).
> >
> > Did we have a concenses on this? I want hpd on my BSW.
>
> Sorry, we agreed on IRC that the ifs were okay as-is, but we're still
> missing review.
Reviewed-by: Ville Syrjälä
Although after checking the spec it does look like HAS_GMCH_DISPLAY
might be a bit more future proof.
>
> BR,
> Jani.
>
>
>
>
> >
> >> -Daniel
> >>
> >> > +} else {
> >> > +dig_shift =
> >> > i915_port_to_hotplug_shift(port);
> >> > +long_hpd = (hotplug_trigger >>
> >> > dig_shift) & PORTB_HOTPLUG_LONG_DETECT;
> >> > }
> >> >
> >> > DRM_DEBUG_DRIVER("digital hpd port %c - %s\n",
> >> > --
> >> > 1.9.1
> >> >
> >> > ___
> >> > Intel-gfx mailing list
> >> > Intel-gfx@lists.freedesktop.org
> >> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>
> >> --
> >> Daniel Vetter
> >> Software Engineer, Intel Corporation
> >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> >> ___
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel OTC
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix short vs. long hpd detection
On Wed, 15 Oct 2014, Ville Syrjälä wrote:
> On Thu, Oct 02, 2014 at 10:26:58AM +0200, Daniel Vetter wrote:
>> On Thu, Oct 02, 2014 at 11:16:32AM +0300, Jani Nikula wrote:
>> > Fix short vs. long hpd detection for non-g4x and non-pch split
>> > platforms.
>> >
>> > Broken since introduction in
>> > commit 13cf550448b58abf8f44f5d6a560f2d20871c965
>> > Author: Dave Airlie
>> > Date: Wed Jun 18 11:29:35 2014 +1000
>> >
>> > drm/i915: rework digital port IRQ handling (v2)
>> >
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83175
>> > Signed-off-by: Jani Nikula
>> > ---
>> > drivers/gpu/drm/i915/i915_irq.c | 14 +++---
>> > 1 file changed, 7 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
>> > b/drivers/gpu/drm/i915/i915_irq.c
>> > index 72cb9397ecc5..3ff6f1bb594a 100644
>> > --- a/drivers/gpu/drm/i915/i915_irq.c
>> > +++ b/drivers/gpu/drm/i915/i915_irq.c
>> > @@ -1708,7 +1708,7 @@ static irqreturn_t gen8_gt_irq_handler(struct
>> > drm_device *dev,
>> > #define HPD_STORM_DETECT_PERIOD 1000
>> > #define HPD_STORM_THRESHOLD 5
>> >
>> > -static int ilk_port_to_hotplug_shift(enum port port)
>> > +static int pch_port_to_hotplug_shift(enum port port)
>> > {
>> >switch (port) {
>> >case PORT_A:
>> > @@ -1724,7 +1724,7 @@ static int ilk_port_to_hotplug_shift(enum port port)
>> >}
>> > }
>> >
>> > -static int g4x_port_to_hotplug_shift(enum port port)
>> > +static int i915_port_to_hotplug_shift(enum port port)
>> > {
>> >switch (port) {
>> >case PORT_A:
>> > @@ -1782,12 +1782,12 @@ static inline void intel_hpd_irq_handler(struct
>> > drm_device *dev,
>> >if (port && dev_priv->hpd_irq_port[port]) {
>> >bool long_hpd;
>> >
>> > - if (IS_G4X(dev)) {
>> > - dig_shift = g4x_port_to_hotplug_shift(port);
>> > - long_hpd = (hotplug_trigger >> dig_shift) &
>> > PORTB_HOTPLUG_LONG_DETECT;
>> > - } else {
>> > - dig_shift = ilk_port_to_hotplug_shift(port);
>> > + if (HAS_PCH_SPLIT(dev)) {
>> > + dig_shift = pch_port_to_hotplug_shift(port);
>> >long_hpd = (dig_hotplug_reg >> dig_shift) &
>> > PORTB_HOTPLUG_LONG_DETECT;
>>
>> Using the new HAS_GMCH_DISPLAY will probably survive longer (i.e. skl).
>
> Did we have a concenses on this? I want hpd on my BSW.
Sorry, we agreed on IRC that the ifs were okay as-is, but we're still
missing review.
BR,
Jani.
>
>> -Daniel
>>
>> > + } else {
>> > + dig_shift = i915_port_to_hotplug_shift(port);
>> > + long_hpd = (hotplug_trigger >> dig_shift) &
>> > PORTB_HOTPLUG_LONG_DETECT;
>> >}
>> >
>> >DRM_DEBUG_DRIVER("digital hpd port %c - %s\n",
>> > --
>> > 1.9.1
>> >
>> > ___
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
--
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: fix short vs. long hpd detection
On Thu, Oct 02, 2014 at 10:26:58AM +0200, Daniel Vetter wrote:
> On Thu, Oct 02, 2014 at 11:16:32AM +0300, Jani Nikula wrote:
> > Fix short vs. long hpd detection for non-g4x and non-pch split
> > platforms.
> >
> > Broken since introduction in
> > commit 13cf550448b58abf8f44f5d6a560f2d20871c965
> > Author: Dave Airlie
> > Date: Wed Jun 18 11:29:35 2014 +1000
> >
> > drm/i915: rework digital port IRQ handling (v2)
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83175
> > Signed-off-by: Jani Nikula
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 14 +++---
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 72cb9397ecc5..3ff6f1bb594a 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1708,7 +1708,7 @@ static irqreturn_t gen8_gt_irq_handler(struct
> > drm_device *dev,
> > #define HPD_STORM_DETECT_PERIOD 1000
> > #define HPD_STORM_THRESHOLD 5
> >
> > -static int ilk_port_to_hotplug_shift(enum port port)
> > +static int pch_port_to_hotplug_shift(enum port port)
> > {
> > switch (port) {
> > case PORT_A:
> > @@ -1724,7 +1724,7 @@ static int ilk_port_to_hotplug_shift(enum port port)
> > }
> > }
> >
> > -static int g4x_port_to_hotplug_shift(enum port port)
> > +static int i915_port_to_hotplug_shift(enum port port)
> > {
> > switch (port) {
> > case PORT_A:
> > @@ -1782,12 +1782,12 @@ static inline void intel_hpd_irq_handler(struct
> > drm_device *dev,
> > if (port && dev_priv->hpd_irq_port[port]) {
> > bool long_hpd;
> >
> > - if (IS_G4X(dev)) {
> > - dig_shift = g4x_port_to_hotplug_shift(port);
> > - long_hpd = (hotplug_trigger >> dig_shift) &
> > PORTB_HOTPLUG_LONG_DETECT;
> > - } else {
> > - dig_shift = ilk_port_to_hotplug_shift(port);
> > + if (HAS_PCH_SPLIT(dev)) {
> > + dig_shift = pch_port_to_hotplug_shift(port);
> > long_hpd = (dig_hotplug_reg >> dig_shift) &
> > PORTB_HOTPLUG_LONG_DETECT;
>
> Using the new HAS_GMCH_DISPLAY will probably survive longer (i.e. skl).
Did we have a concenses on this? I want hpd on my BSW.
> -Daniel
>
> > + } else {
> > + dig_shift = i915_port_to_hotplug_shift(port);
> > + long_hpd = (hotplug_trigger >> dig_shift) &
> > PORTB_HOTPLUG_LONG_DETECT;
> > }
> >
> > DRM_DEBUG_DRIVER("digital hpd port %c - %s\n",
> > --
> > 1.9.1
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
