[Intel-gfx] libdrm vs. kernel-drm: Spelling of caching/cacheing and its impact?
[ Sending to intel-gfx only for 1st feedback ] Hi, while still digging into my next-20130813 issue, I have searched for some defines: $ egrep 'I915_CACHEING_CACHED|I915_CACHING_CACHED|I915_CACHEING_NONE|I915_CACHING_NONE' -nr ~/src/linux-kernel/linux/drivers/gpu/drm/i915/ ~/src/linux-kernel/linux/drivers/gpu/drm/i915/i915_gem.c:3471: args-caching = I915_CACHING_CACHED; ~/src/linux-kernel/linux/drivers/gpu/drm/i915/i915_gem.c:3479: args-caching = I915_CACHING_NONE; ~/src/linux-kernel/linux/drivers/gpu/drm/i915/i915_gem.c:3498: case I915_CACHING_NONE: ~/src/linux-kernel/linux/drivers/gpu/drm/i915/i915_gem.c:3501: case I915_CACHING_CACHED: $ egrep 'I915_CACHEING_CACHED|I915_CACHING_CACHED|I915_CACHEING_NONE|I915_CACHING_NONE' -nr ~/src/linux-kernel/linux/include/drm/ $ egrep 'I915_CACHEING_CACHED|I915_CACHING_CACHED|I915_CACHEING_NONE|I915_CACHING_NONE' -nr ~/src/libdrm/libdrm-git/ ~/src/libdrm/libdrm-git/include/drm/i915_drm.h:709:#define I915_CACHEING_NONE 0 ~/src/libdrm/libdrm-git/include/drm/i915_drm.h:710:#define I915_CACHEING_CACHED1 [ This is libdrm v2.4.46 BTW. ] I also checked intel-gpu-tools which has also cach***e*** pattern in some defines. What's the impact of this misspelling? Regards, - Sedat - ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] libdrm vs. kernel-drm: Spelling of caching/cacheing and its impact?
On Wed, Aug 14, 2013 at 8:45 AM, Sedat Dilek sedat.di...@gmail.com wrote: [ Sending to intel-gfx only for 1st feedback ] Hi, while still digging into my next-20130813 issue, I have searched for some defines: $ egrep 'I915_CACHEING_CACHED|I915_CACHING_CACHED|I915_CACHEING_NONE|I915_CACHING_NONE' -nr ~/src/linux-kernel/linux/drivers/gpu/drm/i915/ ~/src/linux-kernel/linux/drivers/gpu/drm/i915/i915_gem.c:3471: args-caching = I915_CACHING_CACHED; ~/src/linux-kernel/linux/drivers/gpu/drm/i915/i915_gem.c:3479: args-caching = I915_CACHING_NONE; ~/src/linux-kernel/linux/drivers/gpu/drm/i915/i915_gem.c:3498: case I915_CACHING_NONE: ~/src/linux-kernel/linux/drivers/gpu/drm/i915/i915_gem.c:3501: case I915_CACHING_CACHED: $ egrep 'I915_CACHEING_CACHED|I915_CACHING_CACHED|I915_CACHEING_NONE|I915_CACHING_NONE' -nr ~/src/linux-kernel/linux/include/drm/ $ egrep 'I915_CACHEING_CACHED|I915_CACHING_CACHED|I915_CACHEING_NONE|I915_CACHING_NONE' -nr ~/src/libdrm/libdrm-git/ ~/src/libdrm/libdrm-git/include/drm/i915_drm.h:709:#define I915_CACHEING_NONE 0 ~/src/libdrm/libdrm-git/include/drm/i915_drm.h:710:#define I915_CACHEING_CACHED1 [ This is libdrm v2.4.46 BTW. ] I also checked intel-gpu-tools which has also cach***e*** pattern in some defines. What's the impact of this misspelling? Or see this one: $ egrep 'DRM_I915_GEM_SET_CACH|DRM_I915_GEM_GET_CACH' -nr ~/src/linux-kernel/linux/include/drm/ ~/src/linux-kernel/linux/include/uapi/drm/ ~/src/libdrm/libdrm-git/ ~/src/linux-kernel/linux/include/uapi/drm/i915_drm.h:222:#define DRM_I915_GEM_SET_CACHING 0x2f ~/src/linux-kernel/linux/include/uapi/drm/i915_drm.h:223:#define DRM_I915_GEM_GET_CACHING 0x30 ~/src/linux-kernel/linux/include/uapi/drm/i915_drm.h:249:#define DRM_IOCTL_I915_GEM_SET_CACHINGDRM_IOW(DRM_COMMAND_BASE + DRM_I915_GEM_SET_CACHING, struct drm_i915_gem_caching) ~/src/linux-kernel/linux/include/uapi/drm/i915_drm.h:250:#define DRM_IOCTL_I915_GEM_GET_CACHINGDRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_GET_CACHING, struct drm_i915_gem_caching) ~/src/libdrm/libdrm-git/include/drm/i915_drm.h:198:#define DRM_I915_GEM_SET_CACHEING 0x2f ~/src/libdrm/libdrm-git/include/drm/i915_drm.h:199:#define DRM_I915_GEM_GET_CACHEING 0x30 ~/src/libdrm/libdrm-git/include/drm/i915_drm.h:225:#define DRM_IOCTL_I915_GEM_SET_CACHEING DRM_IOW(DRM_COMMAND_BASE + DRM_I915_GEM_SET_CACHEING, struct drm_i915_gem_cacheing) ~/src/libdrm/libdrm-git/include/drm/i915_drm.h:226:#define DRM_IOCTL_I915_GEM_GET_CACHEING DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_GET_CACHEING, struct drm_i915_gem_cacheing) - Sedat - ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] libdrm vs. kernel-drm: Spelling of caching/cacheing and its impact?
On Wed, Aug 14, 2013 at 8:52 AM, Sedat Dilek sedat.di...@gmail.com wrote: On Wed, Aug 14, 2013 at 8:45 AM, Sedat Dilek sedat.di...@gmail.com wrote: [ Sending to intel-gfx only for 1st feedback ] Hi, while still digging into my next-20130813 issue, I have searched for some defines: $ egrep 'I915_CACHEING_CACHED|I915_CACHING_CACHED|I915_CACHEING_NONE|I915_CACHING_NONE' -nr ~/src/linux-kernel/linux/drivers/gpu/drm/i915/ ~/src/linux-kernel/linux/drivers/gpu/drm/i915/i915_gem.c:3471: args-caching = I915_CACHING_CACHED; ~/src/linux-kernel/linux/drivers/gpu/drm/i915/i915_gem.c:3479: args-caching = I915_CACHING_NONE; ~/src/linux-kernel/linux/drivers/gpu/drm/i915/i915_gem.c:3498: case I915_CACHING_NONE: ~/src/linux-kernel/linux/drivers/gpu/drm/i915/i915_gem.c:3501: case I915_CACHING_CACHED: $ egrep 'I915_CACHEING_CACHED|I915_CACHING_CACHED|I915_CACHEING_NONE|I915_CACHING_NONE' -nr ~/src/linux-kernel/linux/include/drm/ $ egrep 'I915_CACHEING_CACHED|I915_CACHING_CACHED|I915_CACHEING_NONE|I915_CACHING_NONE' -nr ~/src/libdrm/libdrm-git/ ~/src/libdrm/libdrm-git/include/drm/i915_drm.h:709:#define I915_CACHEING_NONE 0 ~/src/libdrm/libdrm-git/include/drm/i915_drm.h:710:#define I915_CACHEING_CACHED1 [ This is libdrm v2.4.46 BTW. ] I also checked intel-gpu-tools which has also cach***e*** pattern in some defines. What's the impact of this misspelling? Or see this one: $ egrep 'DRM_I915_GEM_SET_CACH|DRM_I915_GEM_GET_CACH' -nr ~/src/linux-kernel/linux/include/drm/ ~/src/linux-kernel/linux/include/uapi/drm/ ~/src/libdrm/libdrm-git/ ~/src/linux-kernel/linux/include/uapi/drm/i915_drm.h:222:#define DRM_I915_GEM_SET_CACHING 0x2f ~/src/linux-kernel/linux/include/uapi/drm/i915_drm.h:223:#define DRM_I915_GEM_GET_CACHING 0x30 ~/src/linux-kernel/linux/include/uapi/drm/i915_drm.h:249:#define DRM_IOCTL_I915_GEM_SET_CACHINGDRM_IOW(DRM_COMMAND_BASE + DRM_I915_GEM_SET_CACHING, struct drm_i915_gem_caching) ~/src/linux-kernel/linux/include/uapi/drm/i915_drm.h:250:#define DRM_IOCTL_I915_GEM_GET_CACHINGDRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_GET_CACHING, struct drm_i915_gem_caching) ~/src/libdrm/libdrm-git/include/drm/i915_drm.h:198:#define DRM_I915_GEM_SET_CACHEING 0x2f ~/src/libdrm/libdrm-git/include/drm/i915_drm.h:199:#define DRM_I915_GEM_GET_CACHEING 0x30 ~/src/libdrm/libdrm-git/include/drm/i915_drm.h:225:#define DRM_IOCTL_I915_GEM_SET_CACHEING DRM_IOW(DRM_COMMAND_BASE + DRM_I915_GEM_SET_CACHEING, struct drm_i915_gem_cacheing) ~/src/libdrm/libdrm-git/include/drm/i915_drm.h:226:#define DRM_IOCTL_I915_GEM_GET_CACHEING DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_GET_CACHEING, struct drm_i915_gem_cacheing) I have *not* installed next-20130813 linux-headers Debian package (generated via 'make deb-pkg'). $ dpkg -l | grep -i linux-headers ii linux-headers-3.2.0-52 3.2.0-52.78 Header files related to Linux kernel version 3.2.0 ii linux-headers-3.2.0-52-generic 3.2.0-52.78 Linux kernel headers for version 3.2.0 on 64 bit x86 SMP ii linux-headers-generic 3.2.0.52.62 Generic Linux kernel headers So, what I have is kernel-drm v3.2.0-52.78 and libdrm v2.4.46. - Sedat - ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] libdrm vs. kernel-drm: Spelling of caching/cacheing and its impact?
On Wed, Aug 14, 2013 at 9:12 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Aug 14, 2013 at 08:45:47AM +0200, Sedat Dilek wrote: [ Sending to intel-gfx only for 1st feedback ] Hi, while still digging into my next-20130813 issue, I have searched for some defines: [snip] I also checked intel-gpu-tools which has also cach***e*** pattern in some defines. What's the impact of this misspelling? Absolutely zero. The only users do not care what the kernel or even libdrm call it, only that the value works. How can the values work if you have different defined macros (defines)? - Sedat - ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] libdrm vs. kernel-drm: Spelling of caching/cacheing and its impact?
On Wed, Aug 14, 2013 at 08:45:47AM +0200, Sedat Dilek wrote: [ Sending to intel-gfx only for 1st feedback ] Hi, while still digging into my next-20130813 issue, I have searched for some defines: [snip] I also checked intel-gpu-tools which has also cach***e*** pattern in some defines. What's the impact of this misspelling? Absolutely zero. The only users do not care what the kernel or even libdrm call it, only that the value works. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Apply the force-detect VGA w/a to Valleyview
It appears that Valleyview shares its VGA encoder with more recent
siblings and requires the same forced detection cycle after a hardware
reset before we can rely on hotplugging.
Reported-and-tested-by: kobeqin kobe@intel.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67733
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/intel_crt.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index b5a3875..7475200 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -688,7 +688,7 @@ static void intel_crt_reset(struct drm_connector *connector)
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crt *crt = intel_attached_crt(connector);
- if (HAS_PCH_SPLIT(dev)) {
+ if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev)) {
u32 adpa;
adpa = I915_READ(crt-adpa_reg);
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] [v4] drm/i915: Convert execbuf code to use vmas
On Tue, Aug 13, 2013 at 06:11:59PM -0700, Ben Widawsky wrote: On Tue, Aug 13, 2013 at 06:09:09PM -0700, Ben Widawsky wrote: From: Ben Widawsky b...@bwidawsk.net In order to transition more of our code over to using a VMA instead of an OBJ, VM pair - we must have the vma accessible at execbuf time. Up until now, we've only had a VMA when actually binding an object. The previous patch helped handle the distinction on bound vs. unbound. This patch will help us catch leaks, and other issues before we actually shuffle a bunch of stuff around. This attempts to convert all the execbuf code to speak in vmas. Since the execbuf code is very self contained it was a nice isolated conversion. The meat of the code is about turning eb_objects into eb_vma, and then wiring up the rest of the code to use vmas instead of obj, vm pairs. Unfortunately, to do this, we must move the exec_list link from the obj structure. This list is reused in the eviction code, so we must also modify the eviction code to make this work. WARNING: This patch makes an already hotly profiled path slower. The cost is unavoidable. In reply to this mail, I will attach the extra data. [snip] Here is the output from gem_exec_lut_handle both before and after this patch. The results honestly don't make sense to me, but I'll set Chris parse it before scratching my head harder. Before patch relocation: buffers= 1: old= 8060 + 165.3*reloc, lut= 7816 + 164.8*reloc (ns) relocation: buffers= 2: old= 6748 + 166.6*reloc, lut= 6952 + 165.4*reloc (ns) relocation: buffers= 4: old= 8140 + 165.9*reloc, lut= 8216 + 165.4*reloc (ns) relocation: buffers= 8: old= 10732 + 166.0*reloc, lut= 10615 + 165.2*reloc (ns) relocation: buffers= 16: old= 15099 + 167.8*reloc, lut= 15337 + 165.3*reloc (ns) relocation: buffers= 32: old= 26140 + 166.0*reloc, lut= 25488 + 165.5*reloc (ns) relocation: buffers= 64: old= 46300 + 170.5*reloc, lut= 44279 + 166.7*reloc (ns) relocation: buffers= 128: old= 84056 + 176.9*reloc, lut= 85379 + 166.3*reloc (ns) relocation: buffers= 256: old= 174398 + 167.9*reloc, lut= 167744 + 167.0*reloc (ns) relocation: buffers= 512: old= 349688 + 175.7*reloc, lut= 348590 + 170.8*reloc (ns) relocation: buffers=1024: old= 726265 + 191.2*reloc, lut= 719774 + 180.2*reloc (ns) relocation: buffers=2048: old=1456866 + 224.3*reloc, lut=1442087 + 173.0*reloc (ns) skip-relocs: buffers= 1: old= 4445 + 16.0*reloc, lut= 4433 + 15.6*reloc (ns) skip-relocs: buffers= 2: old= 4585 + 16.0*reloc, lut= 4571 + 15.6*reloc (ns) skip-relocs: buffers= 4: old= 5667 + 16.0*reloc, lut= 5340 + 15.6*reloc (ns) skip-relocs: buffers= 8: old= 6051 + 16.1*reloc, lut= 6026 + 15.6*reloc (ns) skip-relocs: buffers= 16: old= 7953 + 16.1*reloc, lut= 7914 + 15.6*reloc (ns) skip-relocs: buffers= 32: old= 11972 + 16.2*reloc, lut= 11875 + 15.7*reloc (ns) skip-relocs: buffers= 64: old= 1 + 16.5*reloc, lut= 19832 + 15.7*reloc (ns) skip-relocs: buffers= 128: old= 37796 + 16.9*reloc, lut= 36539 + 15.9*reloc (ns) skip-relocs: buffers= 256: old= 71604 + 18.1*reloc, lut= 71313 + 16.5*reloc (ns) skip-relocs: buffers= 512: old= 152682 + 24.3*reloc, lut= 141379 + 27.9*reloc (ns) skip-relocs: buffers=1024: old= 314116 + 41.7*reloc, lut= 303019 + 20.1*reloc (ns) skip-relocs: buffers=2048: old= 619784 + 54.1*reloc, lut= 603931 + 20.0*reloc (ns) no-relocs: buffers= 1: old= 4194 + 5.1*reloc, lut= 4206 + 4.8*reloc (ns) no-relocs: buffers= 2: old= 4404 + 5.1*reloc, lut= 4381 + 4.8*reloc (ns) no-relocs: buffers= 4: old= 4926 + 5.1*reloc, lut= 4921 + 4.8*reloc (ns) no-relocs: buffers= 8: old= 5901 + 5.1*reloc, lut= 5822 + 4.9*reloc (ns) no-relocs: buffers= 16: old= 7840 + 5.1*reloc, lut= 7737 + 4.9*reloc (ns) no-relocs: buffers= 32: old= 11842 + 5.1*reloc, lut= 11681 + 4.9*reloc (ns) no-relocs: buffers= 64: old= 19741 + 5.1*reloc, lut= 19542 + 4.8*reloc (ns) no-relocs: buffers= 128: old= 36479 + 5.2*reloc, lut= 35958 + 4.9*reloc (ns) no-relocs: buffers= 256: old= 70171 + 5.4*reloc, lut= 69390 + 5.2*reloc (ns) no-relocs: buffers= 512: old= 147213 + 3.5*reloc, lut= 137953 + 13.0*reloc (ns) no-relocs: buffers=1024: old= 300165 + 4.8*reloc, lut= 293852 + 4.9*reloc (ns) no-relocs: buffers=2048: old= 597992 + 8.3*reloc, lut= 590185 + 2.1*reloc (ns) After patch === relocation: buffers= 1: old= 8075 + 81.4*reloc, lut= 7592 + 80.6*reloc (ns) relocation: buffers= 2: old= 5744 + 82.3*reloc, lut= 5837 + 81.1*reloc (ns) relocation: buffers= 4: old= 4875 + 82.7*reloc, lut= 4871 + 81.6*reloc (ns) relocation: buffers= 8: old= 5729 + 82.7*reloc, lut= 5698 + 81.5*reloc (ns) relocation: buffers= 16: old= 7952 + 83.0*reloc, lut= 7809 + 81.9*reloc (ns) relocation: buffers= 32: old= 11884 + 82.9*reloc, lut=
[Intel-gfx] [PATCH] Correct misspelled caching
Signed-off-by: Sedat Dilek sedat.di...@gmail.com
---
configure.ac | 4 ++--
src/sna/kgem.c | 50 +-
src/sna/kgem.h | 2 +-
3 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/configure.ac b/configure.ac
index 43c33eb..211a086 100644
--- a/configure.ac
+++ b/configure.ac
@@ -371,12 +371,12 @@ fi
AC_ARG_ENABLE(wt,
AS_HELP_STRING([--enable-wt],
-[Enable use of WT cacheing (experimental)
[default=no]]),
+[Enable use of WT caching (experimental)
[default=no]]),
[WT=$enableval],
[WT=no])
AM_CONDITIONAL(USE_WT, test x$WT = xyes)
if test x$WT = xyes; then
- AC_DEFINE(USE_WT,1,[Assume WriteThrough cacheing support])
+ AC_DEFINE(USE_WT,1,[Assume WriteThrough caching support])
xp_msg=$xp_msg wt-cache
fi
diff --git a/src/sna/kgem.c b/src/sna/kgem.c
index 98e801a..e7de38c 100644
--- a/src/sna/kgem.c
+++ b/src/sna/kgem.c
@@ -162,13 +162,13 @@ struct local_i915_gem_userptr {
#define SNOOPED1
#define DISPLAY2
-struct local_i915_gem_cacheing {
+struct local_i915_gem_caching {
uint32_t handle;
- uint32_t cacheing;
+ uint32_t caching;
};
-#define LOCAL_I915_GEM_SET_CACHEING0x2f
-#define LOCAL_IOCTL_I915_GEM_SET_CACHEING DRM_IOW(DRM_COMMAND_BASE +
LOCAL_I915_GEM_SET_CACHEING, struct local_i915_gem_cacheing)
+#define LOCAL_I915_GEM_SET_CACHING 0x2f
+#define LOCAL_IOCTL_I915_GEM_SET_CACHING DRM_IOW(DRM_COMMAND_BASE +
LOCAL_I915_GEM_SET_CACHING, struct local_i915_gem_caching)
struct kgem_buffer {
struct kgem_bo base;
@@ -260,14 +260,14 @@ static bool gem_set_tiling(int fd, uint32_t handle, int
tiling, int stride)
return ret == 0;
}
-static bool gem_set_cacheing(int fd, uint32_t handle, int cacheing)
+static bool gem_set_caching(int fd, uint32_t handle, int caching)
{
- struct local_i915_gem_cacheing arg;
+ struct local_i915_gem_caching arg;
VG_CLEAR(arg);
arg.handle = handle;
- arg.cacheing = cacheing;
- return drmIoctl(fd, LOCAL_IOCTL_I915_GEM_SET_CACHEING, arg) == 0;
+ arg.caching = caching;
+ return drmIoctl(fd, LOCAL_IOCTL_I915_GEM_SET_CACHING, arg) == 0;
}
static uint32_t gem_userptr(int fd, void *ptr, int size, int read_only)
@@ -932,7 +932,7 @@ static bool test_has_llc(struct kgem *kgem)
return has_llc;
}
-static bool test_has_cacheing(struct kgem *kgem)
+static bool test_has_caching(struct kgem *kgem)
{
uint32_t handle;
bool ret;
@@ -948,7 +948,7 @@ static bool test_has_cacheing(struct kgem *kgem)
if (handle == 0)
return false;
- ret = gem_set_cacheing(kgem-fd, handle, UNCACHED);
+ ret = gem_set_caching(kgem-fd, handle, UNCACHED);
gem_close(kgem-fd, handle);
return ret;
}
@@ -1171,12 +1171,12 @@ void kgem_init(struct kgem *kgem, int fd, struct
pci_device *dev, unsigned gen)
kgem-has_llc));
kgem-has_wt = test_has_wt(kgem);
- DBG((%s: has write-through cacheing for scanouts? %d\n, __FUNCTION__,
+ DBG((%s: has write-through caching for scanouts? %d\n, __FUNCTION__,
kgem-has_wt));
- kgem-has_cacheing = test_has_cacheing(kgem);
+ kgem-has_caching = test_has_caching(kgem);
DBG((%s: has set-cache-level? %d\n, __FUNCTION__,
-kgem-has_cacheing));
+kgem-has_caching));
kgem-has_userptr = test_has_userptr(kgem);
DBG((%s: has userptr? %d\n, __FUNCTION__,
@@ -1254,8 +1254,8 @@ void kgem_init(struct kgem *kgem, int fd, struct
pci_device *dev, unsigned gen)
kgem-next_request = __kgem_request_alloc(kgem);
DBG((%s: cpu bo enabled %d: llc? %d, set-cache-level? %d, userptr?
%d\n, __FUNCTION__,
-!DBG_NO_CPU (kgem-has_llc | kgem-has_userptr |
kgem-has_cacheing),
-kgem-has_llc, kgem-has_cacheing, kgem-has_userptr));
+!DBG_NO_CPU (kgem-has_llc | kgem-has_userptr |
kgem-has_caching),
+kgem-has_llc, kgem-has_caching, kgem-has_userptr));
VG_CLEAR(aperture);
aperture.aper_size = 0;
@@ -1344,7 +1344,7 @@ void kgem_init(struct kgem *kgem, int fd, struct
pci_device *dev, unsigned gen)
if (kgem-max_copy_tile_size 16*PAGE_SIZE)
kgem-max_copy_tile_size = 16*PAGE_SIZE;
- if (kgem-has_llc | kgem-has_cacheing | kgem-has_userptr) {
+ if (kgem-has_llc | kgem-has_caching | kgem-has_userptr) {
if (kgem-large_object_size kgem-max_cpu_size)
kgem-large_object_size = kgem-max_cpu_size;
} else
@@ -1870,7 +1870,7 @@ search_snoop_cache(struct kgem *kgem, unsigned int
num_pages, unsigned flags)
DBG((%s: num_pages=%d, flags=%x\n, __FUNCTION__, num_pages, flags));
- if ((kgem-has_cacheing | kgem-has_userptr) == 0)
+ if ((kgem-has_caching |
[Intel-gfx] [PATCH] drm/i915: clarify error paths in create_stolen_for_preallocated
Use the standard inversely ordered goto label stack for everything.
Spotted while reviewing place where we might need to to call
vma_destroy but failed to do so.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem_stolen.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index e20d649..7f4c510 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -392,8 +392,7 @@ i915_gem_object_create_stolen_for_preallocated(struct
drm_device *dev,
ret = drm_mm_reserve_node(ggtt-mm, vma-node);
if (ret) {
DRM_DEBUG_KMS(failed to allocate stolen GTT space\n);
- i915_gem_vma_destroy(vma);
- goto err_out;
+ goto err_vma;
}
}
@@ -404,6 +403,8 @@ i915_gem_object_create_stolen_for_preallocated(struct
drm_device *dev,
return obj;
+err_vma:
+ i915_gem_vma_destroy(vma);
err_out:
drm_mm_put_block(stolen);
drm_gem_object_unreference(obj-base);
--
1.8.4.rc1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Correct misspelled caching
On Wed, Aug 14, 2013 at 10:01:13AM +0200, Sedat Dilek wrote: Signed-off-by: Sedat Dilek sedat.di...@gmail.com Am I the only one who reads that as a hard 'ch' without the 'e'? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] [v2] drm/i915: Remove node only when allocated
On Tue, Aug 13, 2013 at 06:09:06PM -0700, Ben Widawsky wrote: VMAs can be created and not bound. One may think of it as lazy cleanup, and safely gloss over the conditions which manufacture it. In either case, when the object backing the i915 vma is destroyed, we must cleanup the vma without stumbling into a bunch of pitfalls that assume the vma is bound. NOTE: I was pretty certain the above condition could only happen when we introduced the use of VMAs being looked up at execbuf, and already existing. Paulo has hit this though, so I must be missing something. As I believe the patch is correct anyway, therefore I won't scratch my head too hard. If we end up calling evict_everything from i915_gem_object_bind_to_vm then we'll hit this. One more reason for a testcase here ;-) I'll amend the commit message and merge this. I've also applied a tiny bikeshed I've created while reviewing existing vma_create/destroy callsites. -Daniel v2: use goto destroy as a compromise (Chris) Cc: Chris Wilson ch...@chris-wilson.co.uk Cc: Paulo Zanoni paulo.r.zan...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net --- 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 3d9e248b..4a58ead 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2606,6 +2606,9 @@ int i915_vma_unbind(struct i915_vma *vma) if (list_empty(vma-vma_link)) return 0; + if (!drm_mm_node_allocated(vma-node)) + goto destroy; + if (obj-pin_count) return -EBUSY; @@ -2643,6 +2646,8 @@ int i915_vma_unbind(struct i915_vma *vma) obj-map_and_fenceable = true; drm_mm_remove_node(vma-node); + +destroy: i915_gem_vma_destroy(vma); /* Since the unbound list is global, only move to that list if -- 1.8.3.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Correct misspelled caching
On Wed, Aug 14, 2013 at 10:05 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Aug 14, 2013 at 10:01:13AM +0200, Sedat Dilek wrote: Signed-off-by: Sedat Dilek sedat.di...@gmail.com Am I the only one who reads that as a hard 'ch' without the 'e'? I am not an English native or teacher: But from what I know it's... to cache | caching | cached. Google gives me no hits for cach*e*ing. Anyway, the spelling should be consistent in kernel-drm | libdrm | intel-ddx | etc. - Sedat - ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Correct misspelled caching
On Wed, Aug 14, 2013 at 10:09:23AM +0200, Sedat Dilek wrote: On Wed, Aug 14, 2013 at 10:05 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Aug 14, 2013 at 10:01:13AM +0200, Sedat Dilek wrote: Signed-off-by: Sedat Dilek sedat.di...@gmail.com Am I the only one who reads that as a hard 'ch' without the 'e'? I am not an English native or teacher: But from what I know it's... to cache | caching | cached. Google gives me no hits for cach*e*ing. Anyway, the spelling should be consistent in kernel-drm | libdrm | intel-ddx | etc. It was. I blame Ben. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] [v2] drm/i915: Remove node only when allocated
On Wed, Aug 14, 2013 at 10:06:30AM +0200, Daniel Vetter wrote: On Tue, Aug 13, 2013 at 06:09:06PM -0700, Ben Widawsky wrote: VMAs can be created and not bound. One may think of it as lazy cleanup, and safely gloss over the conditions which manufacture it. In either case, when the object backing the i915 vma is destroyed, we must cleanup the vma without stumbling into a bunch of pitfalls that assume the vma is bound. NOTE: I was pretty certain the above condition could only happen when we introduced the use of VMAs being looked up at execbuf, and already existing. Paulo has hit this though, so I must be missing something. As I believe the patch is correct anyway, therefore I won't scratch my head too hard. If we end up calling evict_everything from i915_gem_object_bind_to_vm then we'll hit this. One more reason for a testcase here ;-) I'll amend the commit message and merge this. I've also applied a tiny bikeshed I've created while reviewing existing vma_create/destroy callsites. Actually evict_everything isn't in the callpath, and there's no memory allocation where the shrinker might play havoc. Furthermore the pages are pinned so the shrinker shouldn't be able to sneak in at all. This is a bit unsettling, I need to think more about this. I'll wait with merging this for now. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] intel: Update i915_drm.h and correct misspelled caching
AFAICS, there are more updates needed to be in sync with recent kernel-drm.
I fell over the misspelling when digging into an issue in Linux-next.
The spelling should be consistent in kernel-drm, libdrm, intel-ddx, etc.
Here, I had a look especially at the defined macros (defines).
Signed-off-by: Sedat Dilek sedat.di...@gmail.com
---
include/drm/i915_drm.h | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index aa983f3..61a8407 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -195,8 +195,8 @@ typedef struct _drm_i915_sarea {
#define DRM_I915_GEM_WAIT 0x2c
#define DRM_I915_GEM_CONTEXT_CREATE0x2d
#define DRM_I915_GEM_CONTEXT_DESTROY 0x2e
-#define DRM_I915_GEM_SET_CACHEING 0x2f
-#define DRM_I915_GEM_GET_CACHEING 0x30
+#define DRM_I915_GEM_SET_CACHING 0x2f
+#define DRM_I915_GEM_GET_CACHING 0x30
#define DRM_I915_REG_READ 0x31
#define DRM_IOCTL_I915_INITDRM_IOW( DRM_COMMAND_BASE +
DRM_I915_INIT, drm_i915_init_t)
@@ -222,8 +222,8 @@ typedef struct _drm_i915_sarea {
#define DRM_IOCTL_I915_GEM_PIN DRM_IOWR(DRM_COMMAND_BASE +
DRM_I915_GEM_PIN, struct drm_i915_gem_pin)
#define DRM_IOCTL_I915_GEM_UNPIN DRM_IOW(DRM_COMMAND_BASE +
DRM_I915_GEM_UNPIN, struct drm_i915_gem_unpin)
#define DRM_IOCTL_I915_GEM_BUSYDRM_IOWR(DRM_COMMAND_BASE +
DRM_I915_GEM_BUSY, struct drm_i915_gem_busy)
-#define DRM_IOCTL_I915_GEM_SET_CACHEING
DRM_IOW(DRM_COMMAND_BASE + DRM_I915_GEM_SET_CACHEING, struct
drm_i915_gem_cacheing)
-#define DRM_IOCTL_I915_GEM_GET_CACHEING
DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_GET_CACHEING, struct
drm_i915_gem_cacheing)
+#define DRM_IOCTL_I915_GEM_SET_CACHING DRM_IOW(DRM_COMMAND_BASE +
DRM_I915_GEM_SET_CACHING, struct drm_i915_gem_caching)
+#define DRM_IOCTL_I915_GEM_GET_CACHING DRM_IOWR(DRM_COMMAND_BASE +
DRM_I915_GEM_GET_CACHING, struct drm_i915_gem_caching)
#define DRM_IOCTL_I915_GEM_THROTTLEDRM_IO ( DRM_COMMAND_BASE +
DRM_I915_GEM_THROTTLE)
#define DRM_IOCTL_I915_GEM_ENTERVT DRM_IO(DRM_COMMAND_BASE +
DRM_I915_GEM_ENTERVT)
#define DRM_IOCTL_I915_GEM_LEAVEVT DRM_IO(DRM_COMMAND_BASE +
DRM_I915_GEM_LEAVEVT)
@@ -706,21 +706,22 @@ struct drm_i915_gem_busy {
__u32 busy;
};
-#define I915_CACHEING_NONE 0
-#define I915_CACHEING_CACHED 1
+#define I915_CACHING_NONE 0
+#define I915_CACHING_CACHED1
+#define I915_CACHING_DISPLAY 2
-struct drm_i915_gem_cacheing {
+struct drm_i915_gem_caching {
/**
-* Handle of the buffer to set/get the cacheing level of. */
+* Handle of the buffer to set/get the caching level of. */
__u32 handle;
/**
* Cacheing level to apply or return value
*
-* bits0-15 are for generic cacheing control (i.e. the above defined
+* bits0-15 are for generic caching control (i.e. the above defined
* values). bits16-31 are reserved for platform-specific variations
* (e.g. l3$ caching on gen7). */
- __u32 cacheing;
+ __u32 caching;
};
#define I915_TILING_NONE 0
--
1.8.3.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] [v2] drm/i915: Remove node only when allocated
On Tue, Aug 13, 2013 at 06:09:06PM -0700, Ben Widawsky wrote: VMAs can be created and not bound. One may think of it as lazy cleanup, and safely gloss over the conditions which manufacture it. In either case, when the object backing the i915 vma is destroyed, we must cleanup the vma without stumbling into a bunch of pitfalls that assume the vma is bound. NOTE: I was pretty certain the above condition could only happen when we introduced the use of VMAs being looked up at execbuf, and already existing. Paulo has hit this though, so I must be missing something. As I believe the patch is correct anyway, therefore I won't scratch my head too hard. v2: use goto destroy as a compromise (Chris) Cc: Chris Wilson ch...@chris-wilson.co.uk Cc: Paulo Zanoni paulo.r.zan...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: use vma-node directly and rewrap mapfence in bind
Use () to make for neater alignment of the split lines, too. With this
we ditch another jump through the obj_gtt_size/offset indirection
maze.
Cc: Ben Widawsky benjamin.widaw...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6cb4467..fd48724 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3205,12 +3205,11 @@ search_free:
if (i915_is_ggtt(vm)) {
bool mappable, fenceable;
- fenceable =
- i915_gem_obj_ggtt_size(obj) == fence_size
- (i915_gem_obj_ggtt_offset(obj) (fence_alignment - 1))
== 0;
+ fenceable = (vma-node.size == fence_size
+(vma-node.start (fence_alignment - 1)) == 0);
- mappable =
- vma-node.start + obj-base.size =
dev_priv-gtt.mappable_end;
+ mappable = (vma-node.start + obj-base.size =
+ dev_priv-gtt.mappable_end);
obj-map_and_fenceable = mappable fenceable;
}
--
1.8.4.rc1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Drop the overzealous warning from i915_gem_set_cache_level
On Tue, Aug 13, 2013 at 03:37:56PM +0300, Ville Syrjälä wrote: On Tue, Aug 13, 2013 at 01:20:13PM +0100, Chris Wilson wrote: On Tue, Aug 13, 2013 at 03:12:59PM +0300, Ville Syrjälä wrote: Thinking about this stuff a bit, I think I actually came up with a scenario where we would currently fail to invalidate the CPU cache between non-snooped GPU/GTT access and CPU access: 1. make bo non-snooped w/ pin_display=true (wd=0, rd|=gtt) 2. set to CPU read domain (wd=0 rd|=cpu) 3. set to GTT (or GPU) write domain (wd=gtt, rd=gtt) - CPU cache is stale after this point 4. make bo snooped - pin_display=true still so we directly set (wd=cpu, rd=cpu) 5. set to CPU domain - CPU cache is still stale You will also find the scanout reads stale data as well. Well, assuming you actually write something to the bo w/ the CPU. If not, then it keeps scanning out the correct data. I think an if (obj-pin_display) return -EBUSY; in the set_caching ioctl would be good to fix this. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm: WARN when removing unallocated node
On Tue, Aug 13, 2013 at 06:09:08PM -0700, Ben Widawsky wrote: The conditional is usually a recoverable driver bug, and so WARNing, and preventing the drm_mm code from doing potential damage (BUG) is desirable. This issue was hit and fixed twice while developing the i915 multiple address space code. The first fix is the patch just before this, and is hit on an not frequently occuring error path. Another was fixed during patch iteration, so it's hard to see from the patch: commit c6cfb325677ea6305fb19acf3a4d14ea267f923e Author: Ben Widawsky b...@bwidawsk.net Date: Fri Jul 5 14:41:06 2013 -0700 drm/i915: Embed drm_mm_node in i915 gem obj From the intel-gfx mailing list, we discussed this: References: 20130705191235.ga3...@bwidawsk.net Cc: Dave Airlie airl...@redhat.com CC: dri-de...@lists.freedesktop.org Acked-by: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Ben Widawsky b...@bwidawsk.net Patches 23 of this series are merged to dinq, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Drop the overzealous warning from i915_gem_set_cache_level
On Wed, Aug 14, 2013 at 10:49:11AM +0200, Daniel Vetter wrote: On Tue, Aug 13, 2013 at 03:37:56PM +0300, Ville Syrjälä wrote: On Tue, Aug 13, 2013 at 01:20:13PM +0100, Chris Wilson wrote: On Tue, Aug 13, 2013 at 03:12:59PM +0300, Ville Syrjälä wrote: Thinking about this stuff a bit, I think I actually came up with a scenario where we would currently fail to invalidate the CPU cache between non-snooped GPU/GTT access and CPU access: 1. make bo non-snooped w/ pin_display=true (wd=0, rd|=gtt) 2. set to CPU read domain (wd=0 rd|=cpu) 3. set to GTT (or GPU) write domain (wd=gtt, rd=gtt) - CPU cache is stale after this point 4. make bo snooped - pin_display=true still so we directly set (wd=cpu, rd=cpu) 5. set to CPU domain - CPU cache is still stale You will also find the scanout reads stale data as well. Well, assuming you actually write something to the bo w/ the CPU. If not, then it keeps scanning out the correct data. I think an if (obj-pin_display) return -EBUSY; in the set_caching ioctl would be good to fix this. And we already do that check (as a result of obj-pin_count). Sorted. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Split up execbuf vma conversion
Hi Ben, For my own review benefit I've slashed your patch into pieces and split out 3 prep patches. Overall diff is unchanged with the exception of a now stale FIXME comment that I've killed. Still need to do the in-depth review of the last patch, but looks good thus far. Cheers, Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915: s/obj-exec_list/obj-obj_exec_link
From: Ben Widawsky b...@bwidawsk.net
To convert the execbuf code over to use vmas natively we need to
shuffle the exec_list a bit. This patch here just prepares things with
the debugfs code, which also uses the old exec_list list_head, newly
called obj_exec_link.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
[danvet: Split out from Ben's big patch.]
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_debugfs.c | 12 ++--
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_gem.c | 1 +
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index eb87865..4785d8c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -195,9 +195,9 @@ static int obj_rank_by_stolen(void *priv,
struct list_head *A, struct list_head *B)
{
struct drm_i915_gem_object *a =
- container_of(A, struct drm_i915_gem_object, exec_list);
+ container_of(A, struct drm_i915_gem_object, obj_exec_link);
struct drm_i915_gem_object *b =
- container_of(B, struct drm_i915_gem_object, exec_list);
+ container_of(B, struct drm_i915_gem_object, obj_exec_link);
return a-stolen-start - b-stolen-start;
}
@@ -221,7 +221,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m,
void *data)
if (obj-stolen == NULL)
continue;
- list_add(obj-exec_list, stolen);
+ list_add(obj-obj_exec_link, stolen);
total_obj_size += obj-base.size;
total_gtt_size += i915_gem_obj_ggtt_size(obj);
@@ -231,7 +231,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m,
void *data)
if (obj-stolen == NULL)
continue;
- list_add(obj-exec_list, stolen);
+ list_add(obj-obj_exec_link, stolen);
total_obj_size += obj-base.size;
count++;
@@ -239,11 +239,11 @@ static int i915_gem_stolen_list_info(struct seq_file *m,
void *data)
list_sort(NULL, stolen, obj_rank_by_stolen);
seq_puts(m, Stolen:\n);
while (!list_empty(stolen)) {
- obj = list_first_entry(stolen, typeof(*obj), exec_list);
+ obj = list_first_entry(stolen, typeof(*obj), obj_exec_link);
seq_puts(m,);
describe_obj(m, obj);
seq_putc(m, '\n');
- list_del_init(obj-exec_list);
+ list_del_init(obj-obj_exec_link);
}
mutex_unlock(dev-struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2e7d5f9..6532d97 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1312,6 +1312,8 @@ struct drm_i915_gem_object {
struct list_head global_list;
struct list_head ring_list;
+ /** Used in execbuf to temporarily hold a ref */
+ struct list_head obj_exec_link;
/** This object's place in the batchbuffer or on the eviction list */
struct list_head exec_list;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fd48724..219e2fb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3993,6 +3993,7 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
INIT_LIST_HEAD(obj-global_list);
INIT_LIST_HEAD(obj-ring_list);
INIT_LIST_HEAD(obj-exec_list);
+ INIT_LIST_HEAD(obj-obj_exec_link);
INIT_LIST_HEAD(obj-vma_list);
obj-ops = ops;
--
1.8.4.rc1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: Switch eviction code to use vmas
From: Ben Widawsky b...@bwidawsk.net
The execbuf wants to do relocations usings vmas, so we need a
vma-exec_list. The eviction code also uses the old obj execbuf list
for it's own book-keeping, but would really prefer to deal in vmas
only. So switch it over to the new list.
Again this is just a prep patch for the big execbuf vma conversion.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
[danvet: Split out from Ben's big execbuf vma patch.]
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_drv.h | 4
drivers/gpu/drm/i915/i915_gem.c | 1 +
drivers/gpu/drm/i915/i915_gem_evict.c | 31 ++-
3 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6532d97..b2b9836 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -563,6 +563,10 @@ struct i915_vma {
struct list_head mm_list;
struct list_head vma_link; /* Link in the object's VMA list */
+
+ /** This vma's place in the batchbuffer or on the eviction list */
+ struct list_head exec_list;
+
};
struct i915_ctx_hang_stats {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 219e2fb..122c6b0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4133,6 +4133,7 @@ struct i915_vma *i915_gem_vma_create(struct
drm_i915_gem_object *obj,
INIT_LIST_HEAD(vma-vma_link);
INIT_LIST_HEAD(vma-mm_list);
+ INIT_LIST_HEAD(vma-exec_list);
vma-vm = vm;
vma-obj = obj;
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c
b/drivers/gpu/drm/i915/i915_gem_evict.c
index 425939b..8787588 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -37,7 +37,7 @@ mark_free(struct i915_vma *vma, struct list_head *unwind)
if (vma-obj-pin_count)
return false;
- list_add(vma-obj-exec_list, unwind);
+ list_add(vma-exec_list, unwind);
return drm_mm_scan_add_block(vma-node);
}
@@ -49,7 +49,6 @@ i915_gem_evict_something(struct drm_device *dev, struct
i915_address_space *vm,
drm_i915_private_t *dev_priv = dev-dev_private;
struct list_head eviction_list, unwind_list;
struct i915_vma *vma;
- struct drm_i915_gem_object *obj;
int ret = 0;
trace_i915_gem_evict(dev, min_size, alignment, mappable);
@@ -104,14 +103,13 @@ i915_gem_evict_something(struct drm_device *dev, struct
i915_address_space *vm,
none:
/* Nothing found, clean up and bail out! */
while (!list_empty(unwind_list)) {
- obj = list_first_entry(unwind_list,
- struct drm_i915_gem_object,
+ vma = list_first_entry(unwind_list,
+ struct i915_vma,
exec_list);
- vma = i915_gem_obj_to_vma(obj, vm);
ret = drm_mm_scan_remove_block(vma-node);
BUG_ON(ret);
- list_del_init(obj-exec_list);
+ list_del_init(vma-exec_list);
}
/* We expect the caller to unpin, evict all and try again, or give up.
@@ -125,28 +123,27 @@ found:
* temporary list. */
INIT_LIST_HEAD(eviction_list);
while (!list_empty(unwind_list)) {
- obj = list_first_entry(unwind_list,
- struct drm_i915_gem_object,
+ vma = list_first_entry(unwind_list,
+ struct i915_vma,
exec_list);
- vma = i915_gem_obj_to_vma(obj, vm);
if (drm_mm_scan_remove_block(vma-node)) {
- list_move(obj-exec_list, eviction_list);
- drm_gem_object_reference(obj-base);
+ list_move(vma-exec_list, eviction_list);
+ drm_gem_object_reference(vma-obj-base);
continue;
}
- list_del_init(obj-exec_list);
+ list_del_init(vma-exec_list);
}
/* Unbinding will emit any required flushes */
while (!list_empty(eviction_list)) {
- obj = list_first_entry(eviction_list,
- struct drm_i915_gem_object,
+ vma = list_first_entry(eviction_list,
+ struct i915_vma,
exec_list);
if (ret == 0)
- ret = i915_vma_unbind(i915_gem_obj_to_vma(obj, vm));
+ ret = i915_vma_unbind(vma);
- list_del_init(obj-exec_list);
- drm_gem_object_unreference(obj-base);
+ list_del_init(vma-exec_list);
+ drm_gem_object_unreference(vma-obj-base);
[Intel-gfx] [PATCH 3/4] drm/i915: prepare bind_to_vm for preallocated vma
From: Ben Widawsky b...@bwidawsk.net
In the new execbuf code we want to track buffers using the vmas even
before they're all properly mapped. Which means that bind_to_vm needs
to deal with buffers which have preallocated vmas which aren't yet
bound.
This patch implements this prep work and adjusts our WARN/BUG checks.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
[danvet: Split out from Ben's big execbuf patch. Also move one BUG
back to its original place to deflate the diff a notch.]
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/gpu/drm/i915/i915_gem.c | 23 +--
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b2b9836..4bd66e9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1914,6 +1914,9 @@ unsigned long i915_gem_obj_size(struct
drm_i915_gem_object *o,
struct i915_address_space *vm);
struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
struct i915_address_space *vm);
+struct i915_vma *
+i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj,
+ struct i915_address_space *vm);
/* Some GGTT VM helpers */
#define obj_to_ggtt(obj) \
(((struct drm_i915_private *)(obj)-base.dev-dev_private)-gtt.base)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 122c6b0..d8f322e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3124,9 +3124,6 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object
*obj,
struct i915_vma *vma;
int ret;
- if (WARN_ON(!list_empty(obj-vma_list)))
- return -EBUSY;
-
fence_size = i915_gem_get_gtt_size(dev,
obj-base.size,
obj-tiling_mode);
@@ -3165,16 +3162,17 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object
*obj,
i915_gem_object_pin_pages(obj);
- /* FIXME: For now we only ever use 1 VMA per object */
BUG_ON(!i915_is_ggtt(vm));
- WARN_ON(!list_empty(obj-vma_list));
- vma = i915_gem_vma_create(obj, vm);
+ vma = i915_gem_obj_lookup_or_create_vma(obj, vm);
if (IS_ERR(vma)) {
ret = PTR_ERR(vma);
goto err_unpin;
}
+ /* For now we only ever use 1 vma per object */
+ WARN_ON(!list_is_singular(obj-vma_list));
+
search_free:
ret = drm_mm_insert_node_in_range_generic(vm-mm, vma-node,
size, alignment,
@@ -4883,3 +4881,16 @@ struct i915_vma *i915_gem_obj_to_vma(struct
drm_i915_gem_object *obj,
return NULL;
}
+
+struct i915_vma *
+i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj,
+ struct i915_address_space *vm)
+{
+ struct i915_vma *vma;
+
+ vma = i915_gem_obj_to_vma(obj, vm);
+ if (!vma)
+ vma = i915_gem_vma_create(obj, vm);
+
+ return vma;
+}
--
1.8.4.rc1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: Convert execbuf code to use vmas
From: Ben Widawsky b...@bwidawsk.net
In order to transition more of our code over to using a VMA instead of
an OBJ, VM pair - we must have the vma accessible at execbuf time. Up
until now, we've only had a VMA when actually binding an object.
The previous patch helped handle the distinction on bound vs. unbound.
This patch will help us catch leaks, and other issues before we actually
shuffle a bunch of stuff around.
This attempts to convert all the execbuf code to speak in vmas. Since
the execbuf code is very self contained it was a nice isolated
conversion.
The meat of the code is about turning eb_objects into eb_vma, and then
wiring up the rest of the code to use vmas instead of obj, vm pairs.
Unfortunately, to do this, we must move the exec_list link from the obj
structure. This list is reused in the eviction code, so we must also
modify the eviction code to make this work.
WARNING: This patch makes an already hotly profiled path slower. The cost is
unavoidable. In reply to this mail, I will attach the extra data.
v2: Release table lock early, and two a 2 phase vma lookup to avoid
having to use a GFP_ATOMIC. (Chris)
v3: s/obj_exec_list/obj_exec_link/
Updates to address
commit 6d2b888569d366beb4be72cacfde41adee2c25e1
Author: Chris Wilson ch...@chris-wilson.co.uk
Date: Wed Aug 7 18:30:54 2013 +0100
drm/i915: List objects allocated from stolen memory in debugfs
v4: Use obj = vma-obj for neatness in some places (Chris)
need_reloc_mappable() should return false if ppgtt (Chris)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
[danvet: Split out prep patches. Also remove a FIXME comment which is
now taken care of.]
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_drv.h| 16 +-
drivers/gpu/drm/i915/i915_gem.c| 1 -
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 320 -
3 files changed, 183 insertions(+), 154 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4bd66e9..fc35ae3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -567,6 +567,13 @@ struct i915_vma {
/** This vma's place in the batchbuffer or on the eviction list */
struct list_head exec_list;
+ /**
+* Used for performing relocations during execbuffer insertion.
+*/
+ struct hlist_node exec_node;
+ unsigned long exec_handle;
+ struct drm_i915_gem_exec_object2 *exec_entry;
+
};
struct i915_ctx_hang_stats {
@@ -1318,8 +1325,6 @@ struct drm_i915_gem_object {
struct list_head ring_list;
/** Used in execbuf to temporarily hold a ref */
struct list_head obj_exec_link;
- /** This object's place in the batchbuffer or on the eviction list */
- struct list_head exec_list;
/**
* This is set if the object is on the active lists (has pending
@@ -1405,13 +1410,6 @@ struct drm_i915_gem_object {
void *dma_buf_vmapping;
int vmapping_count;
- /**
-* Used for performing relocations during execbuffer insertion.
-*/
- struct hlist_node exec_node;
- unsigned long exec_handle;
- struct drm_i915_gem_exec_object2 *exec_entry;
-
struct intel_ring_buffer *ring;
/** Breadcrumb of last rendering to the buffer. */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d8f322e..4be3be9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3990,7 +3990,6 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
{
INIT_LIST_HEAD(obj-global_list);
INIT_LIST_HEAD(obj-ring_list);
- INIT_LIST_HEAD(obj-exec_list);
INIT_LIST_HEAD(obj-obj_exec_link);
INIT_LIST_HEAD(obj-vma_list);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 7dcf78c..c3b36f5 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -33,24 +33,24 @@
#include intel_drv.h
#include linux/dma_remapping.h
-struct eb_objects {
- struct list_head objects;
+struct eb_vmas {
+ struct list_head vmas;
int and;
union {
- struct drm_i915_gem_object *lut[0];
+ struct i915_vma *lut[0];
struct hlist_head buckets[0];
};
};
-static struct eb_objects *
-eb_create(struct drm_i915_gem_execbuffer2 *args)
+static struct eb_vmas *
+eb_create(struct drm_i915_gem_execbuffer2 *args, struct i915_address_space *vm)
{
- struct eb_objects *eb = NULL;
+ struct eb_vmas *eb = NULL;
if (args-flags I915_EXEC_HANDLE_LUT) {
int size = args-buffer_count;
- size *= sizeof(struct drm_i915_gem_object *);
- size += sizeof(struct eb_objects);
+ size *= sizeof(struct i915_vma *);
+ size += sizeof(struct
Re: [Intel-gfx] [PATCH 1/4] drm/i915: s/obj-exec_list/obj-obj_exec_link
On Wed, Aug 14, 2013 at 11:38:33AM +0200, Daniel Vetter wrote:
From: Ben Widawsky b...@bwidawsk.net
To convert the execbuf code over to use vmas natively we need to
shuffle the exec_list a bit. This patch here just prepares things with
the debugfs code, which also uses the old exec_list list_head, newly
called obj_exec_link.
Meh, patch subject is missing a in debugfs at the end. I'll fix this
when merging.
-Daniel
Signed-off-by: Ben Widawsky b...@bwidawsk.net
[danvet: Split out from Ben's big patch.]
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_debugfs.c | 12 ++--
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_gem.c | 1 +
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index eb87865..4785d8c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -195,9 +195,9 @@ static int obj_rank_by_stolen(void *priv,
struct list_head *A, struct list_head *B)
{
struct drm_i915_gem_object *a =
- container_of(A, struct drm_i915_gem_object, exec_list);
+ container_of(A, struct drm_i915_gem_object, obj_exec_link);
struct drm_i915_gem_object *b =
- container_of(B, struct drm_i915_gem_object, exec_list);
+ container_of(B, struct drm_i915_gem_object, obj_exec_link);
return a-stolen-start - b-stolen-start;
}
@@ -221,7 +221,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m,
void *data)
if (obj-stolen == NULL)
continue;
- list_add(obj-exec_list, stolen);
+ list_add(obj-obj_exec_link, stolen);
total_obj_size += obj-base.size;
total_gtt_size += i915_gem_obj_ggtt_size(obj);
@@ -231,7 +231,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m,
void *data)
if (obj-stolen == NULL)
continue;
- list_add(obj-exec_list, stolen);
+ list_add(obj-obj_exec_link, stolen);
total_obj_size += obj-base.size;
count++;
@@ -239,11 +239,11 @@ static int i915_gem_stolen_list_info(struct seq_file
*m, void *data)
list_sort(NULL, stolen, obj_rank_by_stolen);
seq_puts(m, Stolen:\n);
while (!list_empty(stolen)) {
- obj = list_first_entry(stolen, typeof(*obj), exec_list);
+ obj = list_first_entry(stolen, typeof(*obj), obj_exec_link);
seq_puts(m,);
describe_obj(m, obj);
seq_putc(m, '\n');
- list_del_init(obj-exec_list);
+ list_del_init(obj-obj_exec_link);
}
mutex_unlock(dev-struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2e7d5f9..6532d97 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1312,6 +1312,8 @@ struct drm_i915_gem_object {
struct list_head global_list;
struct list_head ring_list;
+ /** Used in execbuf to temporarily hold a ref */
+ struct list_head obj_exec_link;
/** This object's place in the batchbuffer or on the eviction list */
struct list_head exec_list;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fd48724..219e2fb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3993,6 +3993,7 @@ void i915_gem_object_init(struct drm_i915_gem_object
*obj,
INIT_LIST_HEAD(obj-global_list);
INIT_LIST_HEAD(obj-ring_list);
INIT_LIST_HEAD(obj-exec_list);
+ INIT_LIST_HEAD(obj-obj_exec_link);
INIT_LIST_HEAD(obj-vma_list);
obj-ops = ops;
--
1.8.4.rc1
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/12] drm: Add a helper to forge HDMI vendor infoframes
More typo fallout:
On Wednesday 14 August 2013 00:17:27 Damien Lespiau wrote:
This can then be used by DRM drivers to setup their vendor infoframes.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/drm_edid.c | 36
include/drm/drm_edid.h | 4
2 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9a07a33..83e1202 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3262,3 +3262,39 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
hdmi_avi_infoframe *frame,
return 0;
}
EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
+
+/**
+ * drm_hdmi_vendor_infoframe_from_display_mode() - fill an HDMI infoframe
with
+ * data from a DRM display mode
+ * @frame: HDMI vendor infoframe
+ * @mode: DRM display mode
+ *
+ * Note that there's is a need to send HDMI vendor infoframes only when
using a
+ * 4k or stereoscopic 3D mode. So when giving any other mode as input this
+ * function will return -EINVAL, error that can be safely ignored.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int
+drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_hdmi_infoframe
*frame,
+ const struct drm_display_mode *mode)
+{
+ int err;
+ u8 vic;
+
+ if (!frame || !mode)
+ return -EINVAL;
+
+ vic = drm_match_hmdi_mode(mode);
Should be hdmi again.
+ if (!vic)
+ return -EINVAL;
+
+ err = hdmi_hdmi_infoframe_init(frame);
+ if (err 0)
+ return err;
+
+ frame-vic = vic;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode);
snip
--
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.com
signature.asc
Description: This is a digitally signed message part.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] HDMI 4k support v2
On Wednesday 14 August 2013 00:17:16 Damien Lespiau wrote:
Following up the first instance of this series:
http://lists.freedesktop.org/archives/dri-devel/2013-August/043125.html
Here is a v2 with Ville's review pass and a few additions:
- Alternate clock modes for 4k resolutions
- HDMI vendor specific infoframe support in drivers/video/hdmi.c
- Enabling of those vendor specific infoframes in the i915 driver
Along the way, a tegra patch was needed for a small consolidation of the code
packing vendor specific infoframes. This patch has only been compile-tested.
On Intel, it's possible to read back the programmed infoframe buffers with
intel-gpu-tools' intel_infoframe and this gives:
Vendor InfoFrame:
- frequency: every vsync
- raw:
f4050181 000c0347 0320
004c
- vendor Id: 0x000c03 (HDMI)
- video format: 0x001
- HDMI VIC: 3
after a $ xrandr --output HDMI1 --mode 3840x2160 -r 24
With the typos fixed, feel free to add:
Reviewed-by: Simon Farnsworth simon.farnswo...@onelan.co.uk
--
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.com
signature.asc
Description: This is a digitally signed message part.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH igt] intel_infoframes: Add support for decoding HDMI VICs
Reviewed-by: Simon Farnsworth simon.farnswo...@onelan.co.uk
On Wednesday 14 August 2013 00:19:14 Damien Lespiau wrote:
The HDMI vendor infoframe can contain a HDMI VIC (as of HDMI 1.4, only
used for 4k formats).
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
tools/intel_infoframes.c | 26 ++
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/tools/intel_infoframes.c b/tools/intel_infoframes.c
index 09fdcb9..b6d289f 100644
--- a/tools/intel_infoframes.c
+++ b/tools/intel_infoframes.c
@@ -184,8 +184,13 @@ typedef union {
uint8_t Rsvd0:5;
uint8_t video_format :3;
- uint8_t Rsvd1 :4;
- uint8_t s3d_structure :4;
+ union {
+ uint8_t vic;
+ struct {
+ uint8_t Rsvd1 :4;
+ uint8_t s3d_structure :4;
+ } s3d;
+ } pb5;
uint8_t Rsvd2:4;
uint8_t s3d_ext_data :4;
@@ -467,13 +472,26 @@ static const char *s3d_structure_to_string(int format)
static void dump_vendor_hdmi(DipInfoFrame *frame)
{
+ int vic_present = frame-vendor.video_format 0x1;
int s3d_present = frame-vendor.video_format 0x2;
printf(- video format: 0x%03x %s\n, frame-vendor.video_format,
s3d_present ? (3D) : );
- if (s3d_present)
+
+ if (vic_present s3d_present) {
+ printf(Error: HDMI VIC and S3D bits set. Only one of those
+ at a time is valid\n);
+ return;
+ }
+
+ if (vic_present)
+ printf(- HDMI VIC: %d\n, frame-vendor.pb5.vic);
+ else if (s3d_present) {
+ int s3d_structure = frame-vendor.pb5.s3d.s3d_structure;
+
printf(- 3D Format: %s\n,
-s3d_structure_to_string(frame-vendor.s3d_structure));
+s3d_structure_to_string(s3d_structure));
+ }
}
static void dump_vendor_info(Transcoder transcoder)
--
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.com
signature.asc
Description: This is a digitally signed message part.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: inline vma_create into lookup_or_create_vma
In the execbuf code we don't clean up any vmas which ended up not
getting bound for code simplicity. To make sure that we don't end up
creating multiple vma for the same vm kill the somewhat dangerous
vma_create function and inline it into lookup_or_create.
This is just a safety measure to prevent surprises in the future.
Also update the somewhat confused comment in the execbuf code and
clarify what kind of magic is going on with a new one.
Cc: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
That's the only concern I could come up with when reading the execbuf
vma conversion patch. So looks good and I'll slurp it all in as soon
as some more head scratching is done for the very first patch in this
series about the vma_unbind fix to only call vma_destroy if the vma
isn't bound.
-Daniel
---
drivers/gpu/drm/i915/i915_drv.h| 2 --
drivers/gpu/drm/i915/i915_gem.c| 41 +-
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 11 +---
drivers/gpu/drm/i915/i915_gem_stolen.c | 2 +-
4 files changed, 26 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fc35ae3..b06a90f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1748,8 +1748,6 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
size_t size);
void i915_gem_free_object(struct drm_gem_object *obj);
-struct i915_vma *i915_gem_vma_create(struct drm_i915_gem_object *obj,
-struct i915_address_space *vm);
void i915_gem_vma_destroy(struct i915_vma *vma);
int __must_check i915_gem_object_pin(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 4be3be9..bb029a4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4121,28 +4121,6 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
i915_gem_object_free(obj);
}
-struct i915_vma *i915_gem_vma_create(struct drm_i915_gem_object *obj,
-struct i915_address_space *vm)
-{
- struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL);
- if (vma == NULL)
- return ERR_PTR(-ENOMEM);
-
- INIT_LIST_HEAD(vma-vma_link);
- INIT_LIST_HEAD(vma-mm_list);
- INIT_LIST_HEAD(vma-exec_list);
- vma-vm = vm;
- vma-obj = obj;
-
- /* Keep GGTT vmas first to make debug easier */
- if (i915_is_ggtt(vm))
- list_add(vma-vma_link, obj-vma_list);
- else
- list_add_tail(vma-vma_link, obj-vma_list);
-
- return vma;
-}
-
void i915_gem_vma_destroy(struct i915_vma *vma)
{
WARN_ON(vma-node.allocated);
@@ -4888,8 +4866,23 @@ i915_gem_obj_lookup_or_create_vma(struct
drm_i915_gem_object *obj,
struct i915_vma *vma;
vma = i915_gem_obj_to_vma(obj, vm);
- if (!vma)
- vma = i915_gem_vma_create(obj, vm);
+ if (!vma) {
+ vma = kzalloc(sizeof(*vma), GFP_KERNEL);
+ if (vma == NULL)
+ return ERR_PTR(-ENOMEM);
+
+ INIT_LIST_HEAD(vma-vma_link);
+ INIT_LIST_HEAD(vma-mm_list);
+ INIT_LIST_HEAD(vma-exec_list);
+ vma-vm = vm;
+ vma-obj = obj;
+
+ /* Keep GGTT vmas first to make debug easier */
+ if (i915_is_ggtt(vm))
+ list_add(vma-vma_link, obj-vma_list);
+ else
+ list_add_tail(vma-vma_link, obj-vma_list);
+ }
return vma;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index c3b36f5..9b3b5f8 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -123,11 +123,16 @@ eb_lookup_vmas(struct eb_vmas *eb,
list_for_each_entry(obj, objects, obj_exec_link) {
struct i915_vma *vma;
+ /*
+* NOTE: We can leak any vmas created here when something fails
+* later on. But that's no issue since vma_unbind can deal with
+* vmas which are not actually bound. And since only
+* lookup_or_create exists as an interface to get at the vma
+* from the (obj, vm) we don't run the risk of creating
+* duplicated vmas for the same vm.
+*/
vma = i915_gem_obj_lookup_or_create_vma(obj, vm);
if (IS_ERR(vma)) {
- /* XXX: We don't need an error path fro vma because if
-* the vma was created just for this execbuf, object
-* unreference should kill it off.*/
Re: [Intel-gfx] [PATCH] drm/i915: Drop the overzealous warning from i915_gem_set_cache_level
On Wed, Aug 14, 2013 at 09:54:05AM +0100, Chris Wilson wrote: On Wed, Aug 14, 2013 at 10:49:11AM +0200, Daniel Vetter wrote: On Tue, Aug 13, 2013 at 03:37:56PM +0300, Ville Syrjälä wrote: On Tue, Aug 13, 2013 at 01:20:13PM +0100, Chris Wilson wrote: On Tue, Aug 13, 2013 at 03:12:59PM +0300, Ville Syrjälä wrote: Thinking about this stuff a bit, I think I actually came up with a scenario where we would currently fail to invalidate the CPU cache between non-snooped GPU/GTT access and CPU access: 1. make bo non-snooped w/ pin_display=true (wd=0, rd|=gtt) 2. set to CPU read domain (wd=0 rd|=cpu) 3. set to GTT (or GPU) write domain (wd=gtt, rd=gtt) - CPU cache is stale after this point 4. make bo snooped - pin_display=true still so we directly set (wd=cpu, rd=cpu) 5. set to CPU domain - CPU cache is still stale You will also find the scanout reads stale data as well. Well, assuming you actually write something to the bo w/ the CPU. If not, then it keeps scanning out the correct data. I think an if (obj-pin_display) return -EBUSY; in the set_caching ioctl would be good to fix this. And we already do that check (as a result of obj-pin_count). Sorted. Indeed. Patch merged to dinq (with a pimped commit message), thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/12] drm/edid: Parse the HDMI CEA block and look for 4k modes
Minor typo - feel free to ignore:
On Wednesday 14 August 2013 00:17:19 Damien Lespiau wrote:
HDMI 1.4 adds 4 4k x 2k modes in the the CEA vendor specific block.
With this commit, we now parse this block and expose the 4k modes that
we find there.
v2: Fix the 4096x2160 string (nice catch!), add comments about
do_hdmi_vsdb_modes() arguments and make it clearer that offset is
relative to the end of the required fields of the HDMI VSDB
(Ville Syrjälä)
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Tested-by: Cancan Feng cancan.f...@intel.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67030
---
drivers/gpu/drm/drm_edid.c | 124
+++--
1 file changed, 109 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9e9b6ed..0faa08e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
snip
@@ -2465,6 +2495,68 @@ do_cea_modes(struct drm_connector *connector, const u8
*db, u8 len)
return modes;
}
+/*
+ * do_hdmi_vsdb_modes - Parse the HDMI Vendor Specific data block
+ * @connector: connector corresponding to the HDMI sink
+ * @db: start of the CEA vendor specific block
+ * @len: length of the CEA block payload, ie. one can access up to db[len]
+ *
+ * Parses the HDMI VSDB looking for modes to add to @connector.
+ */
+static int
+do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len)
+{
+ struct drm_device *dev = connector-dev;
+ int modes = 0, offset = 0, i;
+ u8 vic_len;
+
+ if (len 8)
+ goto out;
+
+ /* no HDMI_Video_Present */
+ if (!(db[8] (1 5)))
+ goto out;
+
+ /* Latency_Fields_Present */
+ if (db[8] (1 7))
+ offset += 2;
+
+ /* I_Latency_Fields_Present */
+ if (db[8] (1 6))
+ offset += 2;
+
+ /* the declared length is not long enough for the 2 first bytes
+ * of additional video format capabilities */
+ offset += 2;
+ if (len (8 + offset))
+ goto out;
+
+ vic_len = db[8 + offset] 5;
+
+ for (i = 0; i vic_len len = (9 + offset + i); i++) {
+ struct drm_display_mode *newmode;
+ u8 vic;
+
+ vic = db[9 + offset + i];
+
+ vic--; /* VICs start at 1 */
+ if (vic = ARRAY_SIZE(edid_4k_modes)) {
+ DRM_ERROR(Unknow HDMI VIC: %d\n, vic);
^^
Missing n - should be Unknown
+ continue;
+ }
+
+ newmode = drm_mode_duplicate(dev, edid_4k_modes[vic]);
+ if (!newmode)
+ continue;
+
+ drm_mode_probed_add(connector, newmode);
+ modes++;
+ }
+
+out:
+ return modes;
+}
+
static int
cea_db_payload_len(const u8 *db)
{
snip
--
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.com
signature.asc
Description: This is a digitally signed message part.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/12] drm: Add support for alternate clocks of 4k modes
On Wed, Aug 14, 2013 at 12:17:20AM +0100, Damien Lespiau wrote:
Suggested-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/drm_edid.c | 68
++
1 file changed, 62 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0faa08e..606335f 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2409,6 +2409,54 @@ u8 drm_match_cea_mode(const struct drm_display_mode
*to_match)
}
EXPORT_SYMBOL(drm_match_cea_mode);
+/*
+ * Calculate the alternate clock for HDMI modes (those from the HDMI vendor
+ * specific block).
+ *
+ * It's almost like cea_mode_alternate_clock(), we just need to add an
+ * exception for the VIC 4 mode (4096x2160@24Hz): no alternate clock for this
+ * one.
+ */
+static unsigned int
+hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode)
+{
+ if (hdmi_mode-vdisplay == 4096 hdmi_mode-hdisplay == 2160)
+ return hdmi_mode-clock;
+
+ return cea_mode_alternate_clock(hdmi_mode);
+}
+
+/*
+ * drm_match_cea_mode - look for a HDMI mode matching given mode
^^^
Fix that, and you get:
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
+ * @to_match: display mode
+ *
+ * An HDMI mode is one defined in the HDMI vendor specific block.
+ *
+ * Returns the HDMI Video ID (VIC) of the mode or 0 if it isn't one.
+ */
+static u8 drm_match_hmdi_mode(const struct drm_display_mode *to_match)
+{
+ u8 mode;
+
+ if (!to_match-clock)
+ return 0;
+
+ for (mode = 0; mode ARRAY_SIZE(edid_4k_modes); mode++) {
+ const struct drm_display_mode *hdmi_mode = edid_4k_modes[mode];
+ unsigned int clock1, clock2;
+
+ /* Make sure to also match alternate clocks */
+ clock1 = hdmi_mode-clock;
+ clock2 = hdmi_mode_alternate_clock(hdmi_mode);
+
+ if ((KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock1) ||
+ KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock2))
+ drm_mode_equal_no_clocks(to_match, hdmi_mode))
+ return mode + 1;
+ }
+ return 0;
+}
+
static int
add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
{
@@ -2426,18 +2474,26 @@ add_alternate_cea_modes(struct drm_connector
*connector, struct edid *edid)
* with the alternate clock for certain CEA modes.
*/
list_for_each_entry(mode, connector-probed_modes, head) {
- const struct drm_display_mode *cea_mode;
+ const struct drm_display_mode *cea_mode = NULL;
struct drm_display_mode *newmode;
- u8 cea_mode_idx = drm_match_cea_mode(mode) - 1;
+ u8 mode_idx = drm_match_cea_mode(mode) - 1;
unsigned int clock1, clock2;
- if (cea_mode_idx = ARRAY_SIZE(edid_cea_modes))
- continue;
+ if (mode_idx ARRAY_SIZE(edid_cea_modes)) {
+ cea_mode = edid_cea_modes[mode_idx];
+ clock2 = cea_mode_alternate_clock(cea_mode);
+ } else {
+ mode_idx = drm_match_hmdi_mode(mode) - 1;
+ if (mode_idx ARRAY_SIZE(edid_4k_modes)) {
+ cea_mode = edid_4k_modes[mode_idx];
+ clock2 = hdmi_mode_alternate_clock(cea_mode);
+ }
+ }
- cea_mode = edid_cea_modes[cea_mode_idx];
+ if (!cea_mode)
+ continue;
clock1 = cea_mode-clock;
- clock2 = cea_mode_alternate_clock(cea_mode);
if (clock1 == clock2)
continue;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/12] video/hdmi: Derive the bar data valid bit from the bar data fields
On Wed, Aug 14, 2013 at 12:17:22AM +0100, Damien Lespiau wrote:
Just like:
Author: Damien Lespiau damien.lesp...@intel.com
Date: Mon Aug 12 11:53:24 2013 +0100
video/hdmi: Don't let the user of this API create invalid infoframes
But this time for the horizontal/vertical bar data present bits.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/video/hdmi.c | 5 +++--
include/linux/hdmi.h | 2 --
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index e36da36..ac84215 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -101,10 +101,11 @@ ssize_t hdmi_avi_infoframe_pack(struct
hdmi_avi_infoframe *frame, void *buffer,
if (frame-active_aspect 0xf)
ptr[0] |= BIT(4);
- if (frame-horizontal_bar_valid)
+ /* Bit 3 and 2 indicate if we transmit horizontal/vertical bar data */
+ if (frame-top_bar || frame-bottom_bar)
ptr[0] |= BIT(3);
- if (frame-vertical_bar_valid)
+ if (frame-left_bar || frame-right_bar)
ptr[0] |= BIT(2);
Technically top=0,bottom=0 or left=0,right=0 is a valid bar setup,
but it would indicate that the entire picture is made up of the
bottom/right bar. I guess no one would really want to use such a
setup, and even if they do, they could just use some N!=0 for
both bars to achieve the same effect. So we don't seem to lose
anything by doing this.
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
ptr[1] = ((frame-colorimetry 0x3) 6) |
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 931474c6..b98340b 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -109,8 +109,6 @@ struct hdmi_avi_infoframe {
unsigned char version;
unsigned char length;
enum hdmi_colorspace colorspace;
- bool horizontal_bar_valid;
- bool vertical_bar_valid;
enum hdmi_scan_mode scan_mode;
enum hdmi_colorimetry colorimetry;
enum hdmi_picture_aspect picture_aspect;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/12] video/hdmi: Introduce helpers for the HDMI vendor specific infoframe
On Wed, Aug 14, 2013 at 12:17:23AM +0100, Damien Lespiau wrote:
Provide the programming model than the other infoframe types.
The generic _pack() function can't handle those yet as we need to move
the vendor OUI in the generic hdmi_vendor_infoframe structure to know
which kind of vendor infoframe we are dealing with.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/video/hdmi.c | 82
include/linux/hdmi.h | 25
2 files changed, 107 insertions(+)
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index ac84215..2059f7b 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -286,6 +286,88 @@ ssize_t hdmi_audio_infoframe_pack(struct
hdmi_audio_infoframe *frame,
EXPORT_SYMBOL(hdmi_audio_infoframe_pack);
/**
+ * hdmi_hdmi_infoframe_init() - initialize an HDMI vendor infoframe
+ * @frame: HDMI vendor infoframe
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame)
+{
+ memset(frame, 0, sizeof(*frame));
+
+ frame-type = HDMI_INFOFRAME_TYPE_VENDOR;
+ frame-version = 1;
+ frame-length = 5; /* we can hardcode the size for now as we don't
+ support neither 3D_Ext_Data nor 3D_Metadata_* */
+
+ /* 0 is a valid value for s3d_struct, so we use a special not set
+ * value */
+ frame-s3d_struct = HDMI_3D_STRUCTURE_INVALID;
+
+ return 0;
+}
+EXPORT_SYMBOL(hdmi_hdmi_infoframe_init);
+
+/**
+ * hdmi_hmdi_infoframe_pack() - write a HDMI vendor infoframe to binary
buffer
+ * @frame: HDMI infoframe
+ * @buffer: destination buffer
+ * @size: size of buffer
+ *
+ * Packs the information contained in the @frame structure into a binary
+ * representation that can be written into the corresponding controller
+ * registers. Also computes the checksum as required by section 5.3.5 of
+ * the HDMI 1.4 specification.
+ *
+ * Returns the number of bytes packed into the binary buffer or a negative
+ * error code on failure.
+ */
+ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
+ void *buffer, size_t size)
+{
+ u8 *ptr = buffer;
+ size_t length;
+
+ /* empty info frame */
+ if (frame-vic == 0 frame-s3d_struct == HDMI_3D_STRUCTURE_INVALID)
+ return -EINVAL;
+
+ /* only one of those can be supplied */
+ if (frame-vic != 0 frame-s3d_struct != HDMI_3D_STRUCTURE_INVALID)
+ return -EINVAL;
+
+ length = HDMI_INFOFRAME_HEADER_SIZE + frame-length;
+
+ if (size length)
+ return -ENOSPC;
+
+ memset(buffer, 0, size);
+
+ ptr[0] = frame-type;
+ ptr[1] = frame-version;
+ ptr[2] = frame-length;
+ ptr[3] = 0; /* checksum */
+
+ /* HDMI OUI */
+ ptr[4] = 0x03;
+ ptr[5] = 0x0c;
+ ptr[6] = 0x00;
+
+ if (frame-vic) {
+ ptr[7] = 0x1 5; /* video format */
+ ptr[8] = frame-vic;
+ } else {
+ ptr[7] = 0x2 5; /* video format */
+ ptr[8] = (frame-s3d_struct 0xf) 4;
+ }
+
+ hdmi_infoframe_checksum(buffer, length);
+
+ return length;
+}
+EXPORT_SYMBOL(hdmi_hdmi_infoframe_pack);
+
+/**
* hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary
*buffer
* @frame: HDMI vendor infoframe
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index b98340b..f5098a8 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -234,11 +234,36 @@ struct hdmi_vendor_infoframe {
ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
void *buffer, size_t size);
+enum hdmi_3d_structure {
+ HDMI_3D_STRUCTURE_INVALID = -1,
+ HDMI_3D_STRUCTURE_FRAME_PACKING = 0,
+ HDMI_3D_STRUCTURE_FIELD_ALTERNATIVE,
+ HDMI_3D_STRUCTURE_LINE_ALTERNATIVE,
+ HDMI_3D_STRUCTURE_SIDE_BY_SIDE_FULL,
+ HDMI_3D_STRUCTURE_L_DEPTH,
+ HDMI_3D_STRUCTURE_L_DEPTH_GFX_GFX_DEPTH,
+ HDMI_3D_STRUCTURE_TOP_BOTTOM,
TOP_AND_BOTTOM maybe?
+ HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF,
The value of this should be 8. Also according to the spec
we must send 3d_ext_data when when 3d_structure = 8, so
either we need a bit more code, or we need to disallow
3d_structure = 8 for the time being.
+};
+
+struct hdmi_hdmi_infoframe {
+ enum hdmi_infoframe_type type;
+ unsigned char version;
+ unsigned char length;
+ u8 vic;
+ enum hdmi_3d_structure s3d_struct;
+};
+
+int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame);
+ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
+ void *buffer, size_t size);
+
union hdmi_infoframe {
struct hdmi_any_infoframe any;
struct hdmi_avi_infoframe avi;
[Intel-gfx] [PATCH] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl
By exporting the ability to map user address and inserting PTEs
representing their backing pages into the GTT, we can exploit UMA in order
to utilize normal application data as a texture source or even as a
render target (depending upon the capabilities of the chipset). This has
a number of uses, with zero-copy downloads to the GPU and efficient
readback making the intermixed streaming of CPU and GPU operations
fairly efficient. This ability has many widespread implications from
faster rendering of client-side software rasterisers (chromium),
mitigation of stalls due to read back (firefox) and to faster pipelining
of texture data (such as pixel buffer objects in GL or data blobs in CL).
v2: Compile with CONFIG_MMU_NOTIFIER
v3: We can sleep while performing invalidate-range, which we can utilise
to drop our page references prior to the kernel manipulating the vma
(for either discard or cloning) and so protect normal users.
v4: Only run the invalidate notifier if the range intercepts the bo.
v5: Prevent userspace from attempting to GTT mmap non-page aligned buffers
v6: Recheck after reacquire mutex for lost mmu.
v7: Fix implicit padding of ioctl struct by rounding to next 64bit boundary.
v8: Fix rebasing error after forwarding porting the back port.
v9: Limit the userptr to page aligned entries. We now expect userspace
to handle all the offset-in-page adjustments itself.
v10: Prevent vma from being copied across fork to avoid issues with cow.
NB: We still have performance concerns over the use of the linear lists
and unfiltered notifies in mmu_notifier which do not scale to our use
case, where we may have many thousands of objects being tracked.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/Makefile |1 +
drivers/gpu/drm/i915/i915_dma.c |1 +
drivers/gpu/drm/i915/i915_drv.h | 18 +-
drivers/gpu/drm/i915/i915_gem.c |3 +
drivers/gpu/drm/i915/i915_gem_userptr.c | 416 +++
drivers/gpu/drm/i915/i915_gpu_error.c |2 +
include/uapi/drm/i915_drm.h | 16 ++
7 files changed, 456 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/i915/i915_gem_userptr.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 9d498e5..2369cfe 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -16,6 +16,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \
i915_gem_gtt.o \
i915_gem_stolen.o \
i915_gem_tiling.o \
+ i915_gem_userptr.o \
i915_sysfs.o \
i915_trace_points.o \
i915_ums.o \
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 3cab741..23a9374 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1894,6 +1894,7 @@ const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY,
i915_gem_context_destroy_ioctl, DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_GEM_CREATE2, i915_gem_create2_ioctl,
DRM_AUTH|DRM_UNLOCKED),
+ DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl,
DRM_UNLOCKED),
};
int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c2da7e9..d6c6626 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -43,6 +43,7 @@
#include linux/intel-iommu.h
#include linux/kref.h
#include linux/pm_qos.h
+#include linux/mmu_notifier.h
/* General customization:
*/
@@ -320,6 +321,7 @@ struct drm_i915_error_state {
u32 tiling:2;
u32 dirty:1;
u32 purgeable:1;
+ u32 userptr:1;
s32 ring:4;
u32 cache_level:2;
} **active_bo, **pinned_bo;
@@ -1296,6 +1298,7 @@ struct drm_i915_gem_object_ops {
*/
int (*get_pages)(struct drm_i915_gem_object *);
void (*put_pages)(struct drm_i915_gem_object *);
+ void (*release)(struct drm_i915_gem_object *);
};
struct drm_i915_gem_object {
@@ -1425,9 +1428,20 @@ struct drm_i915_gem_object {
/** for phy allocated objects */
struct drm_i915_gem_phys_object *phys_obj;
+
+ union {
+ struct i915_gem_userptr {
+ uintptr_t ptr;
+ bool read_only;
+
+ struct mm_struct *mm;
+#if defined(CONFIG_MMU_NOTIFIER)
+ struct mmu_notifier mn;
+#endif
+ } userptr;
+ };
};
#define to_gem_object(obj) (((struct drm_i915_gem_object *)(obj))-base)
-
#define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
/**
@@ -1733,6 +1747,8 @@ int i915_gem_set_tiling_ioctl(struct drm_device *dev,
void *data,
struct drm_file *file_priv);
int
Re: [Intel-gfx] [PATCH] Correct misspelled caching
On Wed, Aug 14, 2013 at 10:13 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Aug 14, 2013 at 10:09:23AM +0200, Sedat Dilek wrote: On Wed, Aug 14, 2013 at 10:05 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Aug 14, 2013 at 10:01:13AM +0200, Sedat Dilek wrote: Signed-off-by: Sedat Dilek sedat.di...@gmail.com Am I the only one who reads that as a hard 'ch' without the 'e'? I am not an English native or teacher: But from what I know it's... to cache | caching | cached. Google gives me no hits for cach*e*ing. Anyway, the spelling should be consistent in kernel-drm | libdrm | intel-ddx | etc. It was. I blame Ben. I can't say :-). Anyway, what's next with this patch? - Sedat - ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/12] video/hdmi: Hook the HDMI vendor infoframe with the generic _pack()
On Wed, Aug 14, 2013 at 12:17:26AM +0100, Damien Lespiau wrote:
With this last bit, hdmi_infoframe_pack() is now able to pack any
infoframe we support.
At the same time, because it's impractical to make two commits out of
this, we get rid of the version that encourages the open coding of the
vendor infoframe packing. We can do so because the only user of this API
has been ported in:
Author: Damien Lespiau damien.lesp...@intel.com
Date: Mon Aug 12 18:08:37 2013 +0100
gpu: host1x: Port the HDMI vendor infoframe code the common helpers
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/video/hdmi.c | 45 +
include/linux/hdmi.h | 24
2 files changed, 21 insertions(+), 48 deletions(-)
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 2059f7b..073f005 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -300,6 +300,7 @@ int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe
*frame)
frame-length = 5; /* we can hardcode the size for now as we don't
support neither 3D_Ext_Data nor 3D_Metadata_* */
+ frame-oui = HDMI_IDENTIFIER;
/* 0 is a valid value for s3d_struct, so we use a special not set
* value */
frame-s3d_struct = HDMI_3D_STRUCTURE_INVALID;
@@ -367,46 +368,18 @@ ssize_t hdmi_hdmi_infoframe_pack(struct
hdmi_hdmi_infoframe *frame,
}
EXPORT_SYMBOL(hdmi_hdmi_infoframe_pack);
-/**
- * hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary
- *buffer
- * @frame: HDMI vendor infoframe
- * @buffer: destination buffer
- * @size: size of buffer
- *
- * Packs the information contained in the @frame structure into a binary
- * representation that can be written into the corresponding controller
- * registers. Also computes the checksum as required by section 5.3.5 of
- * the HDMI 1.4 specification.
- *
- * Returns the number of bytes packed into the binary buffer or a negative
- * error code on failure.
+/*
+ * hdmi_vendor_infoframe_pack() - write a vendor infoframe to binary buffer
*/
-ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
-void *buffer, size_t size)
+static ssize_t hdmi_vendor_infoframe_pack(union hdmi_vendor_infoframe *frame,
+ void *buffer, size_t size)
{
- u8 *ptr = buffer;
- size_t length;
-
- length = HDMI_INFOFRAME_HEADER_SIZE + frame-length;
-
- if (size length)
- return -ENOSPC;
-
- memset(buffer, 0, size);
-
- ptr[0] = frame-type;
- ptr[1] = frame-version;
- ptr[2] = frame-length;
- ptr[3] = 0; /* checksum */
-
- memcpy(ptr[HDMI_INFOFRAME_HEADER_SIZE], frame-data, frame-length);
-
- hdmi_infoframe_checksum(buffer, length);
+ /* we only know about HDMI vendor infoframes */
+ if (frame-any.oui != HDMI_IDENTIFIER)
+ return -EINVAL;
- return length;
+ return hdmi_hdmi_infoframe_pack(frame-hdmi, buffer, size);
}
-EXPORT_SYMBOL(hdmi_vendor_infoframe_pack);
/**
* hdmi_infoframe_pack() - write a HDMI infoframe to binary buffer
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 53bbf0d..5c572e9 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -225,16 +225,6 @@ int hdmi_audio_infoframe_init(struct
hdmi_audio_infoframe *frame);
ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
void *buffer, size_t size);
-struct hdmi_vendor_infoframe {
- enum hdmi_infoframe_type type;
- unsigned char version;
- unsigned char length;
- u8 data[27];
-};
-
-ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
-void *buffer, size_t size);
-
enum hdmi_3d_structure {
HDMI_3D_STRUCTURE_INVALID = -1,
HDMI_3D_STRUCTURE_FRAME_PACKING = 0,
@@ -251,6 +241,7 @@ struct hdmi_hdmi_infoframe {
enum hdmi_infoframe_type type;
unsigned char version;
unsigned char length;
+ int oui;
unsigned perhaps? Same deal below in the 'any' struct.
Doesn't really matter I guess, so w/ or w/o that change:
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
u8 vic;
enum hdmi_3d_structure s3d_struct;
};
@@ -259,12 +250,21 @@ int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe
*frame);
ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
void *buffer, size_t size);
+union hdmi_vendor_infoframe {
+ struct {
+ enum hdmi_infoframe_type type;
+ unsigned char version;
+ unsigned char length;
+ int oui;
+ } any;
+ struct hdmi_hdmi_infoframe hdmi;
+};
+
union hdmi_infoframe {
struct
Re: [Intel-gfx] [PATCH 08/12] gpu: host1x: Port the HDMI vendor infoframe code the common helpers
On Wed, Aug 14, 2013 at 12:17:24AM +0100, Damien Lespiau wrote:
I just wrote the bits to define and pack HDMI vendor specific infoframe.
Port the host1x driver to use those so I can refactor the infoframe code
a bit more.
Cc: Thierry Reding thierry.red...@gmail.com
Cc: Terje Bergström tbergst...@nvidia.com
Cc: linux-te...@vger.kernel.org
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/host1x/drm/hdmi.c | 24
1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/host1x/drm/hdmi.c b/drivers/gpu/host1x/drm/hdmi.c
index 01097da..b548918 100644
--- a/drivers/gpu/host1x/drm/hdmi.c
+++ b/drivers/gpu/host1x/drm/hdmi.c
@@ -539,7 +539,7 @@ static void tegra_hdmi_setup_audio_infoframe(struct
tegra_hdmi *hdmi)
static void tegra_hdmi_setup_stereo_infoframe(struct tegra_hdmi *hdmi)
{
- struct hdmi_vendor_infoframe frame;
+ struct hdmi_hdmi_infoframe frame;
unsigned long value;
u8 buffer[10];
ssize_t err;
@@ -551,26 +551,10 @@ static void tegra_hdmi_setup_stereo_infoframe(struct
tegra_hdmi *hdmi)
return;
}
- memset(frame, 0, sizeof(frame));
+ hdmi_hdmi_infoframe_init(frame);
+ frame.s3d_struct = HDMI_3D_STRUCTURE_FRAME_PACKING;
- frame.type = HDMI_INFOFRAME_TYPE_VENDOR;
- frame.version = 0x01;
- frame.length = 6;
This changes the length of the infoframe from 6 to 5, which is enough
for frame packing, but maybe the commit msg should still mention that
small detail.
-
- frame.data[0] = 0x03; /* regid0 */
- frame.data[1] = 0x0c; /* regid1 */
- frame.data[2] = 0x00; /* regid2 */
- frame.data[3] = 0x02 5; /* video format */
-
- /* TODO: 74 MHz limit? */
- if (1) {
- frame.data[4] = 0x00 4; /* 3D structure */
- } else {
- frame.data[4] = 0x08 4; /* 3D structure */
- frame.data[5] = 0x00 4; /* 3D ext. data */
- }
-
- err = hdmi_vendor_infoframe_pack(frame, buffer, sizeof(buffer));
+ err = hdmi_hdmi_infoframe_pack(frame, buffer, sizeof(buffer));
if (err 0) {
dev_err(hdmi-dev, failed to pack vendor infoframe: %zd\n,
err);
--
1.8.3.1
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
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 07/12] video/hdmi: Introduce helpers for the HDMI vendor specific infoframe
On Wed, Aug 14, 2013 at 12:17:23AM +0100, Damien Lespiau wrote:
Provide the programming model than the other infoframe types.
The generic _pack() function can't handle those yet as we need to move
the vendor OUI in the generic hdmi_vendor_infoframe structure to know
which kind of vendor infoframe we are dealing with.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/video/hdmi.c | 82
include/linux/hdmi.h | 25
2 files changed, 107 insertions(+)
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index ac84215..2059f7b 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -286,6 +286,88 @@ ssize_t hdmi_audio_infoframe_pack(struct
hdmi_audio_infoframe *frame,
EXPORT_SYMBOL(hdmi_audio_infoframe_pack);
/**
+ * hdmi_hdmi_infoframe_init() - initialize an HDMI vendor infoframe
+ * @frame: HDMI vendor infoframe
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame)
+{
+ memset(frame, 0, sizeof(*frame));
+
+ frame-type = HDMI_INFOFRAME_TYPE_VENDOR;
+ frame-version = 1;
+ frame-length = 5; /* we can hardcode the size for now as we don't
+ support neither 3D_Ext_Data nor 3D_Metadata_* */
+
+ /* 0 is a valid value for s3d_struct, so we use a special not set
+ * value */
+ frame-s3d_struct = HDMI_3D_STRUCTURE_INVALID;
+
+ return 0;
+}
+EXPORT_SYMBOL(hdmi_hdmi_infoframe_init);
+
+/**
+ * hdmi_hmdi_infoframe_pack() - write a HDMI vendor infoframe to binary
buffer
Another hmdi typo that wasn't spotted yet :)
+ * @frame: HDMI infoframe
+ * @buffer: destination buffer
+ * @size: size of buffer
+ *
+ * Packs the information contained in the @frame structure into a binary
+ * representation that can be written into the corresponding controller
+ * registers. Also computes the checksum as required by section 5.3.5 of
+ * the HDMI 1.4 specification.
+ *
+ * Returns the number of bytes packed into the binary buffer or a negative
+ * error code on failure.
+ */
+ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
+ void *buffer, size_t size)
+{
+ u8 *ptr = buffer;
+ size_t length;
+
+ /* empty info frame */
+ if (frame-vic == 0 frame-s3d_struct == HDMI_3D_STRUCTURE_INVALID)
+ return -EINVAL;
+
+ /* only one of those can be supplied */
+ if (frame-vic != 0 frame-s3d_struct != HDMI_3D_STRUCTURE_INVALID)
+ return -EINVAL;
+
+ length = HDMI_INFOFRAME_HEADER_SIZE + frame-length;
+
+ if (size length)
+ return -ENOSPC;
+
+ memset(buffer, 0, size);
+
+ ptr[0] = frame-type;
+ ptr[1] = frame-version;
+ ptr[2] = frame-length;
+ ptr[3] = 0; /* checksum */
+
+ /* HDMI OUI */
+ ptr[4] = 0x03;
+ ptr[5] = 0x0c;
+ ptr[6] = 0x00;
+
+ if (frame-vic) {
+ ptr[7] = 0x1 5; /* video format */
+ ptr[8] = frame-vic;
+ } else {
+ ptr[7] = 0x2 5; /* video format */
+ ptr[8] = (frame-s3d_struct 0xf) 4;
+ }
+
+ hdmi_infoframe_checksum(buffer, length);
+
+ return length;
+}
+EXPORT_SYMBOL(hdmi_hdmi_infoframe_pack);
+
+/**
* hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary
*buffer
* @frame: HDMI vendor infoframe
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index b98340b..f5098a8 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -234,11 +234,36 @@ struct hdmi_vendor_infoframe {
ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
void *buffer, size_t size);
+enum hdmi_3d_structure {
+ HDMI_3D_STRUCTURE_INVALID = -1,
+ HDMI_3D_STRUCTURE_FRAME_PACKING = 0,
+ HDMI_3D_STRUCTURE_FIELD_ALTERNATIVE,
+ HDMI_3D_STRUCTURE_LINE_ALTERNATIVE,
+ HDMI_3D_STRUCTURE_SIDE_BY_SIDE_FULL,
+ HDMI_3D_STRUCTURE_L_DEPTH,
+ HDMI_3D_STRUCTURE_L_DEPTH_GFX_GFX_DEPTH,
+ HDMI_3D_STRUCTURE_TOP_BOTTOM,
+ HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF,
+};
+
+struct hdmi_hdmi_infoframe {
+ enum hdmi_infoframe_type type;
+ unsigned char version;
+ unsigned char length;
+ u8 vic;
+ enum hdmi_3d_structure s3d_struct;
+};
+
+int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame);
+ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
+ void *buffer, size_t size);
+
union hdmi_infoframe {
struct hdmi_any_infoframe any;
struct hdmi_avi_infoframe avi;
struct hdmi_spd_infoframe spd;
struct hdmi_vendor_infoframe vendor;
+ struct hdmi_hdmi_infoframe hdmi;
struct hdmi_audio_infoframe audio;
};
Re: [Intel-gfx] HDMI 4k support v2
On Wed, Aug 14, 2013 at 12:17:16AM +0100, Damien Lespiau wrote:
Following up the first instance of this series:
http://lists.freedesktop.org/archives/dri-devel/2013-August/043125.html
Here is a v2 with Ville's review pass and a few additions:
- Alternate clock modes for 4k resolutions
- HDMI vendor specific infoframe support in drivers/video/hdmi.c
- Enabling of those vendor specific infoframes in the i915 driver
The patches I didn't explicitly comment on (01,05,09,12 by my reckoning) are:
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Along the way, a tegra patch was needed for a small consolidation of the code
packing vendor specific infoframes. This patch has only been compile-tested.
On Intel, it's possible to read back the programmed infoframe buffers with
intel-gpu-tools' intel_infoframe and this gives:
Vendor InfoFrame:
- frequency: every vsync
- raw:
f4050181 000c0347 0320
004c
- vendor Id: 0x000c03 (HDMI)
- video format: 0x001
- HDMI VIC: 3
after a $ xrandr --output HDMI1 --mode 3840x2160 -r 24
--
Damien
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
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: Dump the contents of the GTT
On Wed, Aug 14, 2013 at 1:35 PM, Chris Wilson ch...@chris-wilson.co.uk wrote:
Chasing wild speculation that we may be writing the wrong addresses
into the GTT for stolen objects, I would like to inspect those values.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Sedat Dilek sedat.di...@gmail.com
---
Sedak, can you please apply this patch and then attach the contents of
/sys/kernel/debug/dri/0/i915_gem_gtt with the broken display?
Compiling... next-20130814 with drm-intel-nightly (up to commit
d93f59e86ae93066969fa8ae2a6c9ccc7fc4728d) plus this patch.
How do you want your Doner-Kebap? (Going into the city...)
- Sedat -
---
drivers/gpu/drm/i915/i915_debugfs.c | 47
-
1 file changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 10d864c..4edf65a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -390,6 +390,51 @@ static int i915_gem_object_info(struct seq_file *m,
void* data)
return 0;
}
+static int i915_gem_gtt_contents(struct seq_file *m, struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ gen6_gtt_pte_t __iomem *gtt_entries;
+ gen6_gtt_pte_t scratch_pte;
+ gen6_gtt_pte_t zero[8] = {};
+ int i, j, last_zero = 0;
+ int ret;
+
+ if (INTEL_INFO(dev)-gen 6)
+ return 0;
+
+ ret = mutex_lock_interruptible(dev-struct_mutex);
+ if (ret)
+ return ret;
+
+ gtt_entries = (gen6_gtt_pte_t __iomem *)dev_priv-gtt.gsm;
+ scratch_pte =
dev_priv-gtt.base.pte_encode(dev_priv-gtt.base.scratch.addr,
I915_CACHE_LLC);
+ for (i = 0; i gtt_total_entries(dev_priv-gtt); i += 8) {
+ gen6_gtt_pte_t pte[8];
+ int this_zero;
+
+ for (j = 0; j 8; j++) {
+ pte[j] = ioread32(gtt_entries[i+j]);
+ if (pte[j] == scratch_pte)
+ pte[j] = 0;
+ }
+
+ this_zero = memcmp(pte, zero, sizeof(pte)) == 0;
+ if (last_zero this_zero) {
+ if (last_zero++ == 1)
+ seq_puts(m, ...\n);
+ continue;
+ }
+
+ seq_printf(m, [%08x] %08x %08x %08x %08x %08x %08x %08x
%08x\n,
+ i, pte[0], pte[1], pte[2], pte[3], pte[4], pte[5],
pte[6], pte[7]);
+ last_zero = this_zero;
+ }
+
+ mutex_unlock(dev-struct_mutex);
+
+ return 0;
+}
+
static int i915_gem_gtt_info(struct seq_file *m, void *data)
{
struct drm_info_node *node = (struct drm_info_node *) m-private;
@@ -422,7 +467,7 @@ static int i915_gem_gtt_info(struct seq_file *m, void
*data)
seq_printf(m, Total %d objects, %zu bytes, %zu GTT size\n,
count, total_obj_size, total_gtt_size);
- return 0;
+ return i915_gem_gtt_contents(m, dev);
}
static int i915_gem_pageflip_info(struct seq_file *m, void *data)
--
1.8.4.rc2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: inline vma_create into lookup_or_create_vma
On Wed, Aug 14, 2013 at 11:59:09AM +0200, Daniel Vetter wrote: In the execbuf code we don't clean up any vmas which ended up not getting bound for code simplicity. To make sure that we don't end up creating multiple vma for the same vm kill the somewhat dangerous vma_create function and inline it into lookup_or_create. Just make it static. It's a nice little standalone function that helps the reader. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: inline vma_create into lookup_or_create_vma
On Wed, Aug 14, 2013 at 12:49:04PM +0100, Chris Wilson wrote: On Wed, Aug 14, 2013 at 11:59:09AM +0200, Daniel Vetter wrote: In the execbuf code we don't clean up any vmas which ended up not getting bound for code simplicity. To make sure that we don't end up creating multiple vma for the same vm kill the somewhat dangerous vma_create function and inline it into lookup_or_create. Just make it static. It's a nice little standalone function that helps the reader. I'm afraid that someone will use it eventually. I'll smash a __ prefix onto it to make it clear that people better don't touch it. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: inline vma_create into lookup_or_create_vma
In the execbuf code we don't clean up any vmas which ended up not
getting bound for code simplicity. To make sure that we don't end up
creating multiple vma for the same vm kill the somewhat dangerous
vma_create function and inline it into lookup_or_create.
This is just a safety measure to prevent surprises in the future.
Also update the somewhat confused comment in the execbuf code and
clarify what kind of magic is going on with a new one.
v2: Keep the function separate as requested by Chris. But give it a __
prefix for paranoia and move it tighter together with the other vma
stuff.
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_drv.h| 2 --
drivers/gpu/drm/i915/i915_gem.c| 50 +++---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 11 +--
drivers/gpu/drm/i915/i915_gem_stolen.c | 2 +-
4 files changed, 34 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fc35ae3..b06a90f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1748,8 +1748,6 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
size_t size);
void i915_gem_free_object(struct drm_gem_object *obj);
-struct i915_vma *i915_gem_vma_create(struct drm_i915_gem_object *obj,
-struct i915_address_space *vm);
void i915_gem_vma_destroy(struct i915_vma *vma);
int __must_check i915_gem_object_pin(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 4be3be9..0832f61 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4121,9 +4121,20 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
i915_gem_object_free(obj);
}
-struct i915_vma *i915_gem_vma_create(struct drm_i915_gem_object *obj,
+struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
struct i915_address_space *vm)
{
+ struct i915_vma *vma;
+ list_for_each_entry(vma, obj-vma_list, vma_link)
+ if (vma-vm == vm)
+ return vma;
+
+ return NULL;
+}
+
+static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj,
+ struct i915_address_space *vm)
+{
struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL);
if (vma == NULL)
return ERR_PTR(-ENOMEM);
@@ -4143,6 +4154,19 @@ struct i915_vma *i915_gem_vma_create(struct
drm_i915_gem_object *obj,
return vma;
}
+struct i915_vma *
+i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj,
+ struct i915_address_space *vm)
+{
+ struct i915_vma *vma;
+
+ vma = i915_gem_obj_to_vma(obj, vm);
+ if (!vma)
+ vma = __i915_gem_vma_create(obj, vm);
+
+ return vma;
+}
+
void i915_gem_vma_destroy(struct i915_vma *vma)
{
WARN_ON(vma-node.allocated);
@@ -4869,27 +4893,3 @@ unsigned long i915_gem_obj_size(struct
drm_i915_gem_object *o,
return 0;
}
-
-struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
-struct i915_address_space *vm)
-{
- struct i915_vma *vma;
- list_for_each_entry(vma, obj-vma_list, vma_link)
- if (vma-vm == vm)
- return vma;
-
- return NULL;
-}
-
-struct i915_vma *
-i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj,
- struct i915_address_space *vm)
-{
- struct i915_vma *vma;
-
- vma = i915_gem_obj_to_vma(obj, vm);
- if (!vma)
- vma = i915_gem_vma_create(obj, vm);
-
- return vma;
-}
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index c3b36f5..9b3b5f8 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -123,11 +123,16 @@ eb_lookup_vmas(struct eb_vmas *eb,
list_for_each_entry(obj, objects, obj_exec_link) {
struct i915_vma *vma;
+ /*
+* NOTE: We can leak any vmas created here when something fails
+* later on. But that's no issue since vma_unbind can deal with
+* vmas which are not actually bound. And since only
+* lookup_or_create exists as an interface to get at the vma
+* from the (obj, vm) we don't run the risk of creating
+* duplicated vmas for the same vm.
+*/
vma = i915_gem_obj_lookup_or_create_vma(obj, vm);
if (IS_ERR(vma))
Re: [Intel-gfx] [PATCH] drm/i915: Dump the contents of the GTT
On Wed, Aug 14, 2013 at 2:52 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Aug 14, 2013 at 02:31:24PM +0200, Sedat Dilek wrote: Requested output and dmesg files attached. ( Should I attach the one with BROKEN display - I mean w/o doing a re-login and display REPAIRED? ) Yes please. Attached tarball contains: 130413 Aug 14 14:52 dmesg_3.11.0-rc5-next20130814-1-iniza-small_drm-debug-6_0-before-relogin-lightdm-unity2d_VT-1.txt 145973 Aug 14 14:53 dmesg_3.11.0-rc5-next20130814-1-iniza-small_drm-debug-6_1-before-relogin-lightdm-unity2d_X.txt 199199 Aug 14 14:55 dmesg_3.11.0-rc5-next20130814-1-iniza-small_drm-debug-6_2_after-relogin-lightdm-unity2d_X.txt 79398 Aug 14 14:53 i915_gem_gtt_drm-debug-6_1-before-relogin-lightdm-unity2d_X.txt 108374 Aug 14 14:55 i915_gem_gtt_drm-debug-6_2_after-relogin-lightdm-unity2d_X.txt - Sedat - ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Dump the contents of the GTT
On Wed, Aug 14, 2013 at 3:01 PM, Sedat Dilek sedat.di...@gmail.com wrote: On Wed, Aug 14, 2013 at 2:52 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Aug 14, 2013 at 02:31:24PM +0200, Sedat Dilek wrote: Requested output and dmesg files attached. ( Should I attach the one with BROKEN display - I mean w/o doing a re-login and display REPAIRED? ) Yes please. Attached tarball contains: 130413 Aug 14 14:52 dmesg_3.11.0-rc5-next20130814-1-iniza-small_drm-debug-6_0-before-relogin-lightdm-unity2d_VT-1.txt 145973 Aug 14 14:53 dmesg_3.11.0-rc5-next20130814-1-iniza-small_drm-debug-6_1-before-relogin-lightdm-unity2d_X.txt 199199 Aug 14 14:55 dmesg_3.11.0-rc5-next20130814-1-iniza-small_drm-debug-6_2_after-relogin-lightdm-unity2d_X.txt 79398 Aug 14 14:53 i915_gem_gtt_drm-debug-6_1-before-relogin-lightdm-unity2d_X.txt 108374 Aug 14 14:55 i915_gem_gtt_drm-debug-6_2_after-relogin-lightdm-unity2d_X.txt The real attachment. - Sedat - for-ickle.tar.xz.sha256sum Description: Binary data for-ickle.tar.xz Description: Binary data ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] VGA arbiter support for Intel HD?
Hi, I'm trying to add support for device assignment of PCI VGA devices with VFIO and QEMU. For normal, discrete discrete graphics the Linux VGA arbiter works fairly well, disabling VGA on one bridge and adding it to another (though I wish all the kernel VGA drivers made use of it). The i915 driver only seems to support disabling VGA on really old GMCH devices (see intel_modeset_vga_set_state). This means that if I boot with IGD as the primary graphics and attempt to assign a discrete graphics device, all the VGA range accesses are still routed to IGD, I end up getting some error messages from the IGD interrupt handler, and the discrete card never initializes. I spent some time looking through the Sand Bridge, Ivy Bridge, and Haswell datasheets, and I'm a bit concerned whether the hardware even provides a reasonable way to disable VGA anymore. Quoting 2.17 from the Haswell docs: Accesses to the VGA memory range are directed to IGD depend on the configuration. The configuration is specified by: * Internal graphics controller in Device 2 is enabled (DEVEN.D2EN bit 4) * Internal graphics VGA in Device 0 Function 0 is enabled through register GGC bit 1. * IGD's memory accesses (PCICMD2 04 – 05h, MAE bit 1) in Device 2 configuration space are enabled. * VGA compatibility memory accesses (VGA Miscellaneous Output register – MSR Register, bit 1) are enabled. * Software sets the proper value for VGA Memory Map Mode register (VGA GR06 Register, bits 3-2). See the following table for translations. (There's a similar list for VGA I/O range) I've found that if I disable memory and I/O in the PCI command register for IGD then I do get VGA routing to the PEG device and the discrete VBIOS works. This obviously isn't a good option for the VGA arbiter since it entirely disables IGD. The GGC registers aren't meant for runtime switching and are actually locked. Disabling IGD via the device 2 enable bit doesn't seem like and option. I don't quite understand the VGA miscellaneous output register and VGA memory map mode, but the table provided for the latter makes me think they just augment the VGA ranges and don't disable them. Is it possible to support the VGA arbiter with the latest IGD? Is anyone working on it? Thanks, Alex ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Dump the contents of the GTT
On Wed, Aug 14, 2013 at 3:02 PM, Sedat Dilek sedat.di...@gmail.com wrote: On Wed, Aug 14, 2013 at 3:01 PM, Sedat Dilek sedat.di...@gmail.com wrote: On Wed, Aug 14, 2013 at 2:52 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Aug 14, 2013 at 02:31:24PM +0200, Sedat Dilek wrote: Requested output and dmesg files attached. ( Should I attach the one with BROKEN display - I mean w/o doing a re-login and display REPAIRED? ) Yes please. Attached tarball contains: 130413 Aug 14 14:52 dmesg_3.11.0-rc5-next20130814-1-iniza-small_drm-debug-6_0-before-relogin-lightdm-unity2d_VT-1.txt 145973 Aug 14 14:53 dmesg_3.11.0-rc5-next20130814-1-iniza-small_drm-debug-6_1-before-relogin-lightdm-unity2d_X.txt 199199 Aug 14 14:55 dmesg_3.11.0-rc5-next20130814-1-iniza-small_drm-debug-6_2_after-relogin-lightdm-unity2d_X.txt 79398 Aug 14 14:53 i915_gem_gtt_drm-debug-6_1-before-relogin-lightdm-unity2d_X.txt 108374 Aug 14 14:55 i915_gem_gtt_drm-debug-6_2_after-relogin-lightdm-unity2d_X.txt The real attachment. A new tarball with... 1. booting into text-mode (no graphical mode, no X) 2. start lightdm service (upstart) 3. start unity-2d session and logout (re-enter lightdm greeter screen) - Sedat - for-ickle-2.tar.xz Description: Binary data for-ickle-2.tar.xz.sha256sum Description: Binary data ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Dump the contents of the GTT
On Wed, Aug 14, 2013 at 03:02:37PM +0200, Sedat Dilek wrote: On Wed, Aug 14, 2013 at 3:01 PM, Sedat Dilek sedat.di...@gmail.com wrote: On Wed, Aug 14, 2013 at 2:52 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Aug 14, 2013 at 02:31:24PM +0200, Sedat Dilek wrote: Requested output and dmesg files attached. ( Should I attach the one with BROKEN display - I mean w/o doing a re-login and display REPAIRED? ) Yes please. Attached tarball contains: 130413 Aug 14 14:52 dmesg_3.11.0-rc5-next20130814-1-iniza-small_drm-debug-6_0-before-relogin-lightdm-unity2d_VT-1.txt 145973 Aug 14 14:53 dmesg_3.11.0-rc5-next20130814-1-iniza-small_drm-debug-6_1-before-relogin-lightdm-unity2d_X.txt 199199 Aug 14 14:55 dmesg_3.11.0-rc5-next20130814-1-iniza-small_drm-debug-6_2_after-relogin-lightdm-unity2d_X.txt 79398 Aug 14 14:53 i915_gem_gtt_drm-debug-6_1-before-relogin-lightdm-unity2d_X.txt 108374 Aug 14 14:55 i915_gem_gtt_drm-debug-6_2_after-relogin-lightdm-unity2d_X.txt The PTE values do correspond with the stolen addresses for the framebuffer. So I am resonably confident that the driver is self-consistent at least. DSPSURF points to the right location in the GGTT and those entries point to the right location in stolen. The display on the other hand seems to be continuing to read from GGTT offset 0, and so the real framebuffer appears offset by a few lines. How about trying https://patchwork.kernel.org/patch/2841934/ ? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Dump the contents of the GTT
On Wed, Aug 14, 2013 at 3:44 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Aug 14, 2013 at 03:02:37PM +0200, Sedat Dilek wrote: On Wed, Aug 14, 2013 at 3:01 PM, Sedat Dilek sedat.di...@gmail.com wrote: On Wed, Aug 14, 2013 at 2:52 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Aug 14, 2013 at 02:31:24PM +0200, Sedat Dilek wrote: Requested output and dmesg files attached. ( Should I attach the one with BROKEN display - I mean w/o doing a re-login and display REPAIRED? ) Yes please. Attached tarball contains: 130413 Aug 14 14:52 dmesg_3.11.0-rc5-next20130814-1-iniza-small_drm-debug-6_0-before-relogin-lightdm-unity2d_VT-1.txt 145973 Aug 14 14:53 dmesg_3.11.0-rc5-next20130814-1-iniza-small_drm-debug-6_1-before-relogin-lightdm-unity2d_X.txt 199199 Aug 14 14:55 dmesg_3.11.0-rc5-next20130814-1-iniza-small_drm-debug-6_2_after-relogin-lightdm-unity2d_X.txt 79398 Aug 14 14:53 i915_gem_gtt_drm-debug-6_1-before-relogin-lightdm-unity2d_X.txt 108374 Aug 14 14:55 i915_gem_gtt_drm-debug-6_2_after-relogin-lightdm-unity2d_X.txt The PTE values do correspond with the stolen addresses for the framebuffer. So I am resonably confident that the driver is self-consistent at least. DSPSURF points to the right location in the GGTT and those entries point to the right location in stolen. The display on the other hand seems to be continuing to read from GGTT offset 0, and so the real framebuffer appears offset by a few lines. How about trying https://patchwork.kernel.org/patch/2841934/ ? The same screen corruption (see also attached tarball). - Sedat - for-ickle-3.tar.xz Description: Binary data for-ickle-3.tar.xz.sha256sum Description: Binary data ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/14] drm/i915: add MIPI DSI register definitions
On Tue, Aug 13, 2013 at 04:29:43PM +0300, Jani Nikula wrote: Add definitions for VLV MIPI DSI registers. Signed-off-by: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 409 +++ 1 file changed, 409 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index aced53a..32e32b6 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5104,4 +5104,413 @@ #define PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME) #define PIPE_CSC_POSTOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO) +/* VLV MIPI registers */ + +/* XXX: This register seems very messy. MIPIB has only enable and delay. */ +#define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190) +#define _MIPIB_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700) +#define MIPI_PORT_CTRL(pipe) _PIPE(pipe, _MIPIA_PORT_CTRL, _MIPIB_PORT_CTRL) +#define DPI_ENABLE (1 31) /* A + B */ +#define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27 +#define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK(0xf 27) +#define DUAL_LINK_MODE_MASK (1 26) +#define DUAL_LINK_MODE_FRONT_BACK (0 26) +#define DUAL_LINK_MODE_PIXEL_ALTERNATIVE(1 26) 25 is Dither Enable according to my docs. +#define FLOPPED_HSTX(1 23) +#define DE_INVERT (1 19) /* XXX */ One doc has DE_INVERT (and HS/VS invert and border enable bits), another has the FLISDSI stuff. I guess that's the XXX here. +#define MIPIA_FLISDSI_DELAY_COUNT_SHIFT 18 +#define MIPIA_FLISDSI_DELAY_COUNT_MASK (0xf 18) +#define AFE_LATCHOUT(1 17) 16 is LPOutput Hold +#define MIPIB_FLISDSI_DELAY_COUNT_HIGH_SHIFT15 +#define MIPIB_FLISDSI_DELAY_COUNT_HIGH_MASK (1 15) +#define MIPIB_MIPI4DPHY_DELAY_COUNT_SHIFT 11 +#define MIPIB_MIPI4DPHY_DELAY_COUNT_MASK(0xf 11) +#define CSB_SHIFT 9 +#define CSB_MASK(3 9) +#define CSB_20MHZ (0 9) +#define CSB_10MHZ (1 9) +#define CSB_40MHZ (2 9) +#define BANDGAP_MASK(1 8) +#define BANDGAP_PNW_CIRCUIT (0 8) +#define BANDGAP_LNC_CIRCUIT (1 8) +#define MIPIB_FLISDSI_DELAY_COUNT_LOW_SHIFT 5 +#define MIPIB_FLISDSI_DELAY_COUNT_LOW_MASK (7 5) +#define TEARING_EFFECT_DELAY(1 4) /* A + B */ +#define TEARING_EFFECT_SHIFT2 /* A + B */ +#define TEARING_EFFECT_MASK (3 2) +#define TEARING_EFFECT_OFF (0 2) +#define TEARING_EFFECT_DSI (1 2) +#define TEARING_EFFECT_GPIO (2 2) +#define LANE_CONFIGURATION_SHIFT0 +#define LANE_CONFIGURATION_MASK (3 0) +#define LANE_CONFIGURATION_4LANE(0 0) +#define LANE_CONFIGURATION_DUAL_LINK_A (1 0) +#define LANE_CONFIGURATION_DUAL_LINK_B (2 0) + +#define _MIPIA_TEARING_CTRL (VLV_DISPLAY_BASE + 0x61194) +#define _MIPIB_TEARING_CTRL (VLV_DISPLAY_BASE + 0x61704) +#define MIPI_TEARING_CTRL(pipe) _PIPE(pipe, _MIPIA_TEARING_CTRL, _MIPIB_TEARING_CTRL) +#define TEARING_EFFECT_DELAY_SHIFT 0 +#define TEARING_EFFECT_DELAY_MASK (0x 0) + +/* XXX */ +#define _MIPIA_AUTOPWG (VLV_DISPLAY_BASE + 0x611a0) Is the XXX here because the spec says the contents are all reserved? + +/* MIPI DSI Controller and D-PHY registers */ + +#define _MIPIA_DEVICE_READY (VLV_DISPLAY_BASE + 0xb000) +#define _MIPIB_DEVICE_READY (VLV_DISPLAY_BASE + 0xb800) +#define MIPI_DEVICE_READY(pipe) _PIPE(pipe, _MIPIA_DEVICE_READY, _MIPIB_DEVICE_READY) +#define BUS_POSSESSION (1 3) /* set to give bus to receiver */ +#define ULPS_STATE_MASK (3 1) +#define ULPS_STATE_ENTER(2 1) /* XXX */ +#define ULPS_STATE_EXIT (1 1) /* XXX */ Maybe we should have the (0 1) normal operation listed here as well? +#define DEVICE_READY(1 0) + +#define _MIPIA_INTR_STAT (VLV_DISPLAY_BASE + 0xb004) +#define _MIPIB_INTR_STAT
Re: [Intel-gfx] [PATCH] drm/i915: VGA also requires the power well
On Mon, Aug 12, 2013 at 06:06:48PM +, Zanoni, Paulo R wrote:
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
Vetter
Sent: Sunday, August 04, 2013 4:50 PM
To: Paulo Zanoni
Cc: Ville Syrjälä; intel-gfx@lists.freedesktop.org; Zanoni, Paulo R
Subject: Re: [Intel-gfx] [PATCH] drm/i915: VGA also requires the power well
On Fri, Aug 02, 2013 at 02:17:53PM -0300, Paulo Zanoni wrote:
2013/6/6 Ville Syrjälä ville.syrj...@linux.intel.com:
On Wed, Jun 05, 2013 at 06:05:51PM -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
So add a power domain and check for it before we try to read
VGA_CONTROL.
This fixes unclaimed register messages that happen on
suspend/resume.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 3 +++
drivers/gpu/drm/i915/intel_pm.c | 1 +
3 files changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 46b1f70..d51ce13 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -89,6 +89,7 @@ enum port {
#define port_name(p) ((p) + 'A')
enum intel_display_power_domain {
+ POWER_DOMAIN_VGA,
POWER_DOMAIN_PIPE_A,
POWER_DOMAIN_PIPE_B,
POWER_DOMAIN_PIPE_C,
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 4c8fcec..3719d99 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9950,6 +9950,9 @@ void i915_redisable_vga(struct drm_device
*dev)
struct drm_i915_private *dev_priv = dev-dev_private;
u32 vga_reg = i915_vgacntrl_reg(dev);
+ if (!intel_display_power_enabled(dev, POWER_DOMAIN_VGA))
+ return;
+
So it looks like you're essentially making intel_redisable_vga() a nop
for HSW. Shouldn't we instead enable the power well during resume?
We enable the power well during resume, but it's only after this
function...
Hm, so better move the (temporary) power well enabling in the resume
code
up a bit to cover this?
I don't think so. If you look at i915_redisable_vga and commit
0fde901f1ddd2ce0e380a6444f1fb7ca555859e9, you will start thinking that
just closing/opening the lid could make the BIOS somehow enable the
power well and then reenable VGA, so moving the check to earlier in the
resume sequence won't solve any problems, as VGA could be reenabled
later.
Well that's only for machines without opregion afaik. But we lack the
check for that, which I didn't really realize. To avoid blocking this even
longer I've just merged this patch here with a big note added to the
commit message.
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] VGA arbiter support for Intel HD?
On Wed, Aug 14, 2013 at 07:23:57AM -0600, Alex Williamson wrote: Hi, I'm trying to add support for device assignment of PCI VGA devices with VFIO and QEMU. For normal, discrete discrete graphics the Linux VGA arbiter works fairly well, disabling VGA on one bridge and adding it to another (though I wish all the kernel VGA drivers made use of it). The i915 driver only seems to support disabling VGA on really old GMCH devices (see intel_modeset_vga_set_state). This means that if I boot with IGD as the primary graphics and attempt to assign a discrete graphics device, all the VGA range accesses are still routed to IGD, I end up getting some error messages from the IGD interrupt handler, and the discrete card never initializes. I spent some time looking through the Sand Bridge, Ivy Bridge, and Haswell datasheets, and I'm a bit concerned whether the hardware even provides a reasonable way to disable VGA anymore. Quoting 2.17 from the Haswell docs: Accesses to the VGA memory range are directed to IGD depend on the configuration. The configuration is specified by: * Internal graphics controller in Device 2 is enabled (DEVEN.D2EN bit 4) * Internal graphics VGA in Device 0 Function 0 is enabled through register GGC bit 1. * IGD's memory accesses (PCICMD2 04 – 05h, MAE bit 1) in Device 2 configuration space are enabled. * VGA compatibility memory accesses (VGA Miscellaneous Output register – MSR Register, bit 1) are enabled. * Software sets the proper value for VGA Memory Map Mode register (VGA GR06 Register, bits 3-2). See the following table for translations. (There's a similar list for VGA I/O range) I've found that if I disable memory and I/O in the PCI command register for IGD then I do get VGA routing to the PEG device and the discrete VBIOS works. This obviously isn't a good option for the VGA arbiter since it entirely disables IGD. The GGC registers aren't meant for runtime switching and are actually locked. Disabling IGD via the device 2 enable bit doesn't seem like and option. I don't quite understand the VGA miscellaneous output register and VGA memory map mode, but the table provided for the latter makes me think they just augment the VGA ranges and don't disable them. Bit 1 of MSR (0x3c2/0x3cc) should allow you to turn off VGA mem access while leaving other memory space access working. As for VGA I/O decode, IIRC there's no standard bit for that in VGA or PCI config registers, and I can't see any other bit for it in the docs. But I guess you could just turn off I/O space completely via the PCI_COMMAND register. We shouldn't need it for anything beyond i915_disable_vga() and that has the necessary vgaarb calls already. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: check the power well when redisabling VGA
On Mon, Aug 05, 2013 at 05:46:12PM +0200, Daniel Vetter wrote: On Fri, Aug 02, 2013 at 04:22:24PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com If the power well is disabled VGA is guaranteed to be disabled. This fixes unclaimed register messages that happen on suspend/resume. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67517 Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Have you seen my other comment asking whether we could just force-enable the power well a bit earlier in the resume sequence instead? Imo that feels a notch more robust, since we didn't sprinkle tons of power well checks in all the other hw readout paths either. Ok, I've merged this one here (plus comments and the follow-up patch) instead of the other one. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Improved IGT support for text fixtures
Hi all, So while wreaking havoc with igt tests and adding some nice infrastructure to handle subtest status reporting a bit better I've also added support to annotate the common test fixture code. And rolled it out over all tests. I plan to do a decent write up on all the new infrastructure and how to use it when writing testcases, but the immediate benefit is that enumerating testcases now doesn't need to be done on a system with an intel gpu any more. Furthermore (and that's the reason I've done this) we can also finally enumerate the prime_nv subtests without the need for an nvidia gpu. Yang guang: Can you please check that everything works correctly on the QA setup? Right now we should have 303 igt tests when enumerating them with piglit (i.e. subtests included). Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: inline vma_create into lookup_or_create_vma
On Wed, Aug 14, 2013 at 11:59:09AM +0200, Daniel Vetter wrote: In the execbuf code we don't clean up any vmas which ended up not getting bound for code simplicity. To make sure that we don't end up creating multiple vma for the same vm kill the somewhat dangerous vma_create function and inline it into lookup_or_create. This is just a safety measure to prevent surprises in the future. Also update the somewhat confused comment in the execbuf code and clarify what kind of magic is going on with a new one. Cc: Ben Widawsky b...@bwidawsk.net Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- That's the only concern I could come up with when reading the execbuf vma conversion patch. So looks good and I'll slurp it all in as soon as some more head scratching is done for the very first patch in this series about the vma_unbind fix to only call vma_destroy if the vma isn't bound. One thing I've noticed but forgot to mention here is that the reloc code still uses obj_ggtt_size/offset. I guess that will be fixed later on? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] VGA arbiter support for Intel HD?
On Wed, 2013-08-14 at 17:47 +0300, Ville Syrjälä wrote: On Wed, Aug 14, 2013 at 07:23:57AM -0600, Alex Williamson wrote: Hi, I'm trying to add support for device assignment of PCI VGA devices with VFIO and QEMU. For normal, discrete discrete graphics the Linux VGA arbiter works fairly well, disabling VGA on one bridge and adding it to another (though I wish all the kernel VGA drivers made use of it). The i915 driver only seems to support disabling VGA on really old GMCH devices (see intel_modeset_vga_set_state). This means that if I boot with IGD as the primary graphics and attempt to assign a discrete graphics device, all the VGA range accesses are still routed to IGD, I end up getting some error messages from the IGD interrupt handler, and the discrete card never initializes. I spent some time looking through the Sand Bridge, Ivy Bridge, and Haswell datasheets, and I'm a bit concerned whether the hardware even provides a reasonable way to disable VGA anymore. Quoting 2.17 from the Haswell docs: Accesses to the VGA memory range are directed to IGD depend on the configuration. The configuration is specified by: * Internal graphics controller in Device 2 is enabled (DEVEN.D2EN bit 4) * Internal graphics VGA in Device 0 Function 0 is enabled through register GGC bit 1. * IGD's memory accesses (PCICMD2 04 – 05h, MAE bit 1) in Device 2 configuration space are enabled. * VGA compatibility memory accesses (VGA Miscellaneous Output register – MSR Register, bit 1) are enabled. * Software sets the proper value for VGA Memory Map Mode register (VGA GR06 Register, bits 3-2). See the following table for translations. (There's a similar list for VGA I/O range) I've found that if I disable memory and I/O in the PCI command register for IGD then I do get VGA routing to the PEG device and the discrete VBIOS works. This obviously isn't a good option for the VGA arbiter since it entirely disables IGD. The GGC registers aren't meant for runtime switching and are actually locked. Disabling IGD via the device 2 enable bit doesn't seem like and option. I don't quite understand the VGA miscellaneous output register and VGA memory map mode, but the table provided for the latter makes me think they just augment the VGA ranges and don't disable them. Bit 1 of MSR (0x3c2/0x3cc) should allow you to turn off VGA mem access while leaving other memory space access working. As for VGA I/O decode, IIRC there's no standard bit for that in VGA or PCI config registers, and I can't see any other bit for it in the docs. But I guess you could just turn off I/O space completely via the PCI_COMMAND register. We shouldn't need it for anything beyond i915_disable_vga() and that has the necessary vgaarb calls already. Thanks Ville. The MSR seems to work for VGA memory. Disabling I/O via PCI_COMMAND does works, but something is re-enabling it after intel_modeset_vga_set_state(). If I manually disable I/O with setpci then I do have VGA routing to PEG and can still interact with the KMS console on IGD. It's unfortunate that the MSR bit for I/O only disables pieces of the range. If we have no other options, I'll try to hunt down where I/O is being re-enabled and see how feasible it is to avoid. Thanks, Alex ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] HDMI 4k support v3
Follow up on v2: http://lists.freedesktop.org/archives/dri-devel/2013-August/043604.html With the quick and nice reviews from Ville and Simon, it's time for a v3: - Fix embarrassing hmdi typo - Fix the sending of vendor specific infoframes for side-by-side half modes - Smaller changes here and there. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/12] drm: Don't export drm_find_cea_extension() any more
This function is only used inside drm_edid.c.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/drm_edid.c | 5 ++---
include/drm/drm_crtc.h | 1 -
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index dfc7a1b..e014785 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2298,10 +2298,10 @@ add_detailed_modes(struct drm_connector *connector,
struct edid *edid,
#define EDID_CEA_YCRCB422 (1 4)
#define EDID_CEA_VCDB_QS (1 6)
-/**
+/*
* Search EDID for CEA extension block.
*/
-u8 *drm_find_cea_extension(struct edid *edid)
+static u8 *drm_find_cea_extension(struct edid *edid)
{
u8 *edid_ext = NULL;
int i;
@@ -2322,7 +2322,6 @@ u8 *drm_find_cea_extension(struct edid *edid)
return edid_ext;
}
-EXPORT_SYMBOL(drm_find_cea_extension);
/*
* Calculate the alternate clock for the CEA mode
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index fa12a2f..f3ecc6f 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1050,7 +1050,6 @@ extern int drm_mode_gamma_get_ioctl(struct drm_device
*dev,
void *data, struct drm_file *file_priv);
extern int drm_mode_gamma_set_ioctl(struct drm_device *dev,
void *data, struct drm_file *file_priv);
-extern u8 *drm_find_cea_extension(struct edid *edid);
extern u8 drm_match_cea_mode(const struct drm_display_mode *to_match);
extern bool drm_detect_hdmi_monitor(struct edid *edid);
extern bool drm_detect_monitor_audio(struct edid *edid);
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/12] drm/edid: Fix add_cea_modes() style issues
A few styles issues have crept in here, fix them before touching this
code again.
v2: constify arguments that can be (Ville Syrjälä)
v3: constify, but better (Ville Syrjälä)
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/drm_edid.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index e014785..bb25ee2 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2441,10 +2441,11 @@ add_alternate_cea_modes(struct drm_connector
*connector, struct edid *edid)
}
static int
-do_cea_modes (struct drm_connector *connector, u8 *db, u8 len)
+do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len)
{
struct drm_device *dev = connector-dev;
- u8 * mode, cea_mode;
+ const u8 *mode;
+ u8 cea_mode;
int modes = 0;
for (mode = db; mode db + len; mode++) {
@@ -2501,8 +2502,9 @@ cea_db_offsets(const u8 *cea, int *start, int *end)
static int
add_cea_modes(struct drm_connector *connector, struct edid *edid)
{
- u8 * cea = drm_find_cea_extension(edid);
- u8 * db, dbl;
+ const u8 *cea = drm_find_cea_extension(edid);
+ const u8 *db;
+ u8 dbl;
int modes = 0;
if (cea cea_revision(cea) = 3) {
@@ -2516,7 +2518,7 @@ add_cea_modes(struct drm_connector *connector, struct
edid *edid)
dbl = cea_db_payload_len(db);
if (cea_db_tag(db) == VIDEO_BLOCK)
- modes += do_cea_modes (connector, db+1, dbl);
+ modes += do_cea_modes(connector, db + 1, dbl);
}
}
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/12] drm/edid: Parse the HDMI CEA block and look for 4k modes
HDMI 1.4 adds 4 4k x 2k modes in the the CEA vendor specific block.
With this commit, we now parse this block and expose the 4k modes that
we find there.
v2: Fix the 4096x2160 string (nice catch!), add comments about
do_hdmi_vsdb_modes() arguments and make it clearer that offset is
relative to the end of the required fields of the HDMI VSDB
(Ville Syrjälä)
v3: Fix 'Unknow' typo (Simon Farnsworth)
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Tested-by: Cancan Feng cancan.f...@intel.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67030
Reviewed-by: Simon Farnsworth simon.farnswo...@onelan.co.uk
---
drivers/gpu/drm/drm_edid.c | 124 +++--
1 file changed, 109 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index bb25ee2..9de573c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -931,6 +931,36 @@ static const struct drm_display_mode edid_cea_modes[] = {
.vrefresh = 100, },
};
+/*
+ * HDMI 1.4 4k modes.
+ */
+static const struct drm_display_mode edid_4k_modes[] = {
+ /* 1 - 3840x2160@30Hz */
+ { DRM_MODE(3840x2160, DRM_MODE_TYPE_DRIVER, 297000,
+ 3840, 4016, 4104, 4400, 0,
+ 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 30, },
+ /* 2 - 3840x2160@25Hz */
+ { DRM_MODE(3840x2160, DRM_MODE_TYPE_DRIVER, 297000,
+ 3840, 4896, 4984, 5280, 0,
+ 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 25, },
+ /* 3 - 3840x2160@24Hz */
+ { DRM_MODE(3840x2160, DRM_MODE_TYPE_DRIVER, 297000,
+ 3840, 5116, 5204, 5500, 0,
+ 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 24, },
+ /* 4 - 4096x2160@24Hz (SMPTE) */
+ { DRM_MODE(4096x2160, DRM_MODE_TYPE_DRIVER, 297000,
+ 4096, 5116, 5204, 5500, 0,
+ 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 24, },
+};
+
/*** DDC fetch and block validation ***/
static const u8 edid_header[] = {
@@ -2465,6 +2495,68 @@ do_cea_modes(struct drm_connector *connector, const u8
*db, u8 len)
return modes;
}
+/*
+ * do_hdmi_vsdb_modes - Parse the HDMI Vendor Specific data block
+ * @connector: connector corresponding to the HDMI sink
+ * @db: start of the CEA vendor specific block
+ * @len: length of the CEA block payload, ie. one can access up to db[len]
+ *
+ * Parses the HDMI VSDB looking for modes to add to @connector.
+ */
+static int
+do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len)
+{
+ struct drm_device *dev = connector-dev;
+ int modes = 0, offset = 0, i;
+ u8 vic_len;
+
+ if (len 8)
+ goto out;
+
+ /* no HDMI_Video_Present */
+ if (!(db[8] (1 5)))
+ goto out;
+
+ /* Latency_Fields_Present */
+ if (db[8] (1 7))
+ offset += 2;
+
+ /* I_Latency_Fields_Present */
+ if (db[8] (1 6))
+ offset += 2;
+
+ /* the declared length is not long enough for the 2 first bytes
+* of additional video format capabilities */
+ offset += 2;
+ if (len (8 + offset))
+ goto out;
+
+ vic_len = db[8 + offset] 5;
+
+ for (i = 0; i vic_len len = (9 + offset + i); i++) {
+ struct drm_display_mode *newmode;
+ u8 vic;
+
+ vic = db[9 + offset + i];
+
+ vic--; /* VICs start at 1 */
+ if (vic = ARRAY_SIZE(edid_4k_modes)) {
+ DRM_ERROR(Unknown HDMI VIC: %d\n, vic);
+ continue;
+ }
+
+ newmode = drm_mode_duplicate(dev, edid_4k_modes[vic]);
+ if (!newmode)
+ continue;
+
+ drm_mode_probed_add(connector, newmode);
+ modes++;
+ }
+
+out:
+ return modes;
+}
+
static int
cea_db_payload_len(const u8 *db)
{
@@ -2496,6 +2588,21 @@ cea_db_offsets(const u8 *cea, int *start, int *end)
return 0;
}
+static bool cea_db_is_hdmi_vsdb(const u8 *db)
+{
+ int hdmi_id;
+
+ if (cea_db_tag(db) != VENDOR_BLOCK)
+ return false;
+
+ if (cea_db_payload_len(db) 5)
+ return false;
+
+ hdmi_id = db[1] | (db[2] 8) | (db[3] 16);
+
+ return hdmi_id == HDMI_IDENTIFIER;
+}
+
#define for_each_cea_db(cea, i, start, end) \
for ((i) = (start); (i) (end) (i) +
cea_db_payload_len((cea)[(i)]) (end); (i) += cea_db_payload_len((cea)[(i)])
+ 1)
@@ -2519,6 +2626,8 @@ add_cea_modes(struct drm_connector *connector, struct
edid *edid)
[Intel-gfx] [PATCH 04/12] drm: Add support for alternate clocks of 4k modes
v2: Fix hmdi typo (Simon Farnsworth, Ville Syrjälä)
Suggested-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Reviewed-by: Simon Farnsworth simon.farnswo...@onelan.co.uk
---
drivers/gpu/drm/drm_edid.c | 68 ++
1 file changed, 62 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9de573c..2381abd 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2409,6 +2409,54 @@ u8 drm_match_cea_mode(const struct drm_display_mode
*to_match)
}
EXPORT_SYMBOL(drm_match_cea_mode);
+/*
+ * Calculate the alternate clock for HDMI modes (those from the HDMI vendor
+ * specific block).
+ *
+ * It's almost like cea_mode_alternate_clock(), we just need to add an
+ * exception for the VIC 4 mode (4096x2160@24Hz): no alternate clock for this
+ * one.
+ */
+static unsigned int
+hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode)
+{
+ if (hdmi_mode-vdisplay == 4096 hdmi_mode-hdisplay == 2160)
+ return hdmi_mode-clock;
+
+ return cea_mode_alternate_clock(hdmi_mode);
+}
+
+/*
+ * drm_match_hdmi_mode - look for a HDMI mode matching given mode
+ * @to_match: display mode
+ *
+ * An HDMI mode is one defined in the HDMI vendor specific block.
+ *
+ * Returns the HDMI Video ID (VIC) of the mode or 0 if it isn't one.
+ */
+static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match)
+{
+ u8 mode;
+
+ if (!to_match-clock)
+ return 0;
+
+ for (mode = 0; mode ARRAY_SIZE(edid_4k_modes); mode++) {
+ const struct drm_display_mode *hdmi_mode = edid_4k_modes[mode];
+ unsigned int clock1, clock2;
+
+ /* Make sure to also match alternate clocks */
+ clock1 = hdmi_mode-clock;
+ clock2 = hdmi_mode_alternate_clock(hdmi_mode);
+
+ if ((KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock1) ||
+KHZ2PICOS(to_match-clock) == KHZ2PICOS(clock2))
+ drm_mode_equal_no_clocks(to_match, hdmi_mode))
+ return mode + 1;
+ }
+ return 0;
+}
+
static int
add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
{
@@ -2426,18 +2474,26 @@ add_alternate_cea_modes(struct drm_connector
*connector, struct edid *edid)
* with the alternate clock for certain CEA modes.
*/
list_for_each_entry(mode, connector-probed_modes, head) {
- const struct drm_display_mode *cea_mode;
+ const struct drm_display_mode *cea_mode = NULL;
struct drm_display_mode *newmode;
- u8 cea_mode_idx = drm_match_cea_mode(mode) - 1;
+ u8 mode_idx = drm_match_cea_mode(mode) - 1;
unsigned int clock1, clock2;
- if (cea_mode_idx = ARRAY_SIZE(edid_cea_modes))
- continue;
+ if (mode_idx ARRAY_SIZE(edid_cea_modes)) {
+ cea_mode = edid_cea_modes[mode_idx];
+ clock2 = cea_mode_alternate_clock(cea_mode);
+ } else {
+ mode_idx = drm_match_hdmi_mode(mode) - 1;
+ if (mode_idx ARRAY_SIZE(edid_4k_modes)) {
+ cea_mode = edid_4k_modes[mode_idx];
+ clock2 = hdmi_mode_alternate_clock(cea_mode);
+ }
+ }
- cea_mode = edid_cea_modes[cea_mode_idx];
+ if (!cea_mode)
+ continue;
clock1 = cea_mode-clock;
- clock2 = cea_mode_alternate_clock(cea_mode);
if (clock1 == clock2)
continue;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/12] video/hdmi: Don't let the user of this API create invalid infoframes
To set the active aspect ratio value in the AVI infoframe today, you not
only have to set the active_aspect field, but also the active_info_valid
bit. Out of the 1 user of this API, we had 100% misuse, forgetting the
_valid bit. This was fixed in:
Author: Damien Lespiau damien.lesp...@intel.com
Date: Tue Aug 6 20:32:17 2013 +0100
drm: Don't generate invalid AVI infoframes for CEA modes
We can do better and derive the _valid bit from the user wanting to set
the active aspect ratio.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/drm_edid.c | 1 -
drivers/video/hdmi.c | 4 +++-
include/linux/hdmi.h | 1 -
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 2381abd..d76d608 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3259,7 +3259,6 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
hdmi_avi_infoframe *frame,
frame-video_code = drm_match_cea_mode(mode);
frame-picture_aspect = HDMI_PICTURE_ASPECT_NONE;
- frame-active_info_valid = 1;
frame-active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;
return 0;
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 635d569..e36da36 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -96,7 +96,9 @@ ssize_t hdmi_avi_infoframe_pack(struct hdmi_avi_infoframe
*frame, void *buffer,
ptr[0] = ((frame-colorspace 0x3) 5) | (frame-scan_mode 0x3);
- if (frame-active_info_valid)
+ /* Data byte 1, bit 4 has to be set if we provide the active format
+* aspect ratio */
+ if (frame-active_aspect 0xf)
ptr[0] |= BIT(4);
if (frame-horizontal_bar_valid)
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index bc6743e..931474c6 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -109,7 +109,6 @@ struct hdmi_avi_infoframe {
unsigned char version;
unsigned char length;
enum hdmi_colorspace colorspace;
- bool active_info_valid;
bool horizontal_bar_valid;
bool vertical_bar_valid;
enum hdmi_scan_mode scan_mode;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/12] video/hdmi: Derive the bar data valid bit from the bar data fields
Just like:
Author: Damien Lespiau damien.lesp...@intel.com
Date: Mon Aug 12 11:53:24 2013 +0100
video/hdmi: Don't let the user of this API create invalid infoframes
But this time for the horizontal/vertical bar data present bits.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/video/hdmi.c | 5 +++--
include/linux/hdmi.h | 2 --
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index e36da36..ac84215 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -101,10 +101,11 @@ ssize_t hdmi_avi_infoframe_pack(struct hdmi_avi_infoframe
*frame, void *buffer,
if (frame-active_aspect 0xf)
ptr[0] |= BIT(4);
- if (frame-horizontal_bar_valid)
+ /* Bit 3 and 2 indicate if we transmit horizontal/vertical bar data */
+ if (frame-top_bar || frame-bottom_bar)
ptr[0] |= BIT(3);
- if (frame-vertical_bar_valid)
+ if (frame-left_bar || frame-right_bar)
ptr[0] |= BIT(2);
ptr[1] = ((frame-colorimetry 0x3) 6) |
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 931474c6..b98340b 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -109,8 +109,6 @@ struct hdmi_avi_infoframe {
unsigned char version;
unsigned char length;
enum hdmi_colorspace colorspace;
- bool horizontal_bar_valid;
- bool vertical_bar_valid;
enum hdmi_scan_mode scan_mode;
enum hdmi_colorimetry colorimetry;
enum hdmi_picture_aspect picture_aspect;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/12] video/hdmi: Introduce helpers for the HDMI vendor specific infoframe
Provide the same programming model than the other infoframe types.
The generic _pack() function can't handle those yet as we need to move
the vendor OUI in the generic hdmi_vendor_infoframe structure to know
which kind of vendor infoframe we are dealing with.
v2: Fix the value of Side-by-side (half), hmdi typo, pack 3D_Ext_Data
(Ville Syrjälä)
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/video/hdmi.c | 88
include/linux/hdmi.h | 26
2 files changed, 114 insertions(+)
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index ac84215..59c4748 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -286,6 +286,94 @@ ssize_t hdmi_audio_infoframe_pack(struct
hdmi_audio_infoframe *frame,
EXPORT_SYMBOL(hdmi_audio_infoframe_pack);
/**
+ * hdmi_hdmi_infoframe_init() - initialize an HDMI vendor infoframe
+ * @frame: HDMI vendor infoframe
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame)
+{
+ memset(frame, 0, sizeof(*frame));
+
+ frame-type = HDMI_INFOFRAME_TYPE_VENDOR;
+ frame-version = 1;
+
+ /* 0 is a valid value for s3d_struct, so we use a special not set
+* value */
+ frame-s3d_struct = HDMI_3D_STRUCTURE_INVALID;
+
+ return 0;
+}
+EXPORT_SYMBOL(hdmi_hdmi_infoframe_init);
+
+/**
+ * hdmi_hdmi_infoframe_pack() - write a HDMI vendor infoframe to binary buffer
+ * @frame: HDMI infoframe
+ * @buffer: destination buffer
+ * @size: size of buffer
+ *
+ * Packs the information contained in the @frame structure into a binary
+ * representation that can be written into the corresponding controller
+ * registers. Also computes the checksum as required by section 5.3.5 of
+ * the HDMI 1.4 specification.
+ *
+ * Returns the number of bytes packed into the binary buffer or a negative
+ * error code on failure.
+ */
+ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
+void *buffer, size_t size)
+{
+ u8 *ptr = buffer;
+ size_t length;
+
+ /* empty info frame */
+ if (frame-vic == 0 frame-s3d_struct == HDMI_3D_STRUCTURE_INVALID)
+ return -EINVAL;
+
+ /* only one of those can be supplied */
+ if (frame-vic != 0 frame-s3d_struct != HDMI_3D_STRUCTURE_INVALID)
+ return -EINVAL;
+
+ /* for side by side (half) we also need to provide 3D_Ext_Data */
+ if (frame-s3d_struct == HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
+ frame-length = 6;
+ else
+ frame-length = 5;
+
+ length = HDMI_INFOFRAME_HEADER_SIZE + frame-length;
+
+ if (size length)
+ return -ENOSPC;
+
+ memset(buffer, 0, size);
+
+ ptr[0] = frame-type;
+ ptr[1] = frame-version;
+ ptr[2] = frame-length;
+ ptr[3] = 0; /* checksum */
+
+ /* HDMI OUI */
+ ptr[4] = 0x03;
+ ptr[5] = 0x0c;
+ ptr[6] = 0x00;
+
+ if (frame-vic) {
+ ptr[7] = 0x1 5; /* video format */
+ ptr[8] = frame-vic;
+ } else {
+ ptr[7] = 0x2 5; /* video format */
+ ptr[8] = (frame-s3d_struct 0xf) 4;
+ if (frame-s3d_struct == HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
+ ptr[9] = (frame-s3d_ext_data 0xf) 4;
+ }
+
+ hdmi_infoframe_checksum(buffer, length);
+
+ return length;
+}
+EXPORT_SYMBOL(hdmi_hdmi_infoframe_pack);
+
+/**
* hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary
*buffer
* @frame: HDMI vendor infoframe
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index b98340b..e733252 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -234,11 +234,37 @@ struct hdmi_vendor_infoframe {
ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
void *buffer, size_t size);
+enum hdmi_3d_structure {
+ HDMI_3D_STRUCTURE_INVALID = -1,
+ HDMI_3D_STRUCTURE_FRAME_PACKING = 0,
+ HDMI_3D_STRUCTURE_FIELD_ALTERNATIVE,
+ HDMI_3D_STRUCTURE_LINE_ALTERNATIVE,
+ HDMI_3D_STRUCTURE_SIDE_BY_SIDE_FULL,
+ HDMI_3D_STRUCTURE_L_DEPTH,
+ HDMI_3D_STRUCTURE_L_DEPTH_GFX_GFX_DEPTH,
+ HDMI_3D_STRUCTURE_TOP_AND_BOTTOM,
+ HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF = 8,
+};
+
+struct hdmi_hdmi_infoframe {
+ enum hdmi_infoframe_type type;
+ unsigned char version;
+ unsigned char length;
+ u8 vic;
+ enum hdmi_3d_structure s3d_struct;
+ unsigned int s3d_ext_data;
+};
+
+int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame);
+ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
+void *buffer, size_t size);
+
union hdmi_infoframe {
struct hdmi_any_infoframe any;
struct
[Intel-gfx] [PATCH 10/12] video/hdmi: Hook the HDMI vendor infoframe with the generic _pack()
With this last bit, hdmi_infoframe_pack() is now able to pack any
infoframe we support.
At the same time, because it's impractical to make two commits out of
this, we get rid of the version that encourages the open coding of the
vendor infoframe packing. We can do so because the only user of this API
has been ported in:
Author: Damien Lespiau damien.lesp...@intel.com
Date: Mon Aug 12 18:08:37 2013 +0100
gpu: host1x: Port the HDMI vendor infoframe code the common helpers
v2: Change oui to be an unsigned int (Ville Syrjälä)
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/video/hdmi.c | 45 +
include/linux/hdmi.h | 24
2 files changed, 21 insertions(+), 48 deletions(-)
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 59c4748..7ae4f80 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -298,6 +298,7 @@ int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe
*frame)
frame-type = HDMI_INFOFRAME_TYPE_VENDOR;
frame-version = 1;
+ frame-oui = HDMI_IDENTIFIER;
/* 0 is a valid value for s3d_struct, so we use a special not set
* value */
frame-s3d_struct = HDMI_3D_STRUCTURE_INVALID;
@@ -373,46 +374,18 @@ ssize_t hdmi_hdmi_infoframe_pack(struct
hdmi_hdmi_infoframe *frame,
}
EXPORT_SYMBOL(hdmi_hdmi_infoframe_pack);
-/**
- * hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary
- *buffer
- * @frame: HDMI vendor infoframe
- * @buffer: destination buffer
- * @size: size of buffer
- *
- * Packs the information contained in the @frame structure into a binary
- * representation that can be written into the corresponding controller
- * registers. Also computes the checksum as required by section 5.3.5 of
- * the HDMI 1.4 specification.
- *
- * Returns the number of bytes packed into the binary buffer or a negative
- * error code on failure.
+/*
+ * hdmi_vendor_infoframe_pack() - write a vendor infoframe to binary buffer
*/
-ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
- void *buffer, size_t size)
+static ssize_t hdmi_vendor_infoframe_pack(union hdmi_vendor_infoframe *frame,
+ void *buffer, size_t size)
{
- u8 *ptr = buffer;
- size_t length;
-
- length = HDMI_INFOFRAME_HEADER_SIZE + frame-length;
-
- if (size length)
- return -ENOSPC;
-
- memset(buffer, 0, size);
-
- ptr[0] = frame-type;
- ptr[1] = frame-version;
- ptr[2] = frame-length;
- ptr[3] = 0; /* checksum */
-
- memcpy(ptr[HDMI_INFOFRAME_HEADER_SIZE], frame-data, frame-length);
-
- hdmi_infoframe_checksum(buffer, length);
+ /* we only know about HDMI vendor infoframes */
+ if (frame-any.oui != HDMI_IDENTIFIER)
+ return -EINVAL;
- return length;
+ return hdmi_hdmi_infoframe_pack(frame-hdmi, buffer, size);
}
-EXPORT_SYMBOL(hdmi_vendor_infoframe_pack);
/**
* hdmi_infoframe_pack() - write a HDMI infoframe to binary buffer
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 37e0cd7..e24d850 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -225,16 +225,6 @@ int hdmi_audio_infoframe_init(struct hdmi_audio_infoframe
*frame);
ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
void *buffer, size_t size);
-struct hdmi_vendor_infoframe {
- enum hdmi_infoframe_type type;
- unsigned char version;
- unsigned char length;
- u8 data[27];
-};
-
-ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
- void *buffer, size_t size);
-
enum hdmi_3d_structure {
HDMI_3D_STRUCTURE_INVALID = -1,
HDMI_3D_STRUCTURE_FRAME_PACKING = 0,
@@ -251,6 +241,7 @@ struct hdmi_hdmi_infoframe {
enum hdmi_infoframe_type type;
unsigned char version;
unsigned char length;
+ unsigned int oui;
u8 vic;
enum hdmi_3d_structure s3d_struct;
unsigned int s3d_ext_data;
@@ -260,12 +251,21 @@ int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe
*frame);
ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
void *buffer, size_t size);
+union hdmi_vendor_infoframe {
+ struct {
+ enum hdmi_infoframe_type type;
+ unsigned char version;
+ unsigned char length;
+ unsigned int oui;
+ } any;
+ struct hdmi_hdmi_infoframe hdmi;
+};
+
union hdmi_infoframe {
struct hdmi_any_infoframe any;
struct hdmi_avi_infoframe avi;
struct hdmi_spd_infoframe spd;
- struct hdmi_vendor_infoframe vendor;
- struct hdmi_hdmi_infoframe hdmi;
+ union
[Intel-gfx] [PATCH 08/12] gpu: host1x: Port the HDMI vendor infoframe code the common helpers
I just wrote the bits to define and pack HDMI vendor specific infoframe.
Port the host1x driver to use those so I can refactor the infoframe code
a bit more.
This changes the length of the infoframe payload from 6 to 5, which is
enough for the frame packing stereo format.
v2: Pimp up the commit message with the note about the length
(Ville Syrjälä)
Cc: Thierry Reding thierry.red...@gmail.com
Cc: Terje Bergström tbergst...@nvidia.com
Cc: linux-te...@vger.kernel.org
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/host1x/drm/hdmi.c | 24
1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/host1x/drm/hdmi.c b/drivers/gpu/host1x/drm/hdmi.c
index 01097da..b548918 100644
--- a/drivers/gpu/host1x/drm/hdmi.c
+++ b/drivers/gpu/host1x/drm/hdmi.c
@@ -539,7 +539,7 @@ static void tegra_hdmi_setup_audio_infoframe(struct
tegra_hdmi *hdmi)
static void tegra_hdmi_setup_stereo_infoframe(struct tegra_hdmi *hdmi)
{
- struct hdmi_vendor_infoframe frame;
+ struct hdmi_hdmi_infoframe frame;
unsigned long value;
u8 buffer[10];
ssize_t err;
@@ -551,26 +551,10 @@ static void tegra_hdmi_setup_stereo_infoframe(struct
tegra_hdmi *hdmi)
return;
}
- memset(frame, 0, sizeof(frame));
+ hdmi_hdmi_infoframe_init(frame);
+ frame.s3d_struct = HDMI_3D_STRUCTURE_FRAME_PACKING;
- frame.type = HDMI_INFOFRAME_TYPE_VENDOR;
- frame.version = 0x01;
- frame.length = 6;
-
- frame.data[0] = 0x03; /* regid0 */
- frame.data[1] = 0x0c; /* regid1 */
- frame.data[2] = 0x00; /* regid2 */
- frame.data[3] = 0x02 5; /* video format */
-
- /* TODO: 74 MHz limit? */
- if (1) {
- frame.data[4] = 0x00 4; /* 3D structure */
- } else {
- frame.data[4] = 0x08 4; /* 3D structure */
- frame.data[5] = 0x00 4; /* 3D ext. data */
- }
-
- err = hdmi_vendor_infoframe_pack(frame, buffer, sizeof(buffer));
+ err = hdmi_hdmi_infoframe_pack(frame, buffer, sizeof(buffer));
if (err 0) {
dev_err(hdmi-dev, failed to pack vendor infoframe: %zd\n,
err);
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/12] drm/edid: Move HDMI_IDENTIFIER to hdmi.h
We'll need the HDMI OUI for the HDMI vendor infoframe data, so let's
move the DRM one to hdmi.h, might as well use the hdmi header to store
some hdmi defines.
(Note that, in fact, infoframes are part of the CEA-861 standard, and
only the HDMI vendor specific infoframe is special to HDMI, but
details..)
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/drm_edid.c | 1 -
include/linux/hdmi.h | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index d76d608..3aa653f 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2317,7 +2317,6 @@ add_detailed_modes(struct drm_connector *connector,
struct edid *edid,
return closure.modes;
}
-#define HDMI_IDENTIFIER 0x000C03
#define AUDIO_BLOCK0x01
#define VIDEO_BLOCK 0x02
#define VENDOR_BLOCK0x03
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index e733252..37e0cd7 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -18,6 +18,7 @@ enum hdmi_infoframe_type {
HDMI_INFOFRAME_TYPE_AUDIO = 0x84,
};
+#define HDMI_IDENTIFIER 0x000c03
#define HDMI_INFOFRAME_HEADER_SIZE 4
#define HDMI_AVI_INFOFRAME_SIZE13
#define HDMI_SPD_INFOFRAME_SIZE25
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/12] drm: Add a helper to forge HDMI vendor infoframes
This can then be used by DRM drivers to setup their vendor infoframes.
v2: Fix hmdi typo (Simon Farnsworth)
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Reviewed-by: Simon Farnsworth simon.farnswo...@onelan.co.uk
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/drm_edid.c | 36
include/drm/drm_edid.h | 4
2 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 3aa653f..42c62d1 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3263,3 +3263,39 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
hdmi_avi_infoframe *frame,
return 0;
}
EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
+
+/**
+ * drm_hdmi_vendor_infoframe_from_display_mode() - fill an HDMI infoframe with
+ * data from a DRM display mode
+ * @frame: HDMI vendor infoframe
+ * @mode: DRM display mode
+ *
+ * Note that there's is a need to send HDMI vendor infoframes only when using a
+ * 4k or stereoscopic 3D mode. So when giving any other mode as input this
+ * function will return -EINVAL, error that can be safely ignored.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int
+drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_hdmi_infoframe *frame,
+ const struct drm_display_mode *mode)
+{
+ int err;
+ u8 vic;
+
+ if (!frame || !mode)
+ return -EINVAL;
+
+ vic = drm_match_hdmi_mode(mode);
+ if (!vic)
+ return -EINVAL;
+
+ err = hdmi_hdmi_infoframe_init(frame);
+ if (err 0)
+ return err;
+
+ frame-vic = vic;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode);
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index fc481fc..a204e31 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -256,6 +256,7 @@ struct drm_encoder;
struct drm_connector;
struct drm_display_mode;
struct hdmi_avi_infoframe;
+struct hdmi_hdmi_infoframe;
void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid);
int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads);
@@ -268,5 +269,8 @@ int drm_load_edid_firmware(struct drm_connector *connector);
int
drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
const struct drm_display_mode *mode);
+int
+drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_hdmi_infoframe *frame,
+ const struct drm_display_mode
*mode);
#endif /* __DRM_EDID_H__ */
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 12/12] drm/i915/hdmi: Write HDMI vendor specific infoframes
With all the common infoframe bits now in place, we can finally write
the vendor specific infoframes in our driver.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_hdmi.c | 28
2 files changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 17f6252..33427fd1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4157,6 +4157,8 @@
_TRANSCODER(trans, HSW_VIDEO_DIP_CTL_A, HSW_VIDEO_DIP_CTL_B)
#define HSW_TVIDEO_DIP_AVI_DATA(trans) \
_TRANSCODER(trans, HSW_VIDEO_DIP_AVI_DATA_A, HSW_VIDEO_DIP_AVI_DATA_B)
+#define HSW_TVIDEO_DIP_VS_DATA(trans) \
+_TRANSCODER(trans, HSW_VIDEO_DIP_VS_DATA_A, HSW_VIDEO_DIP_VS_DATA_B)
#define HSW_TVIDEO_DIP_SPD_DATA(trans) \
_TRANSCODER(trans, HSW_VIDEO_DIP_SPD_DATA_A, HSW_VIDEO_DIP_SPD_DATA_B)
#define HSW_TVIDEO_DIP_GCP(trans) \
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index a619d94..4148cc8 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -74,6 +74,8 @@ static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
return VIDEO_DIP_SELECT_AVI;
case HDMI_INFOFRAME_TYPE_SPD:
return VIDEO_DIP_SELECT_SPD;
+ case HDMI_INFOFRAME_TYPE_VENDOR:
+ return VIDEO_DIP_SELECT_VENDOR;
default:
DRM_DEBUG_DRIVER(unknown info frame type %d\n, type);
return 0;
@@ -87,6 +89,8 @@ static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
return VIDEO_DIP_ENABLE_AVI;
case HDMI_INFOFRAME_TYPE_SPD:
return VIDEO_DIP_ENABLE_SPD;
+ case HDMI_INFOFRAME_TYPE_VENDOR:
+ return VIDEO_DIP_ENABLE_VENDOR;
default:
DRM_DEBUG_DRIVER(unknown info frame type %d\n, type);
return 0;
@@ -100,6 +104,8 @@ static u32 hsw_infoframe_enable(enum hdmi_infoframe_type
type)
return VIDEO_DIP_ENABLE_AVI_HSW;
case HDMI_INFOFRAME_TYPE_SPD:
return VIDEO_DIP_ENABLE_SPD_HSW;
+ case HDMI_INFOFRAME_TYPE_VENDOR:
+ return VIDEO_DIP_ENABLE_VS_HSW;
default:
DRM_DEBUG_DRIVER(unknown info frame type %d\n, type);
return 0;
@@ -114,6 +120,8 @@ static u32 hsw_infoframe_data_reg(enum hdmi_infoframe_type
type,
return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder);
case HDMI_INFOFRAME_TYPE_SPD:
return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder);
+ case HDMI_INFOFRAME_TYPE_VENDOR:
+ return HSW_TVIDEO_DIP_VS_DATA(cpu_transcoder);
default:
DRM_DEBUG_DRIVER(unknown info frame type %d\n, type);
return 0;
@@ -392,6 +400,21 @@ static void intel_hdmi_set_spd_infoframe(struct
drm_encoder *encoder)
intel_write_infoframe(encoder, frame);
}
+static void
+intel_hdmi_set_hdmi_infoframe(struct drm_encoder *encoder,
+ struct drm_display_mode *adjusted_mode)
+{
+ union hdmi_infoframe frame;
+ int ret;
+
+ ret = drm_hdmi_vendor_infoframe_from_display_mode(frame.vendor.hdmi,
+ adjusted_mode);
+ if (ret 0)
+ return;
+
+ intel_write_infoframe(encoder, frame);
+}
+
static void g4x_set_infoframes(struct drm_encoder *encoder,
struct drm_display_mode *adjusted_mode)
{
@@ -454,6 +477,7 @@ static void g4x_set_infoframes(struct drm_encoder *encoder,
intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
intel_hdmi_set_spd_infoframe(encoder);
+ intel_hdmi_set_hdmi_infoframe(encoder, adjusted_mode);
}
static void ibx_set_infoframes(struct drm_encoder *encoder,
@@ -515,6 +539,7 @@ static void ibx_set_infoframes(struct drm_encoder *encoder,
intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
intel_hdmi_set_spd_infoframe(encoder);
+ intel_hdmi_set_hdmi_infoframe(encoder, adjusted_mode);
}
static void cpt_set_infoframes(struct drm_encoder *encoder,
@@ -550,6 +575,7 @@ static void cpt_set_infoframes(struct drm_encoder *encoder,
intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
intel_hdmi_set_spd_infoframe(encoder);
+ intel_hdmi_set_hdmi_infoframe(encoder, adjusted_mode);
}
static void vlv_set_infoframes(struct drm_encoder *encoder,
@@ -584,6 +610,7 @@ static void vlv_set_infoframes(struct drm_encoder *encoder,
intel_hdmi_set_avi_infoframe(encoder, adjusted_mode);
intel_hdmi_set_spd_infoframe(encoder);
+ intel_hdmi_set_hdmi_infoframe(encoder, adjusted_mode);
}
static void hsw_set_infoframes(struct drm_encoder *encoder,
@@ -611,6 +638,7 @@
Re: [Intel-gfx] [PATCH 2/4] [v3] drm/i915: cleanup mapfence in bind
On Wed, Aug 14, 2013 at 10:18:55AM +0200, Daniel Vetter wrote:
On Tue, Aug 13, 2013 at 06:09:07PM -0700, Ben Widawsky wrote:
Cleanup the map and fenceable setting during bind to make more sense,
and not check i915_is_ggtt() 2 unnecessary times
v2: Move the bools into the if block (Chris) - There are ways to tidy
this function (fence calculations for instance) even further, but they
are quite invasive, so I am punting on those unless specifically asked.
v3: Add newline between variable declaration and logic (Chris)
Recommended-by: Chris Wilson ch...@chris-wilson.co.uk
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_gem.c | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index 4a58ead..01cc016 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3106,7 +3106,6 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object
*obj,
struct drm_device *dev = obj-base.dev;
drm_i915_private_t *dev_priv = dev-dev_private;
u32 size, fence_size, fence_alignment, unfenced_alignment;
- bool mappable, fenceable;
size_t gtt_max =
map_and_fenceable ? dev_priv-gtt.mappable_end : vm-total;
struct i915_vma *vma;
@@ -3191,18 +3190,18 @@ search_free:
list_move_tail(obj-global_list, dev_priv-mm.bound_list);
list_add_tail(vma-mm_list, vm-inactive_list);
- fenceable =
- i915_is_ggtt(vm)
- i915_gem_obj_ggtt_size(obj) == fence_size
- (i915_gem_obj_ggtt_offset(obj) (fence_alignment - 1)) == 0;
+ if (i915_is_ggtt(vm)) {
+ bool mappable, fenceable;
- mappable =
- i915_is_ggtt(vm)
- vma-node.start + obj-base.size = dev_priv-gtt.mappable_end;
+ fenceable =
+ i915_gem_obj_ggtt_size(obj) == fence_size
+ (i915_gem_obj_ggtt_offset(obj) (fence_alignment - 1))
== 0;
Why not go the full mounty and also use vma-node here? Would also make
checkpatch a bit more happy. I'll do a follow-up commit.
-Daniel
You've already done it, so it's moot. The idea I had was to use ggtt
as much as possible when it can only every be ggtt. I think this would
be helpful for both clarity, and debug.
The extra indirection would be immeasurable.
Again, you've already changed it, so meh.
--
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: inline vma_create into lookup_or_create_vma
On Wed, Aug 14, 2013 at 06:47:00PM +0200, Daniel Vetter wrote: On Wed, Aug 14, 2013 at 11:59:09AM +0200, Daniel Vetter wrote: In the execbuf code we don't clean up any vmas which ended up not getting bound for code simplicity. To make sure that we don't end up creating multiple vma for the same vm kill the somewhat dangerous vma_create function and inline it into lookup_or_create. This is just a safety measure to prevent surprises in the future. Also update the somewhat confused comment in the execbuf code and clarify what kind of magic is going on with a new one. Cc: Ben Widawsky b...@bwidawsk.net Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- That's the only concern I could come up with when reading the execbuf vma conversion patch. So looks good and I'll slurp it all in as soon as some more head scratching is done for the very first patch in this series about the vma_unbind fix to only call vma_destroy if the vma isn't bound. One thing I've noticed but forgot to mention here is that the reloc code still uses obj_ggtt_size/offset. I guess that will be fixed later on? -Daniel Yes. -- Ben Widawsky, 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] drm/i915: enable the power well before module unload
From: Paulo Zanoni paulo.r.zan...@intel.com
Our driver initialization doesn't seem to be ready to load when the
power well is disabled: we hit a few Unclaimed register messages. So
do just like we already do for the suspend/resume path: enable the
power well before unloading.
At some point we'll want to be able to survive suspend/resume and
load/unload with the power well disabled, but for now let's just fix
the regression.
Regression introduced by the following commit:
commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41
Author: Paulo Zanoni paulo.r.zan...@intel.com
Date: Wed Jul 3 17:12:13 2013 -0300
drm/i915: switch disable_power_well default value to 1
Bug can be reproduced by running the module_reload script from
intel-gpu-tools.
Cc: sta...@vger.kernel.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67813
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_dma.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
QA (the bug reporter) still didn't confirm this patch fixes the bug, but I can
reproduce it and I can confirm it fixes the problem at least on my machine.
Also please notice we have other problems that happen when we run module_reload
(e.g., gpu hang, no backlight, lockdep error message, eventual oops), this patch
fixes only the Unclaimed register errors.
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index b30404d..d2dc02b 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1688,8 +1688,13 @@ int i915_driver_unload(struct drm_device *dev)
intel_gpu_ips_teardown();
- if (HAS_POWER_WELL(dev))
+ if (HAS_POWER_WELL(dev)) {
+ /* The i915.ko module is still not prepared to be loaded when
+* the power well is not enabled, so just enable it in case
+* we're going to unload/reload. */
+ intel_set_power_well(dev, true);
i915_remove_power_well(dev);
+ }
i915_teardown_sysfs(dev);
--
1.8.1.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't load context at driver init time on SNB
On Tue, Aug 13, 2013 at 1:11 AM, Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Mon, Aug 12, 2013 at 10:33:37AM -0700, Stéphane Marchesin wrote: On Fri, Aug 9, 2013 at 9:55 PM, Ben Widawsky b...@bwidawsk.net wrote: On Fri, Aug 09, 2013 at 08:32:54PM -0700, Stéphane Marchesin wrote: This is a partial revert of b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 (drm/i915: load boot context at driver init time) This bit breaks hardware video decode for me after resume. Signed-off-by: Stéphane Marchesin marc...@chromium.org --- drivers/gpu/drm/i915/intel_pm.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index f895d15..ffa4ab2 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4614,10 +4614,6 @@ static void gen6_init_clock_gating(struct drm_device *dev) ILK_DPARBUNIT_CLOCK_GATE_ENABLE | ILK_DPFDUNIT_CLOCK_GATE_ENABLE); - /* WaMbcDriverBootEnable:snb */ - I915_WRITE(GEN6_MBCTL, I915_READ(GEN6_MBCTL) | -GEN6_MBCTL_ENABLE_BOOT_FETCH); - g4x_disable_trickle_feed(dev); /* The default value should be 0x200 according to docs, but the two I was looking at this a bit with Stéphane. One thing we both see is that the bit isn't sticking as it should. Clearly doing the write is having an effect, but something is seriously wrong. Writing the bit manually with IGT does make the bit stick. Stéphane, could you try to write the bit with IGT before and after resume, and see if it helps? It doesn't seem to stick, and so seems to have no effect. The confusing thing is that video decode works fine before suspend, even though that reg is 0. And after resume, it is broken, and that reg is still 0. So something else is going on. Maybe this reg is write-once-ever? BSpec says This Bit is cleared by the Hardware once the Boot fetch is complete. So it seems like the boot fetch doesn't work correctly, but still clears the bit? Stéphane ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] [v3] drm/i915: cleanup mapfence in bind
On Wed, Aug 14, 2013 at 10:27:42AM -0700, Ben Widawsky wrote:
On Wed, Aug 14, 2013 at 10:18:55AM +0200, Daniel Vetter wrote:
On Tue, Aug 13, 2013 at 06:09:07PM -0700, Ben Widawsky wrote:
Cleanup the map and fenceable setting during bind to make more sense,
and not check i915_is_ggtt() 2 unnecessary times
v2: Move the bools into the if block (Chris) - There are ways to tidy
this function (fence calculations for instance) even further, but they
are quite invasive, so I am punting on those unless specifically asked.
v3: Add newline between variable declaration and logic (Chris)
Recommended-by: Chris Wilson ch...@chris-wilson.co.uk
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_gem.c | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index 4a58ead..01cc016 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3106,7 +3106,6 @@ i915_gem_object_bind_to_vm(struct
drm_i915_gem_object *obj,
struct drm_device *dev = obj-base.dev;
drm_i915_private_t *dev_priv = dev-dev_private;
u32 size, fence_size, fence_alignment, unfenced_alignment;
- bool mappable, fenceable;
size_t gtt_max =
map_and_fenceable ? dev_priv-gtt.mappable_end : vm-total;
struct i915_vma *vma;
@@ -3191,18 +3190,18 @@ search_free:
list_move_tail(obj-global_list, dev_priv-mm.bound_list);
list_add_tail(vma-mm_list, vm-inactive_list);
- fenceable =
- i915_is_ggtt(vm)
- i915_gem_obj_ggtt_size(obj) == fence_size
- (i915_gem_obj_ggtt_offset(obj) (fence_alignment - 1)) == 0;
+ if (i915_is_ggtt(vm)) {
+ bool mappable, fenceable;
- mappable =
- i915_is_ggtt(vm)
- vma-node.start + obj-base.size = dev_priv-gtt.mappable_end;
+ fenceable =
+ i915_gem_obj_ggtt_size(obj) == fence_size
+ (i915_gem_obj_ggtt_offset(obj) (fence_alignment - 1))
== 0;
Why not go the full mounty and also use vma-node here? Would also make
checkpatch a bit more happy. I'll do a follow-up commit.
-Daniel
You've already done it, so it's moot. The idea I had was to use ggtt
as much as possible when it can only every be ggtt. I think this would
be helpful for both clarity, and debug.
I disagree here and I think the extra indirection hampers code readability
- I always end up checking your little functions to make sure they
actually fish out the right value from the right vma. So my plan is that
once this all lands I'll fully rip them out.
This wasn't ever about performance, although I admit that unnecessary
looping in our gem code does irk me a bit ;-) But again mostly from a
clarify pov.
For marking ggtt-only paths I think Chris' suggestion to enclose such code
in if (is_ggtt(vm)) {} blocks which you've implemented here is much better
for clarity.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] VGA arbiter support for Intel HD?
On Wed, Aug 14, 2013 at 11:14:48AM -0600, Alex Williamson wrote: On Wed, 2013-08-14 at 17:47 +0300, Ville Syrjälä wrote: On Wed, Aug 14, 2013 at 07:23:57AM -0600, Alex Williamson wrote: Hi, I'm trying to add support for device assignment of PCI VGA devices with VFIO and QEMU. For normal, discrete discrete graphics the Linux VGA arbiter works fairly well, disabling VGA on one bridge and adding it to another (though I wish all the kernel VGA drivers made use of it). The i915 driver only seems to support disabling VGA on really old GMCH devices (see intel_modeset_vga_set_state). This means that if I boot with IGD as the primary graphics and attempt to assign a discrete graphics device, all the VGA range accesses are still routed to IGD, I end up getting some error messages from the IGD interrupt handler, and the discrete card never initializes. I spent some time looking through the Sand Bridge, Ivy Bridge, and Haswell datasheets, and I'm a bit concerned whether the hardware even provides a reasonable way to disable VGA anymore. Quoting 2.17 from the Haswell docs: Accesses to the VGA memory range are directed to IGD depend on the configuration. The configuration is specified by: * Internal graphics controller in Device 2 is enabled (DEVEN.D2EN bit 4) * Internal graphics VGA in Device 0 Function 0 is enabled through register GGC bit 1. * IGD's memory accesses (PCICMD2 04 – 05h, MAE bit 1) in Device 2 configuration space are enabled. * VGA compatibility memory accesses (VGA Miscellaneous Output register – MSR Register, bit 1) are enabled. * Software sets the proper value for VGA Memory Map Mode register (VGA GR06 Register, bits 3-2). See the following table for translations. (There's a similar list for VGA I/O range) I've found that if I disable memory and I/O in the PCI command register for IGD then I do get VGA routing to the PEG device and the discrete VBIOS works. This obviously isn't a good option for the VGA arbiter since it entirely disables IGD. The GGC registers aren't meant for runtime switching and are actually locked. Disabling IGD via the device 2 enable bit doesn't seem like and option. I don't quite understand the VGA miscellaneous output register and VGA memory map mode, but the table provided for the latter makes me think they just augment the VGA ranges and don't disable them. Bit 1 of MSR (0x3c2/0x3cc) should allow you to turn off VGA mem access while leaving other memory space access working. As for VGA I/O decode, IIRC there's no standard bit for that in VGA or PCI config registers, and I can't see any other bit for it in the docs. But I guess you could just turn off I/O space completely via the PCI_COMMAND register. We shouldn't need it for anything beyond i915_disable_vga() and that has the necessary vgaarb calls already. Thanks Ville. The MSR seems to work for VGA memory. Disabling I/O via PCI_COMMAND does works, but something is re-enabling it after intel_modeset_vga_set_state(). If I manually disable I/O with setpci then I do have VGA routing to PEG and can still interact with the KMS console on IGD. It's unfortunate that the MSR bit for I/O only disables pieces of the range. If we have no other options, I'll try to hunt down where I/O is being re-enabled and see how feasible it is to avoid. Thanks, Hmm. Now that I look at vgaarb more it seems I misunderstood the way it works. Based on the code it looks like it will permanently remove the device from the arbiration if set_vga_decode indicates that it doesn't decode legacy resources. And it calls set_vga_decode w/ decode=false if there are more than two VGA cards in the system. That means i915_disable_vga() is actually broken whenever there is another VGA card in the system. To make it work I think what we'd need to do is always return VGA_RSRC_LEGACY_IO from i915_vga_set_decode(), which will leave the PCI_COMMAND I/O bit in the hands of vgaarb, and then poke at the MSR register to disable the VGA mem decode permanently. But to touch MSR we actually need VGA I/O, so I guess we'd have to do that right after registering w/ vgaarb. Doing it from i915_vga_set_decode() doesn't look possible since there's no guarantee that VGA I/O would end up at the right device at the time that is called. So maybe the following patch might work (although maybe vgaarb itself should be extended a bit to properly support this use case). From 90070e547f1582f8e73d9221d6a31502dea8246d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= ville.syrj...@linux.intel.com Date: Wed, 14 Aug 2013 21:20:57 +0300 Subject: [PATCH] vga arb stuf --- drivers/gpu/drm/i915/i915_dma.c | 13
Re: [Intel-gfx] [PATCH 6.1/9] drm/i915: don't queue PM events we won't process
On Fri, Aug 09, 2013 at 05:04:35PM -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
On SNB/IVB/VLV we only call gen6_rps_irq_handler if one of the IIR
bits set is part of GEN6_PM_RPS_EVENTS, but at gen6_rps_irq_handler we
add all the enabled IIR bits to the work queue, not only the ones that
are part of GEN6_PM_RPS_EVENTS. But then gen6_pm_rps_work only
processes GEN6_PM_RPS_EVENTS, so it's useless to add anything that's
not GEN6_PM_RPS_EVENTS to the work queue.
As a bonus, gen6_rps_irq_handler looks more similar to
hsw_pm_irq_handler, so we may be able to merge them in the future.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 0f46d33..5b51c43 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -959,7 +959,7 @@ static void gen6_rps_irq_handler(struct drm_i915_private
*dev_priv,
*/
spin_lock(dev_priv-irq_lock);
- dev_priv-rps.pm_iir |= pm_iir;
+ dev_priv-rps.pm_iir |= pm_iir GEN6_PM_RPS_EVENTS;
snb_set_pm_irq(dev_priv, dev_priv-rps.pm_iir);
spin_unlock(dev_priv-irq_lock);
@@ -1128,7 +1128,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void
*arg)
if (pipe_stats[0] PIPE_GMBUS_INTERRUPT_STATUS)
gmbus_irq_handler(dev);
- if (pm_iir GEN6_PM_RPS_EVENTS)
+ if (pm_iir)
gen6_rps_irq_handler(dev_priv, pm_iir);
I915_WRITE(GTIIR, gt_iir);
@@ -1433,7 +1433,7 @@ static irqreturn_t ironlake_irq_handler(int irq, void
*arg)
if (pm_iir) {
if (IS_HASWELL(dev))
hsw_pm_irq_handler(dev_priv, pm_iir);
- else if (pm_iir GEN6_PM_RPS_EVENTS)
+ else
gen6_rps_irq_handler(dev_priv, pm_iir);
I915_WRITE(GEN6_PMIIR, pm_iir);
ret = IRQ_HANDLED;
Can you please add WARN_ON(pm_iir ~GEN6_PM_RPS_EVENTS) somewhere in
the code path to make me happy?
Otherwise it's:
Reviewed-by: Ben Widawsky b...@bwidawsk.net
--
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/12] video/hdmi: Introduce helpers for the HDMI vendor specific infoframe
On Wed, Aug 14, 2013 at 06:19:10PM +0100, Damien Lespiau wrote:
Provide the same programming model than the other infoframe types.
The generic _pack() function can't handle those yet as we need to move
the vendor OUI in the generic hdmi_vendor_infoframe structure to know
which kind of vendor infoframe we are dealing with.
v2: Fix the value of Side-by-side (half), hmdi typo, pack 3D_Ext_Data
(Ville Syrjälä)
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/video/hdmi.c | 88
include/linux/hdmi.h | 26
2 files changed, 114 insertions(+)
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index ac84215..59c4748 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -286,6 +286,94 @@ ssize_t hdmi_audio_infoframe_pack(struct
hdmi_audio_infoframe *frame,
EXPORT_SYMBOL(hdmi_audio_infoframe_pack);
/**
+ * hdmi_hdmi_infoframe_init() - initialize an HDMI vendor infoframe
+ * @frame: HDMI vendor infoframe
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int hdmi_hdmi_infoframe_init(struct hdmi_hdmi_infoframe *frame)
+{
+ memset(frame, 0, sizeof(*frame));
+
+ frame-type = HDMI_INFOFRAME_TYPE_VENDOR;
+ frame-version = 1;
+
+ /* 0 is a valid value for s3d_struct, so we use a special not set
+ * value */
+ frame-s3d_struct = HDMI_3D_STRUCTURE_INVALID;
+
+ return 0;
+}
+EXPORT_SYMBOL(hdmi_hdmi_infoframe_init);
+
+/**
+ * hdmi_hdmi_infoframe_pack() - write a HDMI vendor infoframe to binary
buffer
+ * @frame: HDMI infoframe
+ * @buffer: destination buffer
+ * @size: size of buffer
+ *
+ * Packs the information contained in the @frame structure into a binary
+ * representation that can be written into the corresponding controller
+ * registers. Also computes the checksum as required by section 5.3.5 of
+ * the HDMI 1.4 specification.
+ *
+ * Returns the number of bytes packed into the binary buffer or a negative
+ * error code on failure.
+ */
+ssize_t hdmi_hdmi_infoframe_pack(struct hdmi_hdmi_infoframe *frame,
+ void *buffer, size_t size)
+{
+ u8 *ptr = buffer;
+ size_t length;
+
+ /* empty info frame */
+ if (frame-vic == 0 frame-s3d_struct == HDMI_3D_STRUCTURE_INVALID)
+ return -EINVAL;
+
+ /* only one of those can be supplied */
+ if (frame-vic != 0 frame-s3d_struct != HDMI_3D_STRUCTURE_INVALID)
+ return -EINVAL;
+
+ /* for side by side (half) we also need to provide 3D_Ext_Data */
+ if (frame-s3d_struct == HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
Could be = for a bit of future proofing since the spec says we should
send 3d_ext_data even for the reserved 8 values.
+ frame-length = 6;
+ else
+ frame-length = 5;
+
+ length = HDMI_INFOFRAME_HEADER_SIZE + frame-length;
+
+ if (size length)
+ return -ENOSPC;
+
+ memset(buffer, 0, size);
+
+ ptr[0] = frame-type;
+ ptr[1] = frame-version;
+ ptr[2] = frame-length;
+ ptr[3] = 0; /* checksum */
+
+ /* HDMI OUI */
+ ptr[4] = 0x03;
+ ptr[5] = 0x0c;
+ ptr[6] = 0x00;
+
+ if (frame-vic) {
+ ptr[7] = 0x1 5; /* video format */
+ ptr[8] = frame-vic;
+ } else {
+ ptr[7] = 0x2 5; /* video format */
+ ptr[8] = (frame-s3d_struct 0xf) 4;
+ if (frame-s3d_struct == HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
Could be = here too.
But whether or not you make those changes:
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
+ ptr[9] = (frame-s3d_ext_data 0xf) 4;
+ }
+
+ hdmi_infoframe_checksum(buffer, length);
+
+ return length;
+}
+EXPORT_SYMBOL(hdmi_hdmi_infoframe_pack);
+
+/**
* hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary
*buffer
* @frame: HDMI vendor infoframe
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index b98340b..e733252 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -234,11 +234,37 @@ struct hdmi_vendor_infoframe {
ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
void *buffer, size_t size);
+enum hdmi_3d_structure {
+ HDMI_3D_STRUCTURE_INVALID = -1,
+ HDMI_3D_STRUCTURE_FRAME_PACKING = 0,
+ HDMI_3D_STRUCTURE_FIELD_ALTERNATIVE,
+ HDMI_3D_STRUCTURE_LINE_ALTERNATIVE,
+ HDMI_3D_STRUCTURE_SIDE_BY_SIDE_FULL,
+ HDMI_3D_STRUCTURE_L_DEPTH,
+ HDMI_3D_STRUCTURE_L_DEPTH_GFX_GFX_DEPTH,
+ HDMI_3D_STRUCTURE_TOP_AND_BOTTOM,
+ HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF = 8,
+};
+
+struct hdmi_hdmi_infoframe {
+ enum hdmi_infoframe_type type;
+ unsigned char version;
+ unsigned char length;
+ u8 vic;
+ enum
Re: [Intel-gfx] HDMI 4k support v3
On Wed, Aug 14, 2013 at 06:19:03PM +0100, Damien Lespiau wrote: Follow up on v2: http://lists.freedesktop.org/archives/dri-devel/2013-August/043604.html With the quick and nice reviews from Ville and Simon, it's time for a v3: - Fix embarrassing hmdi typo - Fix the sending of vendor specific infoframes for side-by-side half modes - Smaller changes here and there. Looking very good. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com for the rest as well. -- 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 6.3/9] drm/i915: merge HSW and SNB PM irq handlers
On Fri, Aug 09, 2013 at 05:04:37PM -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
Because hsw_pm_irq_handler does exactly what gen6_rps_irq_handler does
and also processes the 2 additional VEBOX bits. So merge those
functions and wrap the VEBOX bits on an IS_HASWELL check. This HSW
check isn't really necessary since the bits are reserved on
SNB/IVB/VLV, but it's a good documentation on who uses them.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 50
++---
1 file changed, 12 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d9ebfb6..8ba5d0a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -939,28 +939,6 @@ static void snb_gt_irq_handler(struct drm_device *dev,
ivybridge_parity_error_irq_handler(dev);
}
-/* Legacy way of handling PM interrupts */
-static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv,
- u32 pm_iir)
-{
- /*
- * IIR bits should never already be set because IMR should
- * prevent an interrupt from being shown in IIR. The warning
- * displays a case where we've unsafely cleared
- * dev_priv-rps.pm_iir. Although missing an interrupt of the same
- * type is not a problem, it displays a problem in the logic.
- *
- * The mask bit in IMR is cleared by dev_priv-rps.work.
- */
-
- spin_lock(dev_priv-irq_lock);
- dev_priv-rps.pm_iir |= pm_iir GEN6_PM_RPS_EVENTS;
- snb_disable_pm_irq(dev_priv, pm_iir GEN6_PM_RPS_EVENTS);
- spin_unlock(dev_priv-irq_lock);
-
- queue_work(dev_priv-wq, dev_priv-rps.work);
-}
-
#define HPD_STORM_DETECT_PERIOD 1000
#define HPD_STORM_THRESHOLD 5
@@ -1027,13 +1005,10 @@ static void dp_aux_irq_handler(struct drm_device *dev)
wake_up_all(dev_priv-gmbus_wait_queue);
}
-/* Unlike gen6_rps_irq_handler() from which this function is originally
derived,
- * we must be able to deal with other PM interrupts. This is complicated
because
- * of the way in which we use the masks to defer the RPS work (which for
- * posterity is necessary because of forcewake).
- */
-static void hsw_pm_irq_handler(struct drm_i915_private *dev_priv,
-u32 pm_iir)
+/* The RPS events need forcewake, so we add them to a work queue and mask
their
+ * IMR bits until the work is done. Other interrupts can be processed without
+ * the work queue. */
+static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32
pm_iir)
{
if (pm_iir GEN6_PM_RPS_EVENTS) {
spin_lock(dev_priv-irq_lock);
@@ -1044,12 +1019,14 @@ static void hsw_pm_irq_handler(struct
drm_i915_private *dev_priv,
queue_work(dev_priv-wq, dev_priv-rps.work);
}
- if (pm_iir PM_VEBOX_USER_INTERRUPT)
- notify_ring(dev_priv-dev, dev_priv-ring[VECS]);
+ if (IS_HASWELL(dev_priv-dev)) {
+ if (pm_iir PM_VEBOX_USER_INTERRUPT)
+ notify_ring(dev_priv-dev, dev_priv-ring[VECS]);
Make this HAS_VEBOX() instead of IS_HASWELL()
- if (pm_iir PM_VEBOX_CS_ERROR_INTERRUPT) {
- DRM_ERROR(VEBOX CS error interrupt 0x%08x\n, pm_iir);
- i915_handle_error(dev_priv-dev, false);
+ if (pm_iir PM_VEBOX_CS_ERROR_INTERRUPT) {
+ DRM_ERROR(VEBOX CS error interrupt 0x%08x\n, pm_iir);
+ i915_handle_error(dev_priv-dev, false);
+ }
}
}
@@ -1424,10 +1401,7 @@ static irqreturn_t ironlake_irq_handler(int irq, void
*arg)
if (INTEL_INFO(dev)-gen = 6) {
u32 pm_iir = I915_READ(GEN6_PMIIR);
if (pm_iir) {
- if (IS_HASWELL(dev))
- hsw_pm_irq_handler(dev_priv, pm_iir);
- else
- gen6_rps_irq_handler(dev_priv, pm_iir);
+ gen6_rps_irq_handler(dev_priv, pm_iir);
I915_WRITE(GEN6_PMIIR, pm_iir);
ret = IRQ_HANDLED;
}
With the couple of small comments, these 3 patches are:
Reviewed-by: Ben Widawsky b...@bwidawsk.net
Though after the IRC discussion you may want to discount my review.
--
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] VGA arbiter support for Intel HD?
On Wed, 2013-08-14 at 21:30 +0300, Ville Syrjälä wrote: On Wed, Aug 14, 2013 at 11:14:48AM -0600, Alex Williamson wrote: On Wed, 2013-08-14 at 17:47 +0300, Ville Syrjälä wrote: On Wed, Aug 14, 2013 at 07:23:57AM -0600, Alex Williamson wrote: Hi, I'm trying to add support for device assignment of PCI VGA devices with VFIO and QEMU. For normal, discrete discrete graphics the Linux VGA arbiter works fairly well, disabling VGA on one bridge and adding it to another (though I wish all the kernel VGA drivers made use of it). The i915 driver only seems to support disabling VGA on really old GMCH devices (see intel_modeset_vga_set_state). This means that if I boot with IGD as the primary graphics and attempt to assign a discrete graphics device, all the VGA range accesses are still routed to IGD, I end up getting some error messages from the IGD interrupt handler, and the discrete card never initializes. I spent some time looking through the Sand Bridge, Ivy Bridge, and Haswell datasheets, and I'm a bit concerned whether the hardware even provides a reasonable way to disable VGA anymore. Quoting 2.17 from the Haswell docs: Accesses to the VGA memory range are directed to IGD depend on the configuration. The configuration is specified by: * Internal graphics controller in Device 2 is enabled (DEVEN.D2EN bit 4) * Internal graphics VGA in Device 0 Function 0 is enabled through register GGC bit 1. * IGD's memory accesses (PCICMD2 04 – 05h, MAE bit 1) in Device 2 configuration space are enabled. * VGA compatibility memory accesses (VGA Miscellaneous Output register – MSR Register, bit 1) are enabled. * Software sets the proper value for VGA Memory Map Mode register (VGA GR06 Register, bits 3-2). See the following table for translations. (There's a similar list for VGA I/O range) I've found that if I disable memory and I/O in the PCI command register for IGD then I do get VGA routing to the PEG device and the discrete VBIOS works. This obviously isn't a good option for the VGA arbiter since it entirely disables IGD. The GGC registers aren't meant for runtime switching and are actually locked. Disabling IGD via the device 2 enable bit doesn't seem like and option. I don't quite understand the VGA miscellaneous output register and VGA memory map mode, but the table provided for the latter makes me think they just augment the VGA ranges and don't disable them. Bit 1 of MSR (0x3c2/0x3cc) should allow you to turn off VGA mem access while leaving other memory space access working. As for VGA I/O decode, IIRC there's no standard bit for that in VGA or PCI config registers, and I can't see any other bit for it in the docs. But I guess you could just turn off I/O space completely via the PCI_COMMAND register. We shouldn't need it for anything beyond i915_disable_vga() and that has the necessary vgaarb calls already. Thanks Ville. The MSR seems to work for VGA memory. Disabling I/O via PCI_COMMAND does works, but something is re-enabling it after intel_modeset_vga_set_state(). If I manually disable I/O with setpci then I do have VGA routing to PEG and can still interact with the KMS console on IGD. It's unfortunate that the MSR bit for I/O only disables pieces of the range. If we have no other options, I'll try to hunt down where I/O is being re-enabled and see how feasible it is to avoid. Thanks, Hmm. Now that I look at vgaarb more it seems I misunderstood the way it works. Based on the code it looks like it will permanently remove the device from the arbiration if set_vga_decode indicates that it doesn't decode legacy resources. And it calls set_vga_decode w/ decode=false if there are more than two VGA cards in the system. That means s/more than two/two or more/ i915_disable_vga() is actually broken whenever there is another VGA card in the system. I didn't follow why i915_disable_vga is broken. It seems like the intention is to disable VGA regardless of how many VGA devices are present. To make it work I think what we'd need to do is always return VGA_RSRC_LEGACY_IO from i915_vga_set_decode(), which will leave the PCI_COMMAND I/O bit in the hands of vgaarb, and then poke at the MSR register to disable the VGA mem decode permanently. But to touch MSR we actually need VGA I/O, so I guess we'd have to do that right after registering w/ vgaarb. Doing it from i915_vga_set_decode() doesn't look possible since there's no guarantee that VGA I/O would end up at the right device at the time that is called. So maybe the following patch might work (although maybe vgaarb itself should be extended
Re: [Intel-gfx] VGA arbiter support for Intel HD?
On Wed, Aug 14, 2013 at 01:39:55PM -0600, Alex Williamson wrote: On Wed, 2013-08-14 at 21:30 +0300, Ville Syrjälä wrote: On Wed, Aug 14, 2013 at 11:14:48AM -0600, Alex Williamson wrote: On Wed, 2013-08-14 at 17:47 +0300, Ville Syrjälä wrote: On Wed, Aug 14, 2013 at 07:23:57AM -0600, Alex Williamson wrote: Hi, I'm trying to add support for device assignment of PCI VGA devices with VFIO and QEMU. For normal, discrete discrete graphics the Linux VGA arbiter works fairly well, disabling VGA on one bridge and adding it to another (though I wish all the kernel VGA drivers made use of it). The i915 driver only seems to support disabling VGA on really old GMCH devices (see intel_modeset_vga_set_state). This means that if I boot with IGD as the primary graphics and attempt to assign a discrete graphics device, all the VGA range accesses are still routed to IGD, I end up getting some error messages from the IGD interrupt handler, and the discrete card never initializes. I spent some time looking through the Sand Bridge, Ivy Bridge, and Haswell datasheets, and I'm a bit concerned whether the hardware even provides a reasonable way to disable VGA anymore. Quoting 2.17 from the Haswell docs: Accesses to the VGA memory range are directed to IGD depend on the configuration. The configuration is specified by: * Internal graphics controller in Device 2 is enabled (DEVEN.D2EN bit 4) * Internal graphics VGA in Device 0 Function 0 is enabled through register GGC bit 1. * IGD's memory accesses (PCICMD2 04 – 05h, MAE bit 1) in Device 2 configuration space are enabled. * VGA compatibility memory accesses (VGA Miscellaneous Output register – MSR Register, bit 1) are enabled. * Software sets the proper value for VGA Memory Map Mode register (VGA GR06 Register, bits 3-2). See the following table for translations. (There's a similar list for VGA I/O range) I've found that if I disable memory and I/O in the PCI command register for IGD then I do get VGA routing to the PEG device and the discrete VBIOS works. This obviously isn't a good option for the VGA arbiter since it entirely disables IGD. The GGC registers aren't meant for runtime switching and are actually locked. Disabling IGD via the device 2 enable bit doesn't seem like and option. I don't quite understand the VGA miscellaneous output register and VGA memory map mode, but the table provided for the latter makes me think they just augment the VGA ranges and don't disable them. Bit 1 of MSR (0x3c2/0x3cc) should allow you to turn off VGA mem access while leaving other memory space access working. As for VGA I/O decode, IIRC there's no standard bit for that in VGA or PCI config registers, and I can't see any other bit for it in the docs. But I guess you could just turn off I/O space completely via the PCI_COMMAND register. We shouldn't need it for anything beyond i915_disable_vga() and that has the necessary vgaarb calls already. Thanks Ville. The MSR seems to work for VGA memory. Disabling I/O via PCI_COMMAND does works, but something is re-enabling it after intel_modeset_vga_set_state(). If I manually disable I/O with setpci then I do have VGA routing to PEG and can still interact with the KMS console on IGD. It's unfortunate that the MSR bit for I/O only disables pieces of the range. If we have no other options, I'll try to hunt down where I/O is being re-enabled and see how feasible it is to avoid. Thanks, Hmm. Now that I look at vgaarb more it seems I misunderstood the way it works. Based on the code it looks like it will permanently remove the device from the arbiration if set_vga_decode indicates that it doesn't decode legacy resources. And it calls set_vga_decode w/ decode=false if there are more than two VGA cards in the system. That means s/more than two/two or more/ i915_disable_vga() is actually broken whenever there is another VGA card in the system. I didn't follow why i915_disable_vga is broken. It seems like the intention is to disable VGA regardless of how many VGA devices are present. I suppose that's the original intention, but i915_disable_vga() needs VGA I/O access, so it is currently broken if VGA I/O is disabled. Originally i915_disable_vga() used MMIO access, but there's a hardware workaround (since ILK IIRC) stating that VGA registers must be accessed w/ port I/O. That's progress for you :( -- Ville Syrjälä Intel OTC ___ Intel-gfx
[Intel-gfx] [PATCH] drm/i915: Only unmask required PM interrupts
Un-masking all PM interrupts causes hardware to generate interrupts regardless of whether the interrupts are enabled on the DE side. Since turbo only need up/down threshold and rc6 timeout interrupt, mask all other interrupts bits to avoid unnecessary overhead/wake up. Change-Id: I6c947df6fd5f60584d39b9e8b8c89faa51a5e827 Signed-off-by: Vinit Azad vinit.a...@intel.com --- drivers/gpu/drm/i915/intel_pm.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8e9ce07..17a0dae 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3328,8 +3328,8 @@ static void gen6_enable_rps(struct drm_device *dev) I915_WRITE(GEN6_PMIMR, I915_READ(GEN6_PMIMR) ~GEN6_PM_RPS_EVENTS); I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS); spin_unlock_irq(dev_priv-rps.lock); - /* unmask all PM interrupts */ - I915_WRITE(GEN6_PMINTRMSK, 0); + /* only unmask PM interrupts we need. Mask all others. */ + I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS); rc6vids = 0; ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, rc6vids); @@ -3596,8 +3596,8 @@ static void valleyview_enable_rps(struct drm_device *dev) WARN_ON(dev_priv-rps.pm_iir != 0); I915_WRITE(GEN6_PMIMR, 0); spin_unlock_irq(dev_priv-rps.lock); - /* enable all PM interrupts */ - I915_WRITE(GEN6_PMINTRMSK, 0); + /* enable only the PM interrupts we need. Mask everything else */ + I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS); gen6_gt_force_wake_put(dev_priv); } -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Only unmask required PM interrupts
On Wed, Aug 14, 2013 at 10:34 PM, Vinit Azad vinit.a...@intel.com wrote: Un-masking all PM interrupts causes hardware to generate interrupts regardless of whether the interrupts are enabled on the DE side. Since turbo only need up/down threshold and rc6 timeout interrupt, mask all other interrupts bits to avoid unnecessary overhead/wake up. Just to clarify since I can't really believe this yet: Even though we disable all other interrupt sources in PMIER and mask them in PMIMR hw still manages to fire off our interrupt handler? Do those interrupts end up setting PMIIR? Thanks, Daniel Change-Id: I6c947df6fd5f60584d39b9e8b8c89faa51a5e827 Signed-off-by: Vinit Azad vinit.a...@intel.com --- drivers/gpu/drm/i915/intel_pm.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8e9ce07..17a0dae 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3328,8 +3328,8 @@ static void gen6_enable_rps(struct drm_device *dev) I915_WRITE(GEN6_PMIMR, I915_READ(GEN6_PMIMR) ~GEN6_PM_RPS_EVENTS); I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS); spin_unlock_irq(dev_priv-rps.lock); - /* unmask all PM interrupts */ - I915_WRITE(GEN6_PMINTRMSK, 0); + /* only unmask PM interrupts we need. Mask all others. */ + I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS); rc6vids = 0; ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, rc6vids); @@ -3596,8 +3596,8 @@ static void valleyview_enable_rps(struct drm_device *dev) WARN_ON(dev_priv-rps.pm_iir != 0); I915_WRITE(GEN6_PMIMR, 0); spin_unlock_irq(dev_priv-rps.lock); - /* enable all PM interrupts */ - I915_WRITE(GEN6_PMINTRMSK, 0); + /* enable only the PM interrupts we need. Mask everything else */ + I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS); gen6_gt_force_wake_put(dev_priv); } -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Only unmask required PM interrupts
Our interrupt handler isn't being fired since we do set the IER bits properly (IIR bits aren't set). The overhead isn't because our driver is reacting to these interrupts, but because hardware keeps generating internal messages when PMINTRMSK doesn't mask out the up/down EI interrupts (which happen periodically). Unfortunately, the default value of PMINTRMSK register is 0 instead of 0x as it is for other IMR, so we have to mask out the interrupts manually. -Original Message- From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Wednesday, August 14, 2013 13:47 To: Azad, Vinit Cc: intel-gfx Subject: Re: [Intel-gfx] [PATCH] drm/i915: Only unmask required PM interrupts On Wed, Aug 14, 2013 at 10:34 PM, Vinit Azad vinit.a...@intel.com wrote: Un-masking all PM interrupts causes hardware to generate interrupts regardless of whether the interrupts are enabled on the DE side. Since turbo only need up/down threshold and rc6 timeout interrupt, mask all other interrupts bits to avoid unnecessary overhead/wake up. Just to clarify since I can't really believe this yet: Even though we disable all other interrupt sources in PMIER and mask them in PMIMR hw still manages to fire off our interrupt handler? Do those interrupts end up setting PMIIR? Thanks, Daniel Change-Id: I6c947df6fd5f60584d39b9e8b8c89faa51a5e827 Signed-off-by: Vinit Azad vinit.a...@intel.com --- drivers/gpu/drm/i915/intel_pm.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8e9ce07..17a0dae 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3328,8 +3328,8 @@ static void gen6_enable_rps(struct drm_device *dev) I915_WRITE(GEN6_PMIMR, I915_READ(GEN6_PMIMR) ~GEN6_PM_RPS_EVENTS); I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS); spin_unlock_irq(dev_priv-rps.lock); - /* unmask all PM interrupts */ - I915_WRITE(GEN6_PMINTRMSK, 0); + /* only unmask PM interrupts we need. Mask all others. */ + I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS); rc6vids = 0; ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, rc6vids); @@ -3596,8 +3596,8 @@ static void valleyview_enable_rps(struct drm_device *dev) WARN_ON(dev_priv-rps.pm_iir != 0); I915_WRITE(GEN6_PMIMR, 0); spin_unlock_irq(dev_priv-rps.lock); - /* enable all PM interrupts */ - I915_WRITE(GEN6_PMINTRMSK, 0); + /* enable only the PM interrupts we need. Mask everything else */ + I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS); gen6_gt_force_wake_put(dev_priv); } -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Only unmask required PM interrupts
On Wed, Aug 14, 2013 at 09:01:22PM +, Azad, Vinit wrote: Our interrupt handler isn't being fired since we do set the IER bits properly (IIR bits aren't set). The overhead isn't because our driver is reacting to these interrupts, but because hardware keeps generating internal messages when PMINTRMSK doesn't mask out the up/down EI interrupts (which happen periodically). Ah, that makes more sense. Added to the commit message, please elaborate more precisely for the next patch what exactly the effects are. Unfortunately, the default value of PMINTRMSK register is 0 instead of 0x as it is for other IMR, so we have to mask out the interrupts manually. -Original Message- From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Wednesday, August 14, 2013 13:47 To: Azad, Vinit Cc: intel-gfx Subject: Re: [Intel-gfx] [PATCH] drm/i915: Only unmask required PM interrupts On Wed, Aug 14, 2013 at 10:34 PM, Vinit Azad vinit.a...@intel.com wrote: Un-masking all PM interrupts causes hardware to generate interrupts regardless of whether the interrupts are enabled on the DE side. Since turbo only need up/down threshold and rc6 timeout interrupt, mask all other interrupts bits to avoid unnecessary overhead/wake up. Just to clarify since I can't really believe this yet: Even though we disable all other interrupt sources in PMIER and mask them in PMIMR hw still manages to fire off our interrupt handler? Do those interrupts end up setting PMIIR? Thanks, Daniel Change-Id: I6c947df6fd5f60584d39b9e8b8c89faa51a5e827 Signed-off-by: Vinit Azad vinit.a...@intel.com Patch merged to dinq with a bit of manual fixup. Please base patches on top of my drm-intel-nightly tree if possible, only really critical fixes (for hangs, black screens, ...) should be pased on -fixes. Thanks for the patch, -Daniel --- drivers/gpu/drm/i915/intel_pm.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8e9ce07..17a0dae 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3328,8 +3328,8 @@ static void gen6_enable_rps(struct drm_device *dev) I915_WRITE(GEN6_PMIMR, I915_READ(GEN6_PMIMR) ~GEN6_PM_RPS_EVENTS); I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS); spin_unlock_irq(dev_priv-rps.lock); - /* unmask all PM interrupts */ - I915_WRITE(GEN6_PMINTRMSK, 0); + /* only unmask PM interrupts we need. Mask all others. */ + I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS); rc6vids = 0; ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, rc6vids); @@ -3596,8 +3596,8 @@ static void valleyview_enable_rps(struct drm_device *dev) WARN_ON(dev_priv-rps.pm_iir != 0); I915_WRITE(GEN6_PMIMR, 0); spin_unlock_irq(dev_priv-rps.lock); - /* enable all PM interrupts */ - I915_WRITE(GEN6_PMINTRMSK, 0); + /* enable only the PM interrupts we need. Mask everything else */ + I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS); gen6_gt_force_wake_put(dev_priv); } -- 1.7.9.5 ___ 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] intel-gpu-tools: --disable-tests option should make cairo-check obsolete
On Wed, Aug 14, 2013 at 11:03 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Aug 14, 2013 at 10:33:42PM +0200, Sedat Dilek wrote: Hi, I am here on Ubuntu/precise and wanted to avoid to upgrade to a higher cairo-version. Daniel's position is that he wants to make it hard for QA to build the tests incorrectly and so skip half of them. I think defaulting to erroring out at configure if cairo is too old, but allowing an explicit --disable-cairo should make all us happy. Hmm, just curious how this should work? Beyond testdisplay... kms_flip and kms_render tests require cairo.h include - not sure which version of cairo is minimum. So, --disable-cairo configure-option should disable above tests? [ tests/Makefile.am ] ... testdisplay_SOURCES = \ testdisplay.c \ testdisplay.h \ testdisplay_hotplug.c \ $(NULL) TESTS_progs += testdisplay LDADD += $(CAIRO_LIBS) $(LIBUDEV_LIBS) $(GLIB_LIBS) AM_CFLAGS += $(CAIRO_CFLAGS) $(LIBUDEV_CFLAGS) $(GLIB_CFLAGS) ... $ grep -i cairo.h -nr ./ ./lib/drmtest.h:34:#include cairo.h ./tests/kms_flip.c:28:#include cairo.h ./tests/kms_render.c:27:#include cairo.h ./tests/testdisplay.c:52:#include cairo.h Thanks in advance. - Sedat - ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] assembler: error for the wrong syntax of SEND instruction on GEN6+
From: Xiang, Haihao haihao.xi...@intel.com
predicate SEND execsize dst sendleadreg payload directsrcoperand instoptions
predicate SEND execsize dst sendleadreg payload imm32reg instoptions
predicate SEND execsize dst sendleadreg payload sndopr imm32reg instoptions
predicate SEND execsize dst sendleadreg payload exp directsrcoperand
instoptions
The above four syntaxes are only used on legacy platforms which support implied
move
from payload to dst.
Signed-off-by: Xiang, Haihao haihao.xi...@intel.com
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
Somehow this ended up in our internal IGT. Anyone have an issue with me
pushing it to igt proper?
---
assembler/gram.y | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/assembler/gram.y b/assembler/gram.y
index e58c1fe..8795797 100644
--- a/assembler/gram.y
+++ b/assembler/gram.y
@@ -1215,6 +1215,9 @@ sendinstruction: predicate sendop execsize exp post_dst
payload msgtarget
}
| predicate sendop execsize dst sendleadreg payload
directsrcoperand instoptions
{
+ if (IS_GENp(6))
+ error(@2, the syntax of send instruction\n);
+
memset($$, 0, sizeof($$));
set_instruction_opcode($$, $2);
GEN($$)-header.destreg__conditionalmod = $5.nr; /* msg reg
index */
@@ -1233,6 +1236,9 @@ sendinstruction: predicate sendop execsize exp post_dst
payload msgtarget
}
| predicate sendop execsize dst sendleadreg payload imm32reg
instoptions
{
+ if (IS_GENp(6))
+ error(@2, the syntax of send instruction\n);
+
if ($7.reg.type != BRW_REGISTER_TYPE_UD
$7.reg.type != BRW_REGISTER_TYPE_D
$7.reg.type != BRW_REGISTER_TYPE_V) {
@@ -1336,6 +1342,9 @@ sendinstruction: predicate sendop execsize exp post_dst
payload msgtarget
}
| predicate sendop execsize dst sendleadreg payload sndopr
imm32reg instoptions
{
+ if (IS_GENp(6))
+ error(@2, the syntax of send instruction\n);
+
if ($8.reg.type != BRW_REGISTER_TYPE_UD
$8.reg.type != BRW_REGISTER_TYPE_D
$8.reg.type != BRW_REGISTER_TYPE_V) {
@@ -1355,15 +1364,16 @@ sendinstruction: predicate sendop execsize exp post_dst
payload msgtarget
if (set_instruction_src1($$, $8, @8) != 0)
YYERROR;
- if (IS_GENp(8)) {
- gen8_set_eot(GEN8($$), !!($7 EX_DESC_EOT_MASK));
- } else if (IS_GENx(5)) {
+ if (IS_GENx(5)) {
GEN($$)-bits2.send_gen5.sfid = ($7 EX_DESC_SFID_MASK);
GEN($$)-bits3.generic_gen5.end_of_thread = !!($7
EX_DESC_EOT_MASK);
}
}
| predicate sendop execsize dst sendleadreg payload exp
directsrcoperand instoptions
{
+ if (IS_GENp(6))
+ error(@2, the syntax of send instruction\n);
+
memset($$, 0, sizeof($$));
set_instruction_opcode($$, $2);
GEN($$)-header.destreg__conditionalmod = $5.nr; /* msg reg
index */
--
1.8.3.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/5] drm/i915: explicit store base gem object in dma_buf-priv
On Thu, Aug 08, 2013 at 09:10:38AM +0200, Daniel Vetter wrote:
Makes it more obviously correct what tricks we play by reusing the drm
prime release helper.
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
Ok, to get things going I've merged the two i915 patches to dinq.
-Daniel
---
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index f7e1682..e918b05 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -27,10 +27,15 @@
#include i915_drv.h
#include linux/dma-buf.h
+static struct drm_i915_gem_object *dma_buf_to_obj(struct dma_buf *buf)
+{
+ return to_intel_bo(buf-priv);
+}
+
static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment
*attachment,
enum dma_data_direction dir)
{
- struct drm_i915_gem_object *obj = attachment-dmabuf-priv;
+ struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment-dmabuf);
struct sg_table *st;
struct scatterlist *src, *dst;
int ret, i;
@@ -85,7 +90,7 @@ static void i915_gem_unmap_dma_buf(struct
dma_buf_attachment *attachment,
struct sg_table *sg,
enum dma_data_direction dir)
{
- struct drm_i915_gem_object *obj = attachment-dmabuf-priv;
+ struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment-dmabuf);
mutex_lock(obj-base.dev-struct_mutex);
@@ -100,7 +105,7 @@ static void i915_gem_unmap_dma_buf(struct
dma_buf_attachment *attachment,
static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
{
- struct drm_i915_gem_object *obj = dma_buf-priv;
+ struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
struct drm_device *dev = obj-base.dev;
struct sg_page_iter sg_iter;
struct page **pages;
@@ -148,7 +153,7 @@ error:
static void i915_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr)
{
- struct drm_i915_gem_object *obj = dma_buf-priv;
+ struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
struct drm_device *dev = obj-base.dev;
int ret;
@@ -191,7 +196,7 @@ static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf,
struct vm_area_struct *
static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, size_t start,
size_t length, enum dma_data_direction direction)
{
- struct drm_i915_gem_object *obj = dma_buf-priv;
+ struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
struct drm_device *dev = obj-base.dev;
int ret;
bool write = (direction == DMA_BIDIRECTIONAL || direction ==
DMA_TO_DEVICE);
@@ -222,9 +227,7 @@ static const struct dma_buf_ops i915_dmabuf_ops = {
struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
struct drm_gem_object *gem_obj, int flags)
{
- struct drm_i915_gem_object *obj = to_intel_bo(gem_obj);
-
- return dma_buf_export(obj, i915_dmabuf_ops, obj-base.size, flags);
+ return dma_buf_export(gem_obj, i915_dmabuf_ops, gem_obj-size, flags);
}
static int i915_gem_object_get_pages_dmabuf(struct drm_i915_gem_object *obj)
@@ -261,7 +264,7 @@ struct drm_gem_object *i915_gem_prime_import(struct
drm_device *dev,
/* is this one of own objects? */
if (dma_buf-ops == i915_dmabuf_ops) {
- obj = dma_buf-priv;
+ obj = dma_buf_to_obj(dma_buf);
/* is it from our device? */
if (obj-base.dev == dev) {
/*
--
1.8.3.2
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] intel-gpu-tools: --disable-tests option should make cairo-check obsolete
On Wed, Aug 14, 2013 at 11:25 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Aug 14, 2013 at 11:14:05PM +0200, Sedat Dilek wrote: On Wed, Aug 14, 2013 at 11:03 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Aug 14, 2013 at 10:33:42PM +0200, Sedat Dilek wrote: Hi, I am here on Ubuntu/precise and wanted to avoid to upgrade to a higher cairo-version. Daniel's position is that he wants to make it hard for QA to build the tests incorrectly and so skip half of them. I think defaulting to erroring out at configure if cairo is too old, but allowing an explicit --disable-cairo should make all us happy. Hmm, just curious how this should work? Beyond testdisplay... kms_flip and kms_render tests require cairo.h include - not sure which version of cairo is minimum. So, --disable-cairo configure-option should disable above tests? [ tests/Makefile.am ] ... testdisplay_SOURCES = \ testdisplay.c \ testdisplay.h \ testdisplay_hotplug.c \ $(NULL) TESTS_progs += testdisplay LDADD += $(CAIRO_LIBS) $(LIBUDEV_LIBS) $(GLIB_LIBS) AM_CFLAGS += $(CAIRO_CFLAGS) $(LIBUDEV_CFLAGS) $(GLIB_CFLAGS) ... $ grep -i cairo.h -nr ./ ./lib/drmtest.h:34:#include cairo.h ./tests/kms_flip.c:28:#include cairo.h ./tests/kms_render.c:27:#include cairo.h ./tests/testdisplay.c:52:#include cairo.h First you would need to split the cairo portion of drmtest.[ch] into its own files and then conditionally compile those, along with the cairo dependent tests. As said I am not an autotools expert. But I was interested in the tools not building the tests. That's why I asked why I am forced to upgrade to a higher cairo release. Why does tools require $(CAIRO_CFLAGS) and $(CAIRO_LIBS)? Hmm, looks like intel_perf_counters and intel_l3_parity do... tools/Makefile.am:40:AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) tools/Makefile.am:41:LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS) $(CAIRO_LIBS) tools/intel_perf_counters.c:47:#include drmtest.h tools/intel_l3_parity.c:38:#include drmtest.h --disable-cairo might be a good idea, but I cannot help here. - Sedat - ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 00/20] prime patches
Hi all, So I've finally tracked down the last thing which I didn't really understand in my earlier patches and made sure it won't ever break again by writing testcases. The one thing still left to do (but I have tests for it already) is to make sure we don't duplicate buffers when importing foreign buffers on two open fds. This is the use-case for which the exynos guys recently posted a few hacky patches. I've already merged the i915 patches from this series. Since there's no real functional depency all the patches here can go through drm-next without issues. Commentsflames highly welcome. Cheers, Daniel Daniel Vetter (19): drm: use common drm_gem_dmabuf_release in i915/exynos drivers drm/prime: remove cargo-cult locking from map_sg helper drm/prime: add a bit of documentation about gem_obj-import_attach drm/gem: move drm_gem_object_handle_unreference_unlocked into drm_gem.c drm/gem: remove bogus NULL check from drm_gem_object_handle_unreference_unlocked drm/gem: WARN about unbalanced handle refcounts drm/gem: fix up flink name create race drm/prime: fix error path in drm_gem_prime_fd_to_handle drm/gem: make drm_gem_object_handle_unreference_unlocked static drm/gem: create drm_gem_dumb_destroy drm/prime: use proper pointer in drm_gem_prime_handle_to_fd drm/prime: shrink critical section protected by prime lock drm/prime: clarify logic a bit in drm_gem_prime_fd_to_handle drm/gem: switch dev-object_name_lock to a mutex drm/gem: completely close gem_open vs. gem_close races drm/prime: proper locking+refcounting for obj-dma_buf link drm/prime: Simplify drm_gem_remove_prime_handles drm/prime: make drm_prime_lookup_buf_handle static drm/prime: Always add exported buffers to the handle cache Inki Dae (1): drm/exynos: explicit store base gem object in dma_buf-priv drivers/gpu/drm/drm_fops.c | 1 + drivers/gpu/drm/drm_gem.c | 178 ++- drivers/gpu/drm/drm_info.c | 2 +- drivers/gpu/drm/drm_prime.c| 190 ++--- drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 35 ++ drivers/gpu/drm/exynos/exynos_drm_gem.c| 2 +- drivers/gpu/drm/i915/i915_gem_dmabuf.c | 13 +- include/drm/drmP.h | 79 ++-- 8 files changed, 297 insertions(+), 203 deletions(-) -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/20] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
Note that this is slightly tricky since both drivers store their
native objects in dma_buf-priv. But both also embed the base
drm_gem_object at the first position, so the implicit cast is ok.
To use the release helper we need to export it, too.
Cc: Inki Dae inki@samsung.com
Cc: Intel Graphics Development intel-gfx@lists.freedesktop.org
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/drm_prime.c| 3 ++-
drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 23 +--
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 13 +
include/drm/drmP.h | 1 +
4 files changed, 5 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 85e450e..a35f206 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -192,7 +192,7 @@ static void drm_gem_unmap_dma_buf(struct dma_buf_attachment
*attach,
/* nothing to be done here */
}
-static void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
+void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
{
struct drm_gem_object *obj = dma_buf-priv;
@@ -202,6 +202,7 @@ static void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
drm_gem_object_unreference_unlocked(obj);
}
}
+EXPORT_SYMBOL(drm_gem_dmabuf_release);
static void *drm_gem_dmabuf_vmap(struct dma_buf *dma_buf)
{
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index a0f997e..3cd56e1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -127,27 +127,6 @@ static void exynos_gem_unmap_dma_buf(struct
dma_buf_attachment *attach,
/* Nothing to do. */
}
-static void exynos_dmabuf_release(struct dma_buf *dmabuf)
-{
- struct exynos_drm_gem_obj *exynos_gem_obj = dmabuf-priv;
-
- /*
-* exynos_dmabuf_release() call means that file object's
-* f_count is 0 and it calls drm_gem_object_handle_unreference()
-* to drop the references that these values had been increased
-* at drm_prime_handle_to_fd()
-*/
- if (exynos_gem_obj-base.export_dma_buf == dmabuf) {
- exynos_gem_obj-base.export_dma_buf = NULL;
-
- /*
-* drop this gem object refcount to release allocated buffer
-* and resources.
-*/
- drm_gem_object_unreference_unlocked(exynos_gem_obj-base);
- }
-}
-
static void *exynos_gem_dmabuf_kmap_atomic(struct dma_buf *dma_buf,
unsigned long page_num)
{
@@ -193,7 +172,7 @@ static struct dma_buf_ops exynos_dmabuf_ops = {
.kunmap = exynos_gem_dmabuf_kunmap,
.kunmap_atomic = exynos_gem_dmabuf_kunmap_atomic,
.mmap = exynos_gem_dmabuf_mmap,
- .release= exynos_dmabuf_release,
+ .release= drm_gem_dmabuf_release,
};
struct dma_buf *exynos_dmabuf_prime_export(struct drm_device *drm_dev,
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index f7af7e4..e918b05 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -103,17 +103,6 @@ static void i915_gem_unmap_dma_buf(struct
dma_buf_attachment *attachment,
mutex_unlock(obj-base.dev-struct_mutex);
}
-static void i915_gem_dmabuf_release(struct dma_buf *dma_buf)
-{
- struct drm_i915_gem_object *obj = dma_buf-priv;
-
- if (obj-base.export_dma_buf == dma_buf) {
- /* drop the reference on the export fd holds */
- obj-base.export_dma_buf = NULL;
- drm_gem_object_unreference_unlocked(obj-base);
- }
-}
-
static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
{
struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
@@ -224,7 +213,7 @@ static int i915_gem_begin_cpu_access(struct dma_buf
*dma_buf, size_t start, size
static const struct dma_buf_ops i915_dmabuf_ops = {
.map_dma_buf = i915_gem_map_dma_buf,
.unmap_dma_buf = i915_gem_unmap_dma_buf,
- .release = i915_gem_dmabuf_release,
+ .release = drm_gem_dmabuf_release,
.kmap = i915_gem_dmabuf_kmap,
.kmap_atomic = i915_gem_dmabuf_kmap_atomic,
.kunmap = i915_gem_dmabuf_kunmap,
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3ecdde6..016e75e 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1495,6 +1495,7 @@ extern struct drm_gem_object *drm_gem_prime_import(struct
drm_device *dev,
struct dma_buf *dma_buf);
extern int drm_gem_prime_fd_to_handle(struct drm_device *dev,
struct drm_file *file_priv, int prime_fd, uint32_t *handle);
+extern void drm_gem_dmabuf_release(struct dma_buf *dma_buf);
extern int drm_prime_handle_to_fd_ioctl(struct drm_device *dev,
[Intel-gfx] [PATCH 02/20] drm/exynos: explicit store base gem object in dma_buf-priv
From: Inki Dae inki@samsung.com
Signed-off-by: Inki Dae inki@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index 3cd56e1..fd76449 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -22,6 +22,11 @@ struct exynos_drm_dmabuf_attachment {
bool is_mapped;
};
+static struct exynos_drm_gem_obj *dma_buf_to_obj(struct dma_buf *buf)
+{
+ return to_exynos_gem_obj(buf-priv);
+}
+
static int exynos_gem_attach_dma_buf(struct dma_buf *dmabuf,
struct device *dev,
struct dma_buf_attachment *attach)
@@ -63,7 +68,7 @@ static struct sg_table *
enum dma_data_direction dir)
{
struct exynos_drm_dmabuf_attachment *exynos_attach = attach-priv;
- struct exynos_drm_gem_obj *gem_obj = attach-dmabuf-priv;
+ struct exynos_drm_gem_obj *gem_obj = dma_buf_to_obj(attach-dmabuf);
struct drm_device *dev = gem_obj-base.dev;
struct exynos_drm_gem_buf *buf;
struct scatterlist *rd, *wr;
@@ -180,7 +185,7 @@ struct dma_buf *exynos_dmabuf_prime_export(struct
drm_device *drm_dev,
{
struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
- return dma_buf_export(exynos_gem_obj, exynos_dmabuf_ops,
+ return dma_buf_export(obj, exynos_dmabuf_ops,
exynos_gem_obj-base.size, flags);
}
@@ -198,8 +203,7 @@ struct drm_gem_object *exynos_dmabuf_prime_import(struct
drm_device *drm_dev,
if (dma_buf-ops == exynos_dmabuf_ops) {
struct drm_gem_object *obj;
- exynos_gem_obj = dma_buf-priv;
- obj = exynos_gem_obj-base;
+ obj = dma_buf-priv;
/* is it from our device? */
if (obj-dev == drm_dev) {
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/20] drm/prime: add a bit of documentation about gem_obj-import_attach
Lifetime rules seem to be solid around -import_attach. So this patch
just properly documents them.
Note that pointing directly at the attachment might have issues for
devices that have multiple struct device *dev parts constituting the
logical gpu and so might need multiple attachment points. Similarly
for drm devices which don't need a dma attachment at all (like udl).
But fixing that up is material for different patches.
Reviewed-by: Rob Clark robdcl...@gmail.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
include/drm/drmP.h | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 016e75e..3711e97 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -678,7 +678,16 @@ struct drm_gem_object {
/* dma buf exported from this GEM object */
struct dma_buf *export_dma_buf;
- /* dma buf attachment backing this object */
+ /**
+* import_attach - dma buf attachment backing this object
+*
+* Any foreign dma_buf imported as a gem object has this set to the
+* attachment point for the device. This is invariant over the lifetime
+* of a gem object.
+*
+* The driver's -gem_free_object callback is responsible for cleaning
+* up the dma_buf attachment and references acquired at import time.
+*/
struct dma_buf_attachment *import_attach;
};
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/20] drm/prime: remove cargo-cult locking from map_sg helper
I've checked both implementations (radeon/nouveau) and they both grab
the page array from ttm simply by dereferencing it and then wrapping
it up with drm_prime_pages_to_sg in the callback and map it with
dma_map_sg (in the helper).
Only the grabbing of the underlying page array is anything we need to
be concerned about, and either those pages are pinned independently,
or we're screwed no matter what.
And indeed, nouveau/radeon pin the backing storage in their
attach/detach functions.
Since I've created this patch cma prime support for dma_buf was added.
drm_gem_cma_prime_get_sg_table only calls kzalloc and the createsmaps
the sg table with dma_get_sgtable. It doesn't touch any gem object
state otherwise. So the cma helpers also look safe.
The only thing we might claim it does is prevent concurrent mapping of
dma_buf attachments. But a) that's not allowed and b) the current code
is racy already since it checks whether the sg mapping exists _before_
grabbing the lock.
So the dev-struct_mutex locking here does absolutely nothing useful,
but only distracts. Remove it.
This should also help Maarten's work to eventually pin the backing
storage more dynamically by preventing locking inversions around
dev-struct_mutex.
v2: Add analysis for recently added cma helper prime code.
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: Maarten Lankhorst maarten.lankho...@canonical.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Acked-by: Maarten Lankhorst maarten.lankho...@canonical.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/drm_prime.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index a35f206..f115962 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -167,8 +167,6 @@ static struct sg_table *drm_gem_map_dma_buf(struct
dma_buf_attachment *attach,
if (WARN_ON(prime_attach-dir != DMA_NONE))
return ERR_PTR(-EBUSY);
- mutex_lock(obj-dev-struct_mutex);
-
sgt = obj-dev-driver-gem_prime_get_sg_table(obj);
if (!IS_ERR(sgt)) {
@@ -182,7 +180,6 @@ static struct sg_table *drm_gem_map_dma_buf(struct
dma_buf_attachment *attach,
}
}
- mutex_unlock(obj-dev-struct_mutex);
return sgt;
}
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/20] drm/gem: move drm_gem_object_handle_unreference_unlocked into drm_gem.c
We have three callers of this function now and it's neither
performance critical nor really small. So an inline function feels
like overkill and unecessarily separates the different parts of the
code.
Since all callers of drm_gem_object_handle_free are now in drm_gem.c
we can make that static (and remove the unused EXPORT_SYMBOL). To
avoid a forward declaration move it (and drm_gem_object_free_bug) up a
bit.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/drm_gem.c | 89 ---
include/drm/drmP.h| 21 +--
2 files changed, 55 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 9ab038c..77c7d19 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -201,6 +201,60 @@ drm_gem_remove_prime_handles(struct drm_gem_object *obj,
struct drm_file *filp)
}
}
+static void drm_gem_object_ref_bug(struct kref *list_kref)
+{
+ BUG();
+}
+
+/**
+ * Called after the last handle to the object has been closed
+ *
+ * Removes any name for the object. Note that this must be
+ * called before drm_gem_object_free or we'll be touching
+ * freed memory
+ */
+static void drm_gem_object_handle_free(struct drm_gem_object *obj)
+{
+ struct drm_device *dev = obj-dev;
+
+ /* Remove any name for this object */
+ spin_lock(dev-object_name_lock);
+ if (obj-name) {
+ idr_remove(dev-object_name_idr, obj-name);
+ obj-name = 0;
+ spin_unlock(dev-object_name_lock);
+ /*
+* The object name held a reference to this object, drop
+* that now.
+ *
+ * This cannot be the last reference, since the handle holds one
too.
+*/
+ kref_put(obj-refcount, drm_gem_object_ref_bug);
+ } else
+ spin_unlock(dev-object_name_lock);
+
+}
+
+void
+drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj)
+{
+ if (obj == NULL)
+ return;
+
+ if (atomic_read(obj-handle_count) == 0)
+ return;
+
+ /*
+ * Must bump handle count first as this may be the last
+ * ref, in which case the object would disappear before we
+ * checked for a name
+ */
+
+ if (atomic_dec_and_test(obj-handle_count))
+ drm_gem_object_handle_free(obj);
+ drm_gem_object_unreference_unlocked(obj);
+}
+
/**
* Removes the mapping from handle to filp for this object.
*/
@@ -533,41 +587,6 @@ drm_gem_object_free(struct kref *kref)
}
EXPORT_SYMBOL(drm_gem_object_free);
-static void drm_gem_object_ref_bug(struct kref *list_kref)
-{
- BUG();
-}
-
-/**
- * Called after the last handle to the object has been closed
- *
- * Removes any name for the object. Note that this must be
- * called before drm_gem_object_free or we'll be touching
- * freed memory
- */
-void drm_gem_object_handle_free(struct drm_gem_object *obj)
-{
- struct drm_device *dev = obj-dev;
-
- /* Remove any name for this object */
- spin_lock(dev-object_name_lock);
- if (obj-name) {
- idr_remove(dev-object_name_idr, obj-name);
- obj-name = 0;
- spin_unlock(dev-object_name_lock);
- /*
-* The object name held a reference to this object, drop
-* that now.
- *
- * This cannot be the last reference, since the handle holds one
too.
-*/
- kref_put(obj-refcount, drm_gem_object_ref_bug);
- } else
- spin_unlock(dev-object_name_lock);
-
-}
-EXPORT_SYMBOL(drm_gem_object_handle_free);
-
void drm_gem_vm_open(struct vm_area_struct *vma)
{
struct drm_gem_object *obj = vma-vm_private_data;
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3711e97..91c8d05 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1572,7 +1572,6 @@ int drm_gem_object_init(struct drm_device *dev,
struct drm_gem_object *obj, size_t size);
void drm_gem_private_object_init(struct drm_device *dev,
struct drm_gem_object *obj, size_t size);
-void drm_gem_object_handle_free(struct drm_gem_object *obj);
void drm_gem_vm_open(struct vm_area_struct *vma);
void drm_gem_vm_close(struct vm_area_struct *vma);
int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
@@ -1619,25 +1618,7 @@ drm_gem_object_handle_reference(struct drm_gem_object
*obj)
atomic_inc(obj-handle_count);
}
-static inline void
-drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj)
-{
- if (obj == NULL)
- return;
-
- if (atomic_read(obj-handle_count) == 0)
- return;
-
- /*
- * Must bump handle count first as this may be the last
- * ref, in which case the object would disappear
[Intel-gfx] [PATCH 06/20] drm/gem: remove bogus NULL check from drm_gem_object_handle_unreference_unlocked
Calling this function with a NULL object is simply a bug, so papering
over a NULL object not a good idea.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/drm_gem.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 77c7d19..b1d9e25 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -238,9 +238,6 @@ static void drm_gem_object_handle_free(struct
drm_gem_object *obj)
void
drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj)
{
- if (obj == NULL)
- return;
-
if (atomic_read(obj-handle_count) == 0)
return;
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/20] drm/gem: WARN about unbalanced handle refcounts
Trying to drop a reference we don't have is a pretty serious bug.
Trying to paper over it is an even worse offense.
So scream into dmesg with a big WARN in case that ever happens.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/drm_gem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index b1d9e25..9118f5f 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -238,7 +238,7 @@ static void drm_gem_object_handle_free(struct
drm_gem_object *obj)
void
drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj)
{
- if (atomic_read(obj-handle_count) == 0)
+ if (WARN_ON(atomic_read(obj-handle_count) == 0))
return;
/*
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/20] drm/gem: fix up flink name create race
This is the 2nd attempt, I've always been a bit dissatisified with the
tricky nature of the first one:
http://lists.freedesktop.org/archives/dri-devel/2012-July/025451.html
The issue is that the flink ioctl can race with calling gem_close on
the last gem handle. In that case we'll end up with a zero handle
count, but an flink name (and it's corresponding reference). Which
results in a neat space leak.
In my first attempt I've solved this by rechecking the handle count.
But fundamentally the issue is that -handle_count isn't your usual
refcount - it can be resurrected from 0 among other things.
For those special beasts atomic_t often suggest way more ordering that
it actually guarantees. To prevent being tricked by those hairy
semantics take the easy way out and simply protect the handle with the
existing dev-object_name_lock.
With that change implemented it's dead easy to fix the flink vs. gem
close reace: When we try to create the name we simply have to check
whether there's still officially a gem handle around and if not refuse
to create the flink name. Since the handle count decrement and flink
name destruction is now also protected by that lock the reace is gone
and we can't ever leak the flink reference again.
Outside of the drm core only the exynos driver looks at the handle
count, and tbh I have no idea why (it's just for debug dmesg output
luckily).
I've considered inlining the drm_gem_object_handle_free, but I plan to
add more name-like things (like the exported dma_buf) to this scheme,
so it's clearer to leave the handle freeing in its own function.
This is exercised by the new gem_flink_race i-g-t testcase, which on
my snb leaks gem objects at a rate of roughly 1k objects/s.
v2: Fix up the error path handling in handle_create and make it more
robust by simply calling object_handle_unreference.
v3: Fix up the handle_unreference logic bug - atomic_dec_and_test
retursn 1 for 0. Oops.
v4: Squash in inlining of drm_gem_object_handle_reference as suggested
by Dave Airlie and add a note that we now have a testcase.
Cc: Dave Airlie airl...@gmail.com
Cc: Inki Dae inki@samsung.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/drm_gem.c | 31 ---
drivers/gpu/drm/drm_info.c | 2 +-
drivers/gpu/drm/exynos/exynos_drm_gem.c | 2 +-
include/drm/drmP.h | 19 ++-
4 files changed, 32 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 9118f5f..c972a8f 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -154,7 +154,7 @@ void drm_gem_private_object_init(struct drm_device *dev,
obj-filp = NULL;
kref_init(obj-refcount);
- atomic_set(obj-handle_count, 0);
+ obj-handle_count = 0;
obj-size = size;
}
EXPORT_SYMBOL(drm_gem_private_object_init);
@@ -218,11 +218,9 @@ static void drm_gem_object_handle_free(struct
drm_gem_object *obj)
struct drm_device *dev = obj-dev;
/* Remove any name for this object */
- spin_lock(dev-object_name_lock);
if (obj-name) {
idr_remove(dev-object_name_idr, obj-name);
obj-name = 0;
- spin_unlock(dev-object_name_lock);
/*
* The object name held a reference to this object, drop
* that now.
@@ -230,15 +228,13 @@ static void drm_gem_object_handle_free(struct
drm_gem_object *obj)
* This cannot be the last reference, since the handle holds one
too.
*/
kref_put(obj-refcount, drm_gem_object_ref_bug);
- } else
- spin_unlock(dev-object_name_lock);
-
+ }
}
void
drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj)
{
- if (WARN_ON(atomic_read(obj-handle_count) == 0))
+ if (WARN_ON(obj-handle_count == 0))
return;
/*
@@ -247,8 +243,11 @@ drm_gem_object_handle_unreference_unlocked(struct
drm_gem_object *obj)
* checked for a name
*/
- if (atomic_dec_and_test(obj-handle_count))
+ spin_lock(obj-dev-object_name_lock);
+ if (--obj-handle_count == 0)
drm_gem_object_handle_free(obj);
+ spin_unlock(obj-dev-object_name_lock);
+
drm_gem_object_unreference_unlocked(obj);
}
@@ -326,17 +325,21 @@ drm_gem_handle_create(struct drm_file *file_priv,
* allocation under our spinlock.
*/
idr_preload(GFP_KERNEL);
+ spin_lock(dev-object_name_lock);
spin_lock(file_priv-table_lock);
ret = idr_alloc(file_priv-object_idr, obj, 1, 0, GFP_NOWAIT);
-
+ drm_gem_object_reference(obj);
+ obj-handle_count++;
spin_unlock(file_priv-table_lock);
+ spin_unlock(dev-object_name_lock);
idr_preload_end();
- if (ret 0)
+ if (ret 0) {
+
