Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback
On Mon, 24 Aug 2015 14:53:19 +0200, Ville Syrjälä wrote: On Mon, Aug 24, 2015 at 02:38:14AM +, Yang, Libin wrote: -Original Message- From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] Sent: Friday, August 21, 2015 11:14 PM To: Yang, Libin Cc: alsa-de...@alsa-project.org; ti...@suse.de; intel- g...@lists.freedesktop.org; daniel.vet...@ffwll.ch; jani.nik...@linux.intel.com Subject: Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback On Tue, Aug 18, 2015 at 02:51:52PM +0800, libin.y...@intel.com wrote: From: Libin Yang libin.y...@intel.com HDMI audio may not work at some frequencies with the HW provided N/CTS. This patch sets the proper N value for the given audio sample rate at the impacted frequencies. At other frequencies, it will use the N/CTS value which HW provides. Signed-off-by: Libin Yang libin.y...@intel.com --- drivers/gpu/drm/i915/intel_audio.c | 119 + 1 file changed, 119 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index dc32cf4..96b97be 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -68,6 +68,31 @@ static const struct { { 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 }, }; +/* HDMI N/CTS table */ +#define TMDS_297M 297000 +#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001) I don't really like the defines. I agree it's not a good name. Do you have some suggestions? I'd probably just have used raw numbers myself. It's quite unsafe. You can easily overlook, e.g. 29700 instead of 297000. Takashi ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Support for the clflush of pre-populated pages
From: Ankitprasad Sharma ankitprasad.r.sha...@intel.com This patch provides a support for the User to immediately flush out the cachelines for the pre-populated pages of an object, at the time of its creation. This will not lead to any redundancy and would further reduce the time for which the 'struct_mutex' is kept locked in execbuffer path, as cache flush of the newly allocated pages is anyways done when the object is submitted to GPU. Signed-off-by: Ankitprasad Sharma ankitprasad.r.sha...@intel.com --- drivers/gpu/drm/i915/i915_dma.c | 2 +- drivers/gpu/drm/i915/i915_gem.c | 3 +++ include/uapi/drm/i915_drm.h | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 955aa16..eb0b31d 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -171,7 +171,7 @@ static int i915_getparam(struct drm_device *dev, void *data, value = HAS_RESOURCE_STREAMER(dev); break; case I915_PARAM_CREATE_VERSION: - value = 3; + value = 4; break; default: DRM_DEBUG(Unknown parameter %d\n, param-param); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 3904feb..dc3435f 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -422,6 +422,9 @@ i915_gem_create(struct drm_file *file, if (ret) return ret; + if (flags I915_CREATE_FLUSH) + i915_gem_object_flush_cpu_write_domain(obj); + mutex_lock(dev-struct_mutex); list_add_tail(obj-global_list, dev_priv-mm.unbound_list); mutex_unlock(dev-struct_mutex); diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 26ea715..547305a 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -469,7 +469,8 @@ struct drm_i915_gem_create { __u32 flags; #define I915_CREATE_PLACEMENT_STOLEN (10) /* Cannot use CPU mmaps */ #define I915_CREATE_POPULATE (11) /* Pre-populate object pages */ -#define __I915_CREATE_UNKNOWN_FLAGS-(I915_CREATE_POPULATE 1) +#define I915_CREATE_FLUSH (12) /* Clflush prepopulated pages */ +#define __I915_CREATE_UNKNOWN_FLAGS-(I915_CREATE_FLUSH 1) }; struct drm_i915_gem_pread { -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: Support for pre-populating the object with system pages
From: Ankitprasad Sharma ankitprasad.r.sha...@intel.com This patch provides support for the User to populate the object with system pages at its creation time. Since this can be safely performed without holding the 'struct_mutex', it would help to reduce the time 'struct_mutex' is kept locked especially during the exec-buffer path, where it is generally held for the longest time. Signed-off-by: Ankitprasad Sharma ankitprasad.r.sha...@intel.com --- drivers/gpu/drm/i915/i915_dma.c | 2 +- drivers/gpu/drm/i915/i915_gem.c | 51 +++-- include/uapi/drm/i915_drm.h | 11 - 3 files changed, 45 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 8319e07..955aa16 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -171,7 +171,7 @@ static int i915_getparam(struct drm_device *dev, void *data, value = HAS_RESOURCE_STREAMER(dev); break; case I915_PARAM_CREATE_VERSION: - value = 2; + value = 3; break; default: DRM_DEBUG(Unknown parameter %d\n, param-param); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c44bd05..3904feb 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -46,6 +46,7 @@ static void i915_gem_object_retire__write(struct drm_i915_gem_object *obj); static void i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring); +static int __i915_gem_object_get_pages(struct drm_i915_gem_object *obj); static bool cpu_cache_is_coherent(struct drm_device *dev, enum i915_cache_level level) @@ -414,6 +415,18 @@ i915_gem_create(struct drm_file *file, if (obj == NULL) return -ENOMEM; + if (flags I915_CREATE_POPULATE) { + struct drm_i915_private *dev_priv = dev-dev_private; + + ret = __i915_gem_object_get_pages(obj); + if (ret) + return ret; + + mutex_lock(dev-struct_mutex); + list_add_tail(obj-global_list, dev_priv-mm.unbound_list); + mutex_unlock(dev-struct_mutex); + } + ret = drm_gem_handle_create(file, obj-base, handle); /* drop reference from allocate - handle holds it now */ drm_gem_object_unreference_unlocked(obj-base); @@ -2328,6 +2341,31 @@ err_pages: return ret; } +static int +__i915_gem_object_get_pages(struct drm_i915_gem_object *obj) +{ + const struct drm_i915_gem_object_ops *ops = obj-ops; + int ret; + + WARN_ON(obj-pages); + + if (obj-madv != I915_MADV_WILLNEED) { + DRM_DEBUG(Attempting to obtain a purgeable object\n); + return -EFAULT; + } + + BUG_ON(obj-pages_pin_count); + + ret = ops-get_pages(obj); + if (ret) + return ret; + + obj-get_page.sg = obj-pages-sgl; + obj-get_page.last = 0; + + return 0; +} + /* Ensure that the associated pages are gathered from the backing storage * and pinned into our object. i915_gem_object_get_pages() may be called * multiple times before they are released by a single call to @@ -2339,28 +2377,17 @@ int i915_gem_object_get_pages(struct drm_i915_gem_object *obj) { struct drm_i915_private *dev_priv = obj-base.dev-dev_private; - const struct drm_i915_gem_object_ops *ops = obj-ops; int ret; if (obj-pages) return 0; - if (obj-madv != I915_MADV_WILLNEED) { - DRM_DEBUG(Attempting to obtain a purgeable object\n); - return -EFAULT; - } - - BUG_ON(obj-pages_pin_count); - - ret = ops-get_pages(obj); + ret = __i915_gem_object_get_pages(obj); if (ret) return ret; list_add_tail(obj-global_list, dev_priv-mm.unbound_list); - obj-get_page.sg = obj-pages-sgl; - obj-get_page.last = 0; - return 0; } diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index f71f75c..26ea715 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -457,20 +457,19 @@ struct drm_i915_gem_create { __u32 handle; __u32 pad; /** -* Requested flags (currently used for placement -* (which memory domain)) +* Requested flags * * You can request that the object be created from special memory * rather than regular system pages using this parameter. Such * irregular objects may have certain restrictions (such as CPU * access to a stolen object is verboten). -* -* This can be used in the future for other purposes too -* e.g. specifying tiling/caching/madvise +* Also using this parameter object can be pre-populated with system +*
[Intel-gfx] [PATCH 0/3] Reduce the time for which 'struct_mutex' is held
From: Ankitprasad Sharma ankitprasad.r.sha...@intel.com We are trying to reduce the time for which the global 'struct_mutex' is locked. Execbuffer ioctl is one place where it is generally held for the longest time. And sometimes because of this occasional glitches/flickers are observed in 60 fps playback (due to miss of V-blank intervals) as page flip calls gets blocked/delayed because the 'struct_mutex' is already locked. For this, we have exposed two new flags in GEM_CREATE ioctl, to pre-populate the object with system memory pages and also do an immediate clflush for the new pages. The third patch too tries to reduce the 'struct_mutex' lock time by moving only those objects to CPU domain in put_pages(), that can either be used in the future or had a CPU mapping. This series is based on an earlier series of Stolen Memory patches, extending the GEM_CREATE ioctl further http://lists.freedesktop.org/archives/intel-gfx/2015-July/072199.html Ankitprasad Sharma (2): drm/i915: Support for pre-populating the object with system pages drm/i915: Support for the clflush of pre-populated pages Chris Wilson (1): drm/i915: Only move to the CPU write domain if keeping the GTT pages drivers/gpu/drm/i915/i915_dma.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 5 ++ drivers/gpu/drm/i915/i915_gem.c | 116 ++-- include/uapi/drm/i915_drm.h | 12 ++--- 4 files changed, 101 insertions(+), 34 deletions(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Only move to the CPU write domain if keeping the GTT pages
From: Chris Wilson ch...@chris-wilson.co.uk We have for a long time been ultra-paranoid about the situation whereby we hand back pages to the system that have been written to by the GPU and potentially simultaneously by the user through a CPU mmapping. We can relax this restriction when we know that the cache domain tracking is true and there can be no stale cacheline invalidatations. This is true if the object has never been CPU mmaped as all internal accesses (i.e. kmap/iomap) are carefully flushed. For a CPU mmaping, one would expect that the invalid cache lines are resolved on PTE/TLB shootdown during munmap(), so the only situation we need to be paranoid about is when such a CPU mmaping exists at the time of put_pages. Given that we need to treat put_pages carefully as we may return live data to the system that we want to use again in the future (i.e. I915_MADV_WILLNEED pages) we can simply treat a live CPU mmaping as a special case of WILLNEED (which it is!). Any I915_MADV_DONTNEED pages and their mmapings are shot down immediately following put_pages. v2: Add a new flag to check if ever a cached CPU mapping was created for the object. This is needed as we have verified that the CPU cachelines are not invalidated upon munmap(). So to ensure correctness, object still needs to be moved to CPU write domain in put_pages(), even if there are no live CPU mappings for I915_MADV_DONTNEED pages. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Ankitprasad Sharma ankitprasad.r.sha...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 5 drivers/gpu/drm/i915/i915_gem.c | 62 +++-- 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8db905a..bb85401 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2096,6 +2096,11 @@ struct drm_i915_gem_object { unsigned int frontbuffer_bits:INTEL_FRONTBUFFER_BITS; + /* +* Whether the object was ever mapped with cached CPU mapping +*/ + unsigned int has_stale_cachelines:1; + unsigned int pin_display; struct sg_table *pages; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index dc3435f..1c965ef 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1741,7 +1741,17 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data, else addr = -ENOMEM; up_write(mm-mmap_sem); + } else { + mutex_lock(dev-struct_mutex); + /* +* the cached mapping could lead to stale cachelines, so an +* invalidation is needed for the object pages, when they are +* released back to kernel +*/ + (to_intel_bo(obj))-has_stale_cachelines = 1; + mutex_unlock(dev-struct_mutex); } + drm_gem_object_unreference_unlocked(obj); if (IS_ERR((void *)addr)) return addr; @@ -2158,24 +2168,46 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object *obj) BUG_ON(obj-madv == __I915_MADV_PURGED); - ret = i915_gem_object_set_to_cpu_domain(obj, true); - if (ret) { - /* In the event of a disaster, abandon all caches and -* hope for the best. -*/ - WARN_ON(ret != -EIO); - i915_gem_clflush_object(obj, true); - obj-base.read_domains = obj-base.write_domain = I915_GEM_DOMAIN_CPU; - } - - i915_gem_gtt_finish_object(obj); - - if (i915_gem_object_needs_bit17_swizzle(obj)) - i915_gem_object_save_bit_17_swizzle(obj); + /* If we need to access the data in the future, we need to +* be sure that the contents of the object is coherent with +* the CPU prior to releasing the pages back to the system. +* Once we unpin them, the mm is free to move them to different +* zones or even swap them out to disk - all without our +* intervention. (Though we could track such operations with our +* own gemfs, if we ever write one.) As such if we want to keep +* the data, set it to the CPU domain now just in case someone +* else touches it. +* +* For a long time we have been paranoid about handing back +* pages to the system with stale cacheline invalidation. For +* all internal use (kmap/iomap), we know that the domain tracking is +* accurate. However, the userspace API is lax and the user can CPU +* mmap the object and invalidate cachelines without our accurate +* tracking. We have been paranoid to be sure that we always flushed +* the cachelines when we stopped using the pages. For which we +* maintain a flag for each object that has been CPU mmapped,
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Support for pre-populating the object with system pages
On Mon, Aug 24, 2015 at 05:28:14PM +0530, ankitprasad.r.sha...@intel.com wrote: +static int +__i915_gem_object_get_pages(struct drm_i915_gem_object *obj) +{ + const struct drm_i915_gem_object_ops *ops = obj-ops; + int ret; + + WARN_ON(obj-pages); + + if (obj-madv != I915_MADV_WILLNEED) { + DRM_DEBUG(Attempting to obtain a purgeable object\n); + return -EFAULT; + } + + BUG_ON(obj-pages_pin_count); Put the parameter checking into i915_gem_object_get_pages(). The __i915 version is only allowed from strict contexts and we can place the burden of being correct on the caller. + ret = ops-get_pages(obj); + if (ret) + return ret; + + obj-get_page.sg = obj-pages-sgl; + obj-get_page.last = 0; + + return 0; +} + /* Ensure that the associated pages are gathered from the backing storage * and pinned into our object. i915_gem_object_get_pages() may be called * multiple times before they are released by a single call to @@ -2339,28 +2377,17 @@ int i915_gem_object_get_pages(struct drm_i915_gem_object *obj) { struct drm_i915_private *dev_priv = obj-base.dev-dev_private; - const struct drm_i915_gem_object_ops *ops = obj-ops; int ret; if (obj-pages) return 0; - if (obj-madv != I915_MADV_WILLNEED) { - DRM_DEBUG(Attempting to obtain a purgeable object\n); - return -EFAULT; - } - - BUG_ON(obj-pages_pin_count); - - ret = ops-get_pages(obj); + ret = __i915_gem_object_get_pages(obj); if (ret) return ret; list_add_tail(obj-global_list, dev_priv-mm.unbound_list); I am tempted to say this should be in a new __i915_gem_object_get_pages__tail_locked() so that we don't have to hunt down users if we ever need to modify the global lists. -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 v4 4/5] drm: Add decoding of i915 ioctls
First batch of i915 drm ioctls * Makefile.am: Add compilation of drm_i915.c. * defs.h: Add extern i915 declarations * drm.c (drm_ioctl): Dispatch i915 ioctls. * drm_i915.c: New file. * xlat/drm_i915_getparams.in: New file. * xlat/drm_i915_setparams.in: New file. * xlat/drm_i915_ioctls.in: New file. Signed-off-by: Patrik Jakobsson patrik.jakobs...@linux.intel.com --- Makefile.am| 1 + defs.h | 2 + drm.c | 15 +- drm_i915.c | 342 + xlat/drm_i915_getparams.in | 28 xlat/drm_i915_ioctls.in| 51 +++ xlat/drm_i915_setparams.in | 4 + 7 files changed, 442 insertions(+), 1 deletion(-) create mode 100644 drm_i915.c create mode 100644 xlat/drm_i915_getparams.in create mode 100644 xlat/drm_i915_ioctls.in create mode 100644 xlat/drm_i915_setparams.in diff --git a/Makefile.am b/Makefile.am index c7c6080..0af46f6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -43,6 +43,7 @@ strace_SOURCES = \ desc.c \ dirent.c\ drm.c \ + drm_i915.c \ epoll.c \ evdev.c \ eventfd.c \ diff --git a/defs.h b/defs.h index dd3c720..1d54295 100644 --- a/defs.h +++ b/defs.h @@ -614,6 +614,8 @@ extern void print_seccomp_filter(struct tcb *tcp, unsigned long); extern int block_ioctl(struct tcb *, const unsigned int, long); #if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H) extern int drm_ioctl(struct tcb *, const unsigned int, long); +extern int drm_i915_ioctl(struct tcb *tcp, const unsigned int, long); +extern int drm_i915_decode_number(struct tcb *, unsigned int); #endif extern int evdev_ioctl(struct tcb *, const unsigned int, long); extern int loop_ioctl(struct tcb *, const unsigned int, long); diff --git a/drm.c b/drm.c index 0829803..e220309 100644 --- a/drm.c +++ b/drm.c @@ -96,12 +96,25 @@ int drm_is_driver(struct tcb *tcp, const char *name) int drm_decode_number(struct tcb *tcp, unsigned int code) { + if (drm_is_priv(tcp-u_arg[1])) { + if (verbose(tcp) drm_is_driver(tcp, i915)) + return drm_i915_decode_number(tcp, code); + } + return 0; } int drm_ioctl(struct tcb *tcp, const unsigned int code, long arg) { - return 0; + int ret = 0; + + /* Check for device specific ioctls */ + if (drm_is_priv(tcp-u_arg[1])) { + if (verbose(tcp) drm_is_driver(tcp, i915)) + ret = drm_i915_ioctl(tcp, code, arg); + } + + return ret; } #endif /* HAVE_DRM_H || HAVE_DRM_DRM_H */ diff --git a/drm_i915.c b/drm_i915.c new file mode 100644 index 000..9af3916 --- /dev/null +++ b/drm_i915.c @@ -0,0 +1,342 @@ +/* + * Copyright (c) 2015 Intel Corporation + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + *notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + *notice, this list of conditions and the following disclaimer in the + *documentation and/or other materials provided with the distribution. + * 3. The name of the author may not be used to endorse or promote products + *derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * Authors: + *Patrik Jakobsson patrik.jakobs...@linux.intel.com + */ + +#include defs.h + +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H) + +#ifdef HAVE_DRM_H +#include drm.h +#include i915_drm.h +#else +#include drm/drm.h +#include drm/i915_drm.h +#endif + +#include xlat/drm_i915_ioctls.h +#include xlat/drm_i915_getparams.h +#include xlat/drm_i915_setparams.h + +static int i915_getparam(struct tcb *tcp, const unsigned int code, long arg) +{ + struct drm_i915_getparam param; + int value; + + if (umove(tcp, arg, param)) + return RVAL_DECODED; + + if (entering(tcp)) { + tprints(, {param=); +
[Intel-gfx] [PATCH v4 1/5] drm: Add config for detecting libdrm
Use pkg-config to try to find libdrm headers. If that fails look for the kernel headers. If no headers are found, drm support will not be compiled. * configure.ac: Use pkg-config to find libdrm Signed-off-by: Patrik Jakobsson patrik.jakobs...@linux.intel.com --- configure.ac | 5 + 1 file changed, 5 insertions(+) diff --git a/configure.ac b/configure.ac index 0819f83..ef43278 100644 --- a/configure.ac +++ b/configure.ac @@ -870,6 +870,11 @@ fi AM_CONDITIONAL([USE_LIBUNWIND], [test x$use_libunwind = xyes]) AC_MSG_RESULT([$use_libunwind]) +PKG_CHECK_MODULES([LIBDRM], [libdrm], + [CPPFLAGS=$CPPFLAGS $LIBDRM_CFLAGS + AC_CHECK_HEADERS([drm.h i915_drm.h])], + [AC_CHECK_HEADERS([drm/drm.h drm/i915_drm.h])]) + if test $arch = mips test $no_create != yes; then mkdir -p linux/mips if $srcdir/linux/mips/genstub.sh linux/mips; then -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 5/5] drm: Add decoding of DRM and KMS ioctls
First batch of drm / kms ioctls. * drm.c (drm_version, drm_get_unique, drm_get_magic, drm_wait_vblank, drm_mode_get_resources, drm_mode_print_modeinfo, drm_mode_get_crtc, drm_mode_set_crtc, drm_mode_cursor, drm_mode_cursor2, drm_mode_get_gamma, drm_mode_set_gamma, drm_mode_get_encoder, drm_mode_get_connector, drm_mode_get_property, drm_mode_set_property, drm_mode_get_prop_blob, drm_mode_add_fb drm_mode_get_fb, drm_mode_rm_fb, drm_mode_page_flip, drm_mode_dirty_fb, drm_mode_create_dumb, drm_mode_map_dumb, drm_mode_destroy_dumb, drm_gem_close): New function (drm_ioctl): Dispatch drm ioctls Signed-off-by: Patrik Jakobsson patrik.jakobs...@linux.intel.com --- drm.c | 543 ++ 1 file changed, 543 insertions(+) diff --git a/drm.c b/drm.c index e220309..06f2265 100644 --- a/drm.c +++ b/drm.c @@ -104,6 +104,471 @@ int drm_decode_number(struct tcb *tcp, unsigned int code) return 0; } +static int drm_version(struct tcb *tcp, const unsigned int code, long arg) +{ + struct drm_version ver; + + if (exiting(tcp)) { + if (umove(tcp, arg, ver)) + return RVAL_DECODED; + + tprintf(, {version_major=%d, version_minor=%d, version_patchlevel=%d, + name_len=%lu, name=, ver.version_major, + ver.version_minor, ver.version_patchlevel, + ver.name_len); + printstr(tcp, (long)ver.name, ver.name_len); + tprintf(, date_len=%lu, date=, ver.date_len); + printstr(tcp, (long)ver.date, ver.date_len); + tprintf(, desc_len=%lu, desc=, ver.desc_len); + printstr(tcp, (long)ver.desc, ver.desc_len); + tprints(}); + } + + return RVAL_DECODED | 1; +} + +static int drm_get_unique(struct tcb *tcp, const unsigned int code, long arg) +{ + struct drm_unique unique; + + if (exiting(tcp)) { + if (umove(tcp, arg, unique)) + return RVAL_DECODED; + + tprintf(, {unique_len=%lu, unique=, unique.unique_len); + printstr(tcp, (long)unique.unique, unique.unique_len); + tprints(}); + } + + return RVAL_DECODED | 1; +} + +static int drm_get_magic(struct tcb *tcp, const unsigned int code, long arg) +{ + struct drm_auth auth; + + if (exiting(tcp)) { + if (umove(tcp, arg, auth)) + return RVAL_DECODED; + + tprintf(, {magic=%u}, auth.magic); + } + + return RVAL_DECODED | 1; +} + +static int drm_wait_vblank(struct tcb *tcp, const unsigned int code, long arg) +{ + union drm_wait_vblank vblank; + + if (umove(tcp, arg, vblank)) + return RVAL_DECODED; + + if (entering(tcp)) { + tprintf(, {request={type=%u, sequence=%u, signal=%lu}, + vblank.request.type, vblank.request.sequence, + vblank.request.signal); + } else if (exiting(tcp)) { + tprintf(, reply={type=%u, sequence=%u, tval_sec=%ld, tval_usec=%ld}}, + vblank.reply.type, vblank.reply.sequence, + vblank.reply.tval_sec, vblank.reply.tval_usec); + } + + return RVAL_DECODED | 1; +} + +static int drm_mode_get_resources(struct tcb *tcp, const unsigned int code, long arg) +{ + struct drm_mode_card_res res; + + if (exiting(tcp)) { + if (umove(tcp, arg, res)) + return RVAL_DECODED; + + tprintf(, {fb_id_ptr=%p, crtc_id_ptr=%p, connector_id_ptr=%p, + encoder_id_ptr=%p, count_fbs=%u, count_crtcs=%u, + count_connectors=%u, count_encoders=%u, min_width=%u, + max_width=%u, min_height=%u, max_height=%u}, + (void *)res.fb_id_ptr, (void *)res.crtc_id_ptr, + (void *)res.connector_id_ptr, (void *)res.encoder_id_ptr, + res.count_fbs, res.count_crtcs, res.count_connectors, + res.count_encoders, res.min_width, res.max_width, + res.min_height, res.max_height); + } + + return RVAL_DECODED | 1; +} + +static void drm_mode_print_modeinfo(struct drm_mode_modeinfo *info) +{ + tprintf(clock=%u, hdisplay=%hu, hsync_start=%hu, hsync_end=%hu, + htotal=%hu, hskew=%hu, vdisplay=%hu, vsync_start=%hu, + vsync_end=%hu, vtotal=%hu, vscan=%hu, vrefresh=%u, + flags=0x%x, type=%u, name=%s, info-clock, info-hdisplay, + info-hsync_start, info-hsync_end, info-htotal, info-hskew, + info-vdisplay, info-vsync_start, info-vsync_end, + info-vtotal, info-vscan, info-vrefresh, info-flags, + info-type, info-name); +} + +static int drm_mode_get_crtc(struct tcb *tcp, const unsigned int
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Always enable execlists on BDW for vgpu
On pe, 2015-08-21 at 10:24 +0800, Zhiyuan Lv wrote: Hi Joonas, Thanks for the review! And my reply inline. Regards, -Zhiyuan On Thu, Aug 20, 2015 at 02:23:11PM +0300, Joonas Lahtinen wrote: Hi, On to, 2015-08-20 at 17:40 +0800, Zhiyuan Lv wrote: On Thu, Aug 20, 2015 at 10:22:37AM +0100, Chris Wilson wrote: On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote: Hi Chris, On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote: On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote: Broadwell hardware supports both ring buffer mode and execlist mode. When i915 runs inside a VM with Intel GVT-g, we allow execlist mode only. The reason is that GVT-g does not support the dynamic mode switch between ring buffer mode and execlist mode when running multiple virtual machines. Consider that ring buffer mode is legacy mode, it makes sense to drop it inside virtual machines. If that is the case, you should query the host as to what mode it is running. Thanks for the reply! You mean we query the host mode, then tell the guest driver inside VM, so that it could use the same mode as host right? That might be a little complicated, and the only benefit is to support legacy ring buffer mode ... The only benefit being that the guest works no matter what the host does? Supporting ring buffer mode may need more work in GVT-g. When we started to enable BDW support, ring buffer mode used to work but was not well tested. And the inter-VM switch (all in ringbuffer mode) may be tricker comparing with driver context switch with MI_SET_CONTEXT, because we need to switch ring buffer whereas driver does not. Regarding this, the EXECLIST mode looks cleaner. In order to support that, we may have to: 1, change more LRI commands to MMIO in current driver; 2, more testing/debugging of inter-VM context switch flow. Based on that, I think we should really make statement that ring buffer mode is not supported by GVT-g on BDW :-) I think just move the vpgu test even before test for GEN9 just making it return true on intel_vgpu_active(dev) and add a comment that currently vGPU only supports execlist command submission, and then add Will do. Thanks! an error early in the init in some appropriate spot if intel_vgpu_active is true but logical ring context are not available (which would practically mean GEN 8). Does that sound OK? Ring buffer mode for Haswell with vgpu is still supported. So probably I have the check like below? Thanks! Ah sorry, I missed that. @@ -332,6 +332,12 @@ int i915_gem_context_init(struct drm_device *dev) if (WARN_ON(dev_priv-ring[RCS].default_context)) return 0; + if (intel_vgpu_active(dev)) { + if (WARN_ON(HAS_LOGICAL_RING_CONTEXTS(dev) + !i915.enable_execlist)) + return 0; + } + This looks fine to me. Maybe comment might be in place stating that support is not yet implemented, but could be. Regards, Joonas if (i915.enable_execlists) { /* NB: intentionally left blank. We will allocate our own * backing objects as we need them, thank you very much */ Regards, Joonas -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 3/5] drm: Add dispatcher and driver identification for DRM
* Makefile.am: Add compilation of drm.c. * defs.h: Add extern declaration of drm_ioctl when drm headers are found. * drm.c: New file. * ioctl.c (ioctl_decode): Dispatch drm ioctls when drm headers are found. Signed-off-by: Patrik Jakobsson patrik.jakobs...@linux.intel.com --- Makefile.am | 1 + defs.h | 3 ++ drm.c | 107 ioctl.c | 4 +++ 4 files changed, 115 insertions(+) create mode 100644 drm.c diff --git a/Makefile.am b/Makefile.am index f70f6d2..c7c6080 100644 --- a/Makefile.am +++ b/Makefile.am @@ -42,6 +42,7 @@ strace_SOURCES = \ count.c \ desc.c \ dirent.c\ + drm.c \ epoll.c \ evdev.c \ eventfd.c \ diff --git a/defs.h b/defs.h index bc3bd83..dd3c720 100644 --- a/defs.h +++ b/defs.h @@ -612,6 +612,9 @@ extern const char *sprint_open_modes(int); extern void print_seccomp_filter(struct tcb *tcp, unsigned long); extern int block_ioctl(struct tcb *, const unsigned int, long); +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H) +extern int drm_ioctl(struct tcb *, const unsigned int, long); +#endif extern int evdev_ioctl(struct tcb *, const unsigned int, long); extern int loop_ioctl(struct tcb *, const unsigned int, long); extern int mtd_ioctl(struct tcb *, const unsigned int, long); diff --git a/drm.c b/drm.c new file mode 100644 index 000..0829803 --- /dev/null +++ b/drm.c @@ -0,0 +1,107 @@ +/* + * Copyright (c) 2015 Intel Corporation + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + *notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + *notice, this list of conditions and the following disclaimer in the + *documentation and/or other materials provided with the distribution. + * 3. The name of the author may not be used to endorse or promote products + *derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * Authors: + *Patrik Jakobsson patrik.jakobs...@linux.intel.com + */ + +#include defs.h + +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H) + +#ifdef HAVE_DRM_H +#include drm.h +#else +#include drm/drm.h +#endif + +#include sys/param.h + +#define DRM_MAX_NAME_LEN 128 + +inline int drm_is_priv(const unsigned int num) +{ + return (_IOC_NR(num) = DRM_COMMAND_BASE + _IOC_NR(num) DRM_COMMAND_END); +} + +static char *drm_get_driver_name(struct tcb *tcp) +{ + char path[PATH_MAX]; + char link[PATH_MAX]; + int ret; + + if (getfdpath(tcp, tcp-u_arg[0], path, PATH_MAX - 1) 0) + return NULL; + + snprintf(link, PATH_MAX, /sys/class/drm/%s/device/driver, +basename(path)); + + ret = readlink(link, path, PATH_MAX - 1); + if (ret 0) + return NULL; + + path[ret] = '\0'; + return strdup(basename(path)); +} + +static void drm_free_priv(void *data) +{ + free(data); +} + +int drm_is_driver(struct tcb *tcp, const char *name) +{ + char *priv; + + /* +* If no private data is allocated we are detecting the driver name for +* the first time and must resolve it. +*/ + if (tcp-priv_data == NULL) { + tcp-priv_data = drm_get_driver_name(tcp); + if (tcp-priv_data == NULL) + return 0; + + tcp-free_priv_data = drm_free_priv; + } + + priv = tcp-priv_data; + + return strncmp(name, priv, DRM_MAX_NAME_LEN) == 0; +} + +int drm_decode_number(struct tcb *tcp, unsigned int code) +{ + return 0; +} + +int drm_ioctl(struct tcb *tcp, const unsigned int code, long arg) +{ + return 0; +} + +#endif /* HAVE_DRM_H || HAVE_DRM_DRM_H */ diff --git a/ioctl.c b/ioctl.c index 284828a..dfd35d8 100644 --- a/ioctl.c +++ b/ioctl.c @@ -248,6 +248,10 @@ ioctl_decode(struct tcb *tcp) case 0x22: return
[Intel-gfx] [PATCH v4 0/5] drm: Add decoding for DRM/KMS and i915 ioctls
This set of patches adds a dispatcher for handling DRM ioctls. The kernel headers for DRM might not be available on all distributions so we depend on libdrm for those. If libdrm is not available we fall back on the kernel headers. Since DRM drivers share the same range of private ioctl numbers I've added a function for detecting the driver based on it's name. Changes in v2: * Rebased to master * Added Changelog to commits * Keep strace_SOURCES list sorted * Removed unneeded includes * Reduced number of driver name checks by adding tcb private data * Use tprints() for regular strings * Reworked entering() / exiting() handling for all ioctls * Use printstr() to print strings in properly quoted form Changes in v3: * Moved all umove() into state checks for single state ioctls * Removed extra curly bracket * Moved param argument into entering() state in i915_setparam() * Don't return before private data is freed in drm_ioctl() Changes in v4: * Rebased to master * Rewrote commit messages to GNU changelog standard * Added private data support to struct tcb * Reworked drm driver identification * Reworked drm header detection * Use recently added return types for decode functions * Various small fixes Patrik Jakobsson (5): drm: Add config for detecting libdrm drm: Add private data field to trace control block drm: Add dispatcher and driver identification for DRM drm: Add decoding of i915 ioctls drm: Add decoding of DRM and KMS ioctls Makefile.am| 2 + configure.ac | 5 + defs.h | 11 + drm.c | 663 + drm_i915.c | 342 +++ ioctl.c| 4 + strace.c | 14 + syscall.c | 1 + xlat/drm_i915_getparams.in | 28 ++ xlat/drm_i915_ioctls.in| 51 xlat/drm_i915_setparams.in | 4 + 11 files changed, 1125 insertions(+) create mode 100644 drm.c create mode 100644 drm_i915.c create mode 100644 xlat/drm_i915_getparams.in create mode 100644 xlat/drm_i915_ioctls.in create mode 100644 xlat/drm_i915_setparams.in -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 2/5] drm: Add private data field to trace control block
We need to be able to store private data in the tcb across it's lifetime. To ensure proper destruction of the data a free_priv_data callback must be provided if an allocation is stored in priv_data. The callback is executed automatically when the life of the tcb ends. * defs.h: Add extern declaration of free_tcb_priv_data. (struct tcb): Add priv_data and free_priv_data. * strace.c (free_tcb_priv_data): New function (drop_tcb): Execute free_tcb_priv_data callback * syscall.c (trace_syscall_exiting): Execute free_tcb_priv_data callback Signed-off-by: Patrik Jakobsson patrik.jakobs...@linux.intel.com --- defs.h| 6 ++ strace.c | 14 ++ syscall.c | 1 + 3 files changed, 21 insertions(+) diff --git a/defs.h b/defs.h index 9059026..bc3bd83 100644 --- a/defs.h +++ b/defs.h @@ -266,6 +266,10 @@ struct tcb { int u_error;/* Error code */ long scno; /* System call number */ long u_arg[MAX_ARGS]; /* System call arguments */ + + void *priv_data;/* Private data for syscall decoding functions */ + void (*free_priv_data)(void *); /* Callback for freeing priv_data */ + #if defined(LINUX_MIPSN32) || defined(X32) long long ext_arg[MAX_ARGS]; long long u_lrval; /* long long return value */ @@ -470,6 +474,8 @@ extern void get_regs(pid_t pid); extern int get_scno(struct tcb *tcp); extern const char *syscall_name(long scno); +extern void free_tcb_priv_data(struct tcb *tcp); + extern int umoven(struct tcb *, long, unsigned int, void *); #define umove(pid, addr, objp) \ umoven((pid), (addr), sizeof(*(objp)), (void *) (objp)) diff --git a/strace.c b/strace.c index 9b93e79..7a71152 100644 --- a/strace.c +++ b/strace.c @@ -711,12 +711,26 @@ alloctcb(int pid) error_msg_and_die(bug in alloctcb); } +void +free_tcb_priv_data(struct tcb *tcp) +{ + if (tcp-priv_data) { + if (tcp-free_priv_data) { + tcp-free_priv_data(tcp-priv_data); + tcp-free_priv_data = NULL; + } + tcp-priv_data = NULL; + } +} + static void droptcb(struct tcb *tcp) { if (tcp-pid == 0) return; + free_tcb_priv_data(tcp); + #ifdef USE_LIBUNWIND if (stack_trace_enabled) { unwind_tcb_fin(tcp); diff --git a/syscall.c b/syscall.c index 396a7dd..39b973b 100644 --- a/syscall.c +++ b/syscall.c @@ -1099,6 +1099,7 @@ trace_syscall_exiting(struct tcb *tcp) #endif ret: + free_tcb_priv_data(tcp); tcp-flags = ~TCB_INSYSCALL; tcp-sys_func_rval = 0; return 0; -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Support for the clflush of pre-populated pages
On Mon, Aug 24, 2015 at 05:28:15PM +0530, ankitprasad.r.sha...@intel.com wrote: From: Ankitprasad Sharma ankitprasad.r.sha...@intel.com This patch provides a support for the User to immediately flush out the cachelines for the pre-populated pages of an object, at the time of its creation. This will not lead to any redundancy and would further reduce the time for which the 'struct_mutex' is kept locked in execbuffer path, as cache flush of the newly allocated pages is anyways done when the object is submitted to GPU. Signed-off-by: Ankitprasad Sharma ankitprasad.r.sha...@intel.com 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
Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback
On Mon, Aug 24, 2015 at 02:38:14AM +, Yang, Libin wrote: -Original Message- From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] Sent: Friday, August 21, 2015 11:14 PM To: Yang, Libin Cc: alsa-de...@alsa-project.org; ti...@suse.de; intel- g...@lists.freedesktop.org; daniel.vet...@ffwll.ch; jani.nik...@linux.intel.com Subject: Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback On Tue, Aug 18, 2015 at 02:51:52PM +0800, libin.y...@intel.com wrote: From: Libin Yang libin.y...@intel.com HDMI audio may not work at some frequencies with the HW provided N/CTS. This patch sets the proper N value for the given audio sample rate at the impacted frequencies. At other frequencies, it will use the N/CTS value which HW provides. Signed-off-by: Libin Yang libin.y...@intel.com --- drivers/gpu/drm/i915/intel_audio.c | 119 + 1 file changed, 119 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index dc32cf4..96b97be 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -68,6 +68,31 @@ static const struct { { 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 }, }; +/* HDMI N/CTS table */ +#define TMDS_297M 297000 +#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001) I don't really like the defines. I agree it's not a good name. Do you have some suggestions? I'd probably just have used raw numbers myself. +static const struct { + int sample_rate; + int clock; + int n; + int cts; +} aud_ncts[] = { + { 44100, TMDS_296M, 4459, 234375 }, + { 44100, TMDS_297M, 4704, 247500 }, + { 48000, TMDS_296M, 5824, 281250 }, + { 48000, TMDS_297M, 5120, 247500 }, + { 32000, TMDS_296M, 5824, 421875 }, + { 32000, TMDS_297M, 3072, 222750 }, + { 88200, TMDS_296M, 8918, 234375 }, + { 88200, TMDS_297M, 9408, 247500 }, + { 96000, TMDS_296M, 11648, 281250 }, + { 96000, TMDS_297M, 10240, 247500 }, + { 176400, TMDS_296M, 17836, 234375 }, + { 176400, TMDS_297M, 18816, 247500 }, + { 44100, TMDS_296M, 23296, 281250 }, + { 44100, TMDS_297M, 20480, 247500 }, +}; Last two should be 192 kHz. All the other values seem to match the spec. Oh, my typo. Thanks for pointing it out. These do assume 8bpc, but as the spec doesn't even specify N/CTS values for deep color modes @ 297MHz, so I suppose the expectations is that the max TMDS clock will never be so high as to allow them. If/when we start using manual programming for other TMDS clock rates we'll need to consider 12bpc as well. These values are recommended from HDMI spec. It's not related to the bpp. Am I wrong? I'm find the value from HDMISpecification 1.4: table 7-1, table 7-2 and table 7-3. There are separate tables for deep color modes in appendix D. Tables D-1 through D-3 are of interest to us since we can only do the 12bpc deep color mode. + /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */ static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode) { @@ -90,6 +115,31 @@ static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode) return hdmi_audio_clock[i].config; } +static int audio_config_get_n(struct drm_display_mode *mode, int rate) mode can be const. OK. I will change it. +{ + int i; + + for (i = 0; i ARRAY_SIZE(aud_ncts); i++) { + if ((rate == aud_ncts[i].sample_rate) + (mode-clock == aud_ncts[i].clock)) { + return aud_ncts[i].n; + } + } + return 0; +} + +/* check whether N/CTS/M need be set manually */ +static bool audio_rate_need_prog(struct intel_crtc *crtc, + struct drm_display_mode *mode) +{ + if (((mode-clock == TMDS_297M) || + (mode-clock == TMDS_296M)) + intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI)) + return true; + else + return false; +} + static bool intel_eld_uptodate(struct drm_connector *connector, int reg_eldv, uint32_t bits_eldv, int reg_elda, uint32_t bits_elda, @@ -514,12 +564,81 @@ static int i915_audio_component_get_cdclk_freq(struct device *dev) return ret; } +static int i915_audio_component_sync_audio_rate(struct device *dev, + int port, int rate) +{ + struct drm_i915_private *dev_priv = dev_to_i915(dev); + struct drm_device *drm_dev = dev_priv-dev; + struct intel_encoder *intel_encoder; + struct intel_digital_port *intel_dig_port; + struct intel_crtc *crtc; + struct drm_display_mode *mode; + enum pipe pipe = -1; + u32 tmp; + int n_low, n_up, n; + + /* 1. get the pipe
Re: [Intel-gfx] [PATCH 1/7] drm/i915: Remove duplicated dpcd write on hsw_psr_enable_sink.
Em Qui, 2015-08-20 às 17:55 -0700, Rodrigo Vivi escreveu: This is wrong since my commit (89251b17). The intention of that commit was to remove this one here that is also wrong anyway, but it was forgotten. You mentioned the current code is wrong, but it would be really nice if you could describe why it is wrong and what are the implications of it being wrong. Was this causing some specific problem or was it just found by code inspection? In case it was found just by code inspection, you could try to elaborate on the possible problems brought by the bad commit. A more elaborate description helps not only the reviewers, but also the possible backporters and even helps us judging whether it should be merged by Daniel or Jani. Anyway, as far as I see the patch looks correct, so if you could provide another paragraph for Daniel to amend on the commit message: Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_psr.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index a04b4dc..51f0514 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -170,9 +170,6 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp) aux_clock_divider = intel_dp -get_aux_clock_divider(intel_dp, 0); - drm_dp_dpcd_writeb(intel_dp-aux, DP_PSR_EN_CFG, -DP_PSR_ENABLE ~DP_PSR_MAIN_LINK_ACTIVE); - /* Enable AUX frame sync at sink */ if (dev_priv-psr.aux_frame_sync) drm_dp_dpcd_writeb(intel_dp-aux, ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Only move to the CPU write domain if keeping the GTT pages
On Mon, Aug 24, 2015 at 05:28:16PM +0530, ankitprasad.r.sha...@intel.com wrote: + /* + * the cached mapping could lead to stale cachelines, so an + * invalidation is needed for the object pages, when they are + * released back to kernel + */ Multiline comments should be full sentences (i.e. captilised and punctuated). + (to_intel_bo(obj))-has_stale_cachelines = 1; Was there a buy one, get one free other on brackets? Is there a bug in the to_intel_bo() macro? -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 V2] drm/i915/skl+: Add YUV pixel format in Capability list
On Fri, Jul 17, 2015 at 07:20:41PM +0530, Kumar, Mahesh wrote: GEN = 9 supports YUV format for all planes, but it's not exported in Capability list of primary plane. Add YUV formats in skl_primary_formats list. Don't rely on fb-bits_per_pixel as intel_framebuffer_init is not filling bits_per_pixel field of fb-struct for YUV pixel format. This leads to divide by zero error during watermark calculation. V2: Don't break NV12 case. Signed-off-by: Kumar, Mahesh mahesh1.ku...@intel.com Cc: Konduru, Chandra chandra.kond...@intel.com --- IGT changes made for testcase will be sent in separate patch. drivers/gpu/drm/i915/intel_display.c | 4 drivers/gpu/drm/i915/intel_pm.c | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index af0bcfe..d31704a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -72,6 +72,10 @@ static const uint32_t skl_primary_formats[] = { DRM_FORMAT_ABGR, DRM_FORMAT_XRGB2101010, DRM_FORMAT_XBGR2101010, + DRM_FORMAT_YUYV, + DRM_FORMAT_YVYU, + DRM_FORMAT_UYVY, + DRM_FORMAT_VYUY, }; /* Cursor formats */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 5eeddc9..5768f8c 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3164,7 +3164,8 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc, if (fb) { p-plane[0].enabled = true; p-plane[0].bytes_per_pixel = fb-pixel_format == DRM_FORMAT_NV12 ? - drm_format_plane_cpp(fb-pixel_format, 1) : fb-bits_per_pixel / 8; + drm_format_plane_cpp(fb-pixel_format, 1) : + drm_format_plane_cpp(fb-pixel_format, 0); Someone should really fix the SKL WM code to treat the Y plane as the normal case and CbCr as the special case. Currently we do just the opposite which leads to this kind of checks all over the place. p-plane[0].y_bytes_per_pixel = fb-pixel_format == DRM_FORMAT_NV12 ? drm_format_plane_cpp(fb-pixel_format, 0) : 0; p-plane[0].tiling = fb-modifier[0]; -- 1.9.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 i-g-t] kms_addfb_basic: Require fb modifiers for unused field tests
On 20 August 2015 at 15:43, Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com wrote: The drm core doesn't check unused fields of ADDFB2 for pre-FB_MODIFIERS userspace, so require that and use the local version of the defines. Signed-off-by: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com --- tests/kms_addfb_basic.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c index f10e12b..732d6dc 100644 --- a/tests/kms_addfb_basic.c +++ b/tests/kms_addfb_basic.c @@ -51,6 +51,7 @@ static void invalid_tests(int fd) f.height = 512; f.pixel_format = DRM_FORMAT_XRGB; f.pitches[0] = 512*4; + f.flags = LOCAL_DRM_MODE_FB_MODIFIERS; igt_fixture { gem_bo = gem_create(fd, 1024*1024*4); @@ -60,35 +61,43 @@ static void invalid_tests(int fd) f.handles[0] = gem_bo; - igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, f) == 0); + igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, f) == 0); Missing igt_require_fb_modifiers()? Maybe this needs to be placed in a separate subtest to prevent unrelated tests failing or skipping? igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, f.fb_id) == 0); f.fb_id = 0; } igt_subtest(unused-handle) { + igt_require_fb_modifiers(fd); + f.handles[1] = gem_bo_small; - igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, f) == -1 + igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, f) == -1 errno == EINVAL); f.handles[1] = 0; } igt_subtest(unused-pitches) { + igt_require_fb_modifiers(fd); + f.pitches[1] = 512; - igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, f) == -1 + igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, f) == -1 errno == EINVAL); f.pitches[1] = 0; } igt_subtest(unused-offsets) { + igt_require_fb_modifiers(fd); + f.offsets[1] = 512; - igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, f) == -1 + igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, f) == -1 errno == EINVAL); f.offsets[1] = 0; } igt_subtest(unused-modifier) { + igt_require_fb_modifiers(fd); + f.modifier[1] = LOCAL_I915_FORMAT_MOD_X_TILED; - igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, f) == -1 + igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, f) == -1 errno == EINVAL); f.modifier[1] = 0; } -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] doc: drm: Fix mis-spelling of i915_guc_submission includes
In commit d1675198e: drm/i915: Integrate GuC-based command submission the drm.tmpl include lines reference the intel_guc_submission.c but the patch adds the file i915_guc_submission.c. drm.tmpl fails to build with: docproc: .//drivers/gpu/drm/i915/intel_guc_submission.c: No such file or directory Change the file reference to the actual file. Signed-off-by: Graham Whaley graham.wha...@linux.intel.com --- This looks/feels like the right fix to me, but I am not intimate enough with the APIs to verify for sure - some confirmation would be good. Documentation/DocBook/drm.tmpl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index a01fca9..66bc646 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -4160,8 +4160,8 @@ int num_ioctls;/synopsis /sect2 sect2 titleGuC Client/title -!Pdrivers/gpu/drm/i915/intel_guc_submission.c GuC-based command submissison -!Idrivers/gpu/drm/i915/intel_guc_submission.c +!Pdrivers/gpu/drm/i915/i915_guc_submission.c GuC-based command submissison +!Idrivers/gpu/drm/i915/i915_guc_submission.c /sect2 /sect1 -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback
-Original Message- From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] Sent: Monday, August 24, 2015 8:53 PM To: Yang, Libin Cc: alsa-de...@alsa-project.org; ti...@suse.de; intel- g...@lists.freedesktop.org; daniel.vet...@ffwll.ch; jani.nik...@linux.intel.com Subject: Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback On Mon, Aug 24, 2015 at 02:38:14AM +, Yang, Libin wrote: -Original Message- From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] Sent: Friday, August 21, 2015 11:14 PM To: Yang, Libin Cc: alsa-de...@alsa-project.org; ti...@suse.de; intel- g...@lists.freedesktop.org; daniel.vet...@ffwll.ch; jani.nik...@linux.intel.com Subject: Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback On Tue, Aug 18, 2015 at 02:51:52PM +0800, libin.y...@intel.com wrote: From: Libin Yang libin.y...@intel.com HDMI audio may not work at some frequencies with the HW provided N/CTS. This patch sets the proper N value for the given audio sample rate at the impacted frequencies. At other frequencies, it will use the N/CTS value which HW provides. Signed-off-by: Libin Yang libin.y...@intel.com --- drivers/gpu/drm/i915/intel_audio.c | 119 + 1 file changed, 119 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index dc32cf4..96b97be 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -68,6 +68,31 @@ static const struct { { 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 }, }; +/* HDMI N/CTS table */ +#define TMDS_297M 297000 +#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001) I don't really like the defines. I agree it's not a good name. Do you have some suggestions? I'd probably just have used raw numbers myself. +static const struct { + int sample_rate; + int clock; + int n; + int cts; +} aud_ncts[] = { + { 44100, TMDS_296M, 4459, 234375 }, + { 44100, TMDS_297M, 4704, 247500 }, + { 48000, TMDS_296M, 5824, 281250 }, + { 48000, TMDS_297M, 5120, 247500 }, + { 32000, TMDS_296M, 5824, 421875 }, + { 32000, TMDS_297M, 3072, 222750 }, + { 88200, TMDS_296M, 8918, 234375 }, + { 88200, TMDS_297M, 9408, 247500 }, + { 96000, TMDS_296M, 11648, 281250 }, + { 96000, TMDS_297M, 10240, 247500 }, + { 176400, TMDS_296M, 17836, 234375 }, + { 176400, TMDS_297M, 18816, 247500 }, + { 44100, TMDS_296M, 23296, 281250 }, + { 44100, TMDS_297M, 20480, 247500 }, +}; Last two should be 192 kHz. All the other values seem to match the spec. Oh, my typo. Thanks for pointing it out. These do assume 8bpc, but as the spec doesn't even specify N/CTS values for deep color modes @ 297MHz, so I suppose the expectations is that the max TMDS clock will never be so high as to allow them. If/when we start using manual programming for other TMDS clock rates we'll need to consider 12bpc as well. These values are recommended from HDMI spec. It's not related to the bpp. Am I wrong? I'm find the value from HDMISpecification 1.4: table 7-1, table 7-2 and table 7-3. There are separate tables for deep color modes in appendix D. Tables D-1 through D-3 are of interest to us since we can only do the 12bpc deep color mode. Yes, we found the D tables in the spec before, and it seems a little Complicated. And the table 7-x seems to be more general. This is the reason we use table 7-x. OK. We will use D-1, D-2 and D-3 table for N/CTS. + /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */ static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode) { @@ -90,6 +115,31 @@ static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode) return hdmi_audio_clock[i].config; } +static int audio_config_get_n(struct drm_display_mode *mode, int rate) mode can be const. OK. I will change it. +{ + int i; + + for (i = 0; i ARRAY_SIZE(aud_ncts); i++) { + if ((rate == aud_ncts[i].sample_rate) + (mode-clock == aud_ncts[i].clock)) { + return aud_ncts[i].n; + } + } + return 0; +} + +/* check whether N/CTS/M need be set manually */ +static bool audio_rate_need_prog(struct intel_crtc *crtc, + struct drm_display_mode *mode) +{ + if (((mode-clock == TMDS_297M) || +(mode-clock == TMDS_296M)) + intel_pipe_has_type(crtc,
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Fix PSR disable sequence on core platforms.
Em Qui, 2015-08-20 às 17:55 -0700, Rodrigo Vivi escreveu: According to spec the disable sequence is: Driver will do the following on PSR Disable. 1. Disable PSR in PSR control register, SRD_CTL[bit 31]. 2. Poll on PSR idle 3. Wait for VBlank 4. Disable VSC DIP. Shouldn't this be done at intel_psr_exit() instead of intel_psr_disable()? In case it's yes (which is my guess), then the wait_for_vblank() is probably going to slow down everything, so we may need to use some sort of delayed work. In case it's no, then I don't think we need the wait_for_vblank() since the encoder disable only happens after the pipe disable. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_psr.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 51f0514..92e2b467 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -459,6 +459,10 @@ static void hsw_psr_disable(struct intel_dp *intel_dp) struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); struct drm_device *dev = intel_dig_port-base.base.dev; struct drm_i915_private *dev_priv = dev-dev_private; + struct drm_crtc *crtc = intel_dig_port-base.base.crtc; + enum pipe pipe = to_intel_crtc(crtc)-pipe; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc-config -cpu_transcoder); if (dev_priv-psr.active) { I915_WRITE(EDP_PSR_CTL(dev), @@ -469,6 +473,12 @@ static void hsw_psr_disable(struct intel_dp *intel_dp) EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10)) DRM_ERROR(Timed out waiting for PSR Idle State\n); + intel_wait_for_vblank(dev, pipe); + + I915_WRITE(ctl_reg, I915_READ(ctl_reg) + ~VIDEO_DIP_ENABLE_VSC_HSW); + POSTING_READ(ctl_reg); + dev_priv-psr.active = false; } else { WARN_ON(I915_READ(EDP_PSR_CTL(dev)) EDP_PSR_ENABLE); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: PSR: Let's rely more on frontbuffer tracking.
Em Qui, 2015-08-20 às 17:55 -0700, Rodrigo Vivi escreveu: Many reasons here: - Hardware tracking also has hidden corner cases Can you please elaborate more on that? I really really really really really think we should try as hard as possible to cook some IGT cases if something is affecting us :) - Frontbuffer tracking is mature and reliable now - Our sw exit by unseting bit 31 is really fast and reliable. But doesn't it trigger an automatic link retraining? Also frontbuffer tracking flush means invalidate and flush. I don't know what are the implications of this in the current context. So, let's rely more and do the proper meaning of flush for all cases without any workaround. I'm really in favor of the idea that if the HW can properly handle the flips, we should rely on it, since in a lot of modern desktop environments we basically do one flip per frame. Did we study how this patch affects the PSR residency on the different cases we care about? (yes, I know FBC is not relying on the HW for flips, but this is on the optimization TODO list after we finally merge the bug fixes) Due to the benefits of relying on the HW tracking, I think you'll have to bring some good arguments to sell this patch to me. But a Testcase: tag would totally do it :) Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_psr.c | 22 +++--- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 92e2b467..63bbab2 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -719,25 +719,9 @@ void intel_psr_flush(struct drm_device *dev, frontbuffer_bits = INTEL_FRONTBUFFER_ALL_MASK(pipe); dev_priv-psr.busy_frontbuffer_bits = ~frontbuffer_bits; - if (HAS_DDI(dev)) { - /* - * By definition every flush should mean invalidate + flush, - * however on core platforms let's minimize the - * disable/re-enable so we can avoid the invalidate when flip - * originated the flush. - */ - if (frontbuffer_bits origin != ORIGIN_FLIP) - intel_psr_exit(dev); - } else { - /* - * On Valleyview and Cherryview we don't use hardware tracking - * so any plane updates or cursor moves don't result in a PSR - * invalidating. Which means we need to manually fake this in - * software for all flushes. - */ - if (frontbuffer_bits) - intel_psr_exit(dev); - } + /* By definition flush = invalidate + flush */ + if (frontbuffer_bits) + intel_psr_exit(dev); if (!dev_priv-psr.active !dev_priv -psr.busy_frontbuffer_bits) schedule_delayed_work(dev_priv-psr.work, ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] scripts/kernel-doc: Improve Markdown results
On Fri, 2015-08-21 at 16:39 -0300, Danilo Cesar Lemes de Paula wrote: Using pandoc as the Markdown engine cause some minor side effects as pandoc includes main para tags for almost everything. Original Markdown support approach removes those main tags, but it caused some inconsistencies when that tag is not the main one, like: something../something para.../para As kernel-doc was already including a para tag, it causes the presence of double para tags (parapara), which is not supported by DocBook spec. Html target gets away with it, so it causes no harm, although other targets might not be so lucky (pdf as example). We're now delegating the inclusion of the main para tag to pandoc only, as it knows when it's necessary or not. That behavior causes a corner case, the only situation where we're certainly that para is not needed, which is the refpurpose content. For those cases, we're using a $output_markdown_nopara = 1 control var. Signed-off-by: Danilo Cesar Lemes de Paula Feel free to add my: Tested-by: Graham Whaley graham.wha...@linux.intel.com Graham danilo.ce...@collabora.co.uk Cc: Randy Dunlap rdun...@infradead.org Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Jonathan Corbet cor...@lwn.net Cc: Herbert Xu herb...@gondor.apana.org.au Cc: Stephan Mueller smuel...@chronox.de Cc: Michal Marek mma...@suse.cz Cc: linux-ker...@vger.kernel.org Cc: linux-...@vger.kernel.org Cc: intel-gfx intel-gfx@lists.freedesktop.org Cc: dri-devel dri-de...@lists.freedesktop.org Cc: Graham Whaley graham.wha...@linux.intel.com --- Thanks to Graham Whaley who helped me to debug this. scripts/kernel-doc | 48 ++-- 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/scripts/kernel-doc b/scripts/kernel-doc index 3850c1e..12a106c 100755 --- a/scripts/kernel-doc +++ b/scripts/kernel-doc @@ -288,6 +288,7 @@ my $use_markdown = 0; my $verbose = 0; my $output_mode = man; my $output_preformatted = 0; +my $output_markdown_nopara = 0; my $no_doc_sections = 0; my @highlights = @highlights_man; my $blankline = $blankline_man; @@ -529,8 +530,11 @@ sub markdown_to_docbook { close(CHLD_OUT); close(CHLD_ERR); - # pandoc insists in adding Main para/para, we should remove them. - $content =~ s:\A\s*para\s*\n(.*)\n/para\Z$:$1:egsm; + if ($output_markdown_nopara) { + # pandoc insists in adding Main para/para, sometimes we + # want to remove them. + $content =~ s:\A\s*para\s*\n(.*)\n/para\Z$:$1:egsm; + } return $content; } @@ -605,7 +609,7 @@ sub output_highlight { $line =~ s/^\s*//; } if ($line eq ){ - if (! $output_preformatted) { + if (! $output_preformatted ! $use_markdown) { print $lineprefix, local_unescape($blankline); } } else { @@ -1026,7 +1030,7 @@ sub output_section_xml(%) { # programlisting is already included by pandoc print programlisting\n unless $use_markdown; $output_preformatted = 1; - } else { + } elsif (! $use_markdown) { print para\n; } output_highlight($args{'sections'}{$section}); @@ -1034,7 +1038,7 @@ sub output_section_xml(%) { if ($section =~ m/EXAMPLE/i) { print /programlisting\n unless $use_markdown; print /informalexample\n; - } else { + } elsif (! $use_markdown) { print /para\n; } print /refsect1\n; @@ -1066,7 +1070,9 @@ sub output_function_xml(%) { print refname . $args{'function'} . /refname\n; print refpurpose\n; print ; +$output_markdown_nopara = 1; output_highlight ($args{'purpose'}); +$output_markdown_nopara = 0; print /refpurpose\n; print /refnamediv\n; @@ -1104,10 +1110,12 @@ sub output_function_xml(%) { $parameter_name =~ s/\[.*//; print varlistentry\n termparameter$parameter/parameter/term\n; - printlistitem\npara\n; + printlistitem\n; + print para\n unless $use_markdown; $lineprefix= ; output_highlight($args{'parameterdescs'}{$parameter_name}); - print /para\n /listitem\n /varlistentry\n; + print /para\n unless $use_markdown; + print/listitem\n /varlistentry\n; } print /variablelist\n; } else { @@ -1143,7 +1151,9 @@ sub output_struct_xml(%) { print refname . $args{'type'} . . $args{'struct'} . /refname\n; print refpurpose\n; print ; +$output_markdown_nopara = 1; output_highlight ($args{'purpose'}); +$output_markdown_nopara = 0; print /refpurpose\n; print /refnamediv\n; @@ -1196,9 +1206,11 @@ sub output_struct_xml(%) {
[Intel-gfx] [PATCH libdrm 03/17] intel: resolve shadowing warnings
Cc: intel-gfx@lists.freedesktop.org Signed-off-by: Emil Velikov emil.l.veli...@gmail.com --- intel/intel_bufmgr_fake.c | 2 +- intel/intel_bufmgr_gem.c | 7 +++ intel/intel_decode.c | 7 ++- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/intel/intel_bufmgr_fake.c b/intel/intel_bufmgr_fake.c index 75387b7..551e05d 100644 --- a/intel/intel_bufmgr_fake.c +++ b/intel/intel_bufmgr_fake.c @@ -1460,7 +1460,7 @@ restart: assert(ret == 0); if (bufmgr_fake-exec != NULL) { - int ret = bufmgr_fake-exec(bo, used, bufmgr_fake-exec_priv); + ret = bufmgr_fake-exec(bo, used, bufmgr_fake-exec_priv); if (ret != 0) { pthread_mutex_unlock(bufmgr_fake-lock); return ret; diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index 2723e21..cf55a53 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -2165,8 +2165,7 @@ drm_intel_gem_bo_exec(drm_intel_bo *bo, int used, drm_intel_gem_dump_validation_list(bufmgr_gem); for (i = 0; i bufmgr_gem-exec_count; i++) { - drm_intel_bo *bo = bufmgr_gem-exec_bos[i]; - drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo; + bo_gem = (drm_intel_bo_gem *) bufmgr_gem-exec_bos[i]; bo_gem-idle = false; @@ -2186,6 +2185,7 @@ do_exec2(drm_intel_bo *bo, int used, drm_intel_context *ctx, unsigned int flags) { drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bo-bufmgr; + drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo; struct drm_i915_gem_execbuffer2 execbuf; int ret = 0; int i; @@ -2260,8 +2260,7 @@ skip_execution: drm_intel_gem_dump_validation_list(bufmgr_gem); for (i = 0; i bufmgr_gem-exec_count; i++) { - drm_intel_bo *bo = bufmgr_gem-exec_bos[i]; - drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *)bo; + bo_gem = (drm_intel_bo_gem *) bufmgr_gem-exec_bos[i]; bo_gem-idle = false; diff --git a/intel/intel_decode.c b/intel/intel_decode.c index 2b902a3..345d457 100644 --- a/intel/intel_decode.c +++ b/intel/intel_decode.c @@ -3630,7 +3630,6 @@ decode_3d_965(struct drm_intel_decode *ctx) case 0x7a00: if (IS_GEN6(devid) || IS_GEN7(devid)) { - unsigned int i; if (len != 4 len != 5) fprintf(out, Bad count in PIPE_CONTROL\n); @@ -3732,8 +3731,6 @@ decode_3d_965(struct drm_intel_decode *ctx) if (opcode_3d-func) { return opcode_3d-func(ctx); } else { - unsigned int i; - instr_out(ctx, 0, %s\n, opcode_3d-name); for (i = 1; i len; i++) { @@ -3883,9 +3880,9 @@ drm_intel_decode_set_head_tail(struct drm_intel_decode *ctx, void drm_intel_decode_set_output_file(struct drm_intel_decode *ctx, -FILE *out) +FILE *output) { - ctx-out = out; + ctx-out = output; } /** -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH libdrm 04/17] intel: error out on has_error in exec2
Just like we do for the original exec() Cc: intel-gfx@lists.freedesktop.org Signed-off-by: Emil Velikov emil.l.veli...@gmail.com --- intel/intel_bufmgr_gem.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index cf55a53..5287419 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -2190,6 +2190,9 @@ do_exec2(drm_intel_bo *bo, int used, drm_intel_context *ctx, int ret = 0; int i; + if (bo_gem-has_error) + return -ENOMEM; + switch (flags 0x7) { default: return -EINVAL; -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback
On Mon, Aug 24, 2015 at 03:35:33PM +, Yang, Libin wrote: -Original Message- From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] Sent: Monday, August 24, 2015 8:53 PM To: Yang, Libin Cc: alsa-de...@alsa-project.org; ti...@suse.de; intel- g...@lists.freedesktop.org; daniel.vet...@ffwll.ch; jani.nik...@linux.intel.com Subject: Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback On Mon, Aug 24, 2015 at 02:38:14AM +, Yang, Libin wrote: -Original Message- From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] Sent: Friday, August 21, 2015 11:14 PM To: Yang, Libin Cc: alsa-de...@alsa-project.org; ti...@suse.de; intel- g...@lists.freedesktop.org; daniel.vet...@ffwll.ch; jani.nik...@linux.intel.com Subject: Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback On Tue, Aug 18, 2015 at 02:51:52PM +0800, libin.y...@intel.com wrote: From: Libin Yang libin.y...@intel.com HDMI audio may not work at some frequencies with the HW provided N/CTS. This patch sets the proper N value for the given audio sample rate at the impacted frequencies. At other frequencies, it will use the N/CTS value which HW provides. Signed-off-by: Libin Yang libin.y...@intel.com --- drivers/gpu/drm/i915/intel_audio.c | 119 + 1 file changed, 119 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index dc32cf4..96b97be 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -68,6 +68,31 @@ static const struct { { 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 }, }; +/* HDMI N/CTS table */ +#define TMDS_297M 297000 +#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001) I don't really like the defines. I agree it's not a good name. Do you have some suggestions? I'd probably just have used raw numbers myself. +static const struct { + int sample_rate; + int clock; + int n; + int cts; +} aud_ncts[] = { + { 44100, TMDS_296M, 4459, 234375 }, + { 44100, TMDS_297M, 4704, 247500 }, + { 48000, TMDS_296M, 5824, 281250 }, + { 48000, TMDS_297M, 5120, 247500 }, + { 32000, TMDS_296M, 5824, 421875 }, + { 32000, TMDS_297M, 3072, 222750 }, + { 88200, TMDS_296M, 8918, 234375 }, + { 88200, TMDS_297M, 9408, 247500 }, + { 96000, TMDS_296M, 11648, 281250 }, + { 96000, TMDS_297M, 10240, 247500 }, + { 176400, TMDS_296M, 17836, 234375 }, + { 176400, TMDS_297M, 18816, 247500 }, + { 44100, TMDS_296M, 23296, 281250 }, + { 44100, TMDS_297M, 20480, 247500 }, +}; Last two should be 192 kHz. All the other values seem to match the spec. Oh, my typo. Thanks for pointing it out. These do assume 8bpc, but as the spec doesn't even specify N/CTS values for deep color modes @ 297MHz, so I suppose the expectations is that the max TMDS clock will never be so high as to allow them. If/when we start using manual programming for other TMDS clock rates we'll need to consider 12bpc as well. These values are recommended from HDMI spec. It's not related to the bpp. Am I wrong? I'm find the value from HDMISpecification 1.4: table 7-1, table 7-2 and table 7-3. There are separate tables for deep color modes in appendix D. Tables D-1 through D-3 are of interest to us since we can only do the 12bpc deep color mode. Yes, we found the D tables in the spec before, and it seems a little Complicated. And the table 7-x seems to be more general. This is the reason we use table 7-x. What's complicated? With 8bpc you use 7-x, with 12bpc you use D-x. But as stated there are no 297MHz values in the D-x tables so the assumption seems to be that the max TMDS clock will be ~300Mhz and so there's no way to to get a 297Mhz pixel clock with deep color modes. The fact that the D-x tables refer to the pixel clock instead of TMDS clock is also a bit confusing. Seems they forgot about pixel replication there. But I believe we should just consider the pixel replicated pixel clock when consulting these tables. OK. We will use D-1, D-2 and D-3 table for N/CTS. If you're only worried about 297Mhz modes, then there should be no need to consult the D-x tables at this time. + /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */ static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode) { @@ -90,6 +115,31 @@ static u32 audio_config_hdmi_pixel_clock(struct drm_display_mode *mode) return hdmi_audio_clock[i].config; } +static int
Re: [Intel-gfx] About the iGVT-g's requirement to pin guest contexts in VM
Attach the big picture to help the discussion: Windows Guest Linux Guest Linux Guest (i915 guest mode) * We are here +--+ +--+ +---+ | | | | |Guest Context Lifecycle Management | |Windows Driver| | i915 | | +---+ | | | | +| |CREATE/DESTROY/PIN/UNPIN | | | (GUEST MODE) | | (GUEST MODE) | | | ++ ++ | | | | | | | | |Execlist Context| |Execlist Context| | | | | | | | | +-^--+ +--^-+ | | | | | | | +---|---|---+ | +--+ +--+ +-|---|-+ |NOTIFICATION | +-|---|-+ | XenGT Shadow Context Lifecycle|Management | | | | | | +-|---|--+ | | +---+ | +---v---+ +-v--+ | | | | i915 | | | Shadow Context| | Shadow Context | | | | | Host Mode | | +---+ ++ | | | +---+ ++ | | DOM0 Linux (i915 host mode w/XenGT) | +---+ +---+ | Hypervisor| +---+ SHADOW CONTEXT SUBMISSION As you can see, in this picture, each guest execlist context will have an related shadow context in host, and XenGT will be responsible for a. update SHADOW context via GUEST context when guest wants to submit an workload. b. submitting the shadow context into hardware. c. update the guest context via shadow context, when shadow context is finished. d. inject virtual context switch interrupt into guest. Then guest will see hey, my job was retired from hardware, then I can do something to my context. NOTIFICATION BETWEEN HOST AND GUEST Now in our design we have built a notification channel for guest to notify XenGT due to performance reason. With this channel, guest can play like this Hey XenGT, I created a new context, please shadow it! Oh, I have destroyed the context, please stop tracking this context. But when this trick comes before guest pin/unpin, it has problems. PROBLEMS First, guest context pin/unpin will cause LRCA change, which breaks relationship between guest context and shadow context. As you can see that in our design, XenGT needs an approach to find the related shadow context according to something of an guest context. For now we find the related shadow context via guest context ID(LRCA in fact). But whatever it is, I think there should be something unique. XenGT has no knowledge about guest context pin/unpin. Guest may swap out an context if it sees the seqno has been passed. When the XenGT wants to access guest context, the guest context may be not there (swapped-out already). It's hard and complicated for XenGT to track guest context without an unique context ID and a stable execlist context backing store. For now the whole tricks are only works under virtualization environment and will not affect the native i915. Welcome to discussions! :) Thanks, Zhi. -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Monday, August 24, 2015 6:23 PM To: intel-gfx@lists.freedesktop.org; igv...@lists.01.org; Wang, Zhi A; Tian, Kevin; joonas.lahti...@linux.intel.com Subject: Re: About the iGVT-g's requirement to pin guest contexts in VM On Mon, Aug 24, 2015 at 06:04:28PM +0800, Zhiyuan Lv wrote: Hi Chris, On Thu, Aug 20, 2015 at 09:36:00AM +0100, Chris Wilson wrote: On Thu, Aug 20, 2015 at 03:45:21PM +0800, Zhiyuan Lv wrote: Intel GVT-g will perform EXECLIST context shadowing and ring buffer shadowing. The shadow copy is created when guest creates a context. If a context changes its LRCA address, the hypervisor is hard to know whether it is a new context or not. We always pin context objects to global GTT to make life easier. Nak. Please explain why we need to workaround a bug in the host. We cannot pin the context as that breaks userspace (e.g. synmark) who can and will try to use more contexts than we have room. Could you have a look at below reasons and kindly give us your inputs? 1, Due to the GGTT partitioning, the global graphics memory
Re: [Intel-gfx] [PATCH 5/7] drm/i915: Delay first PSR activation.
Em Qui, 2015-08-20 às 17:55 -0700, Rodrigo Vivi escreveu: This affects PSR on VLV, CHV, HSW and BDW. When debuging the frozen screen caused by HW tracking with low power state I noticed that if we keep moving the mouse non stop you will miss the screen updates for a while. At least until we stop moving the mouse for a small time and move again. The actual enabling should happen immediately after Display Port enabling sequence finished with links trained and everything enabled. However we face many issues when enabling PSR right after a modeset. On VLV/CHV we face blank screens on this scenario and on HSW+ we face a recoverable frozen screen, at least until next exit-activate sequence. Another workaround for the same issue here would be to increase re-enable idle time from 100 to 500 as we did for VLV/CHV. However this patch workaround this issue in a better way since it doesn't reduce PSR residency and also allow us to reduce the delay time between re-enables at least on VLV/CHV. It sounds like this could use a little more debugging. Have we tried moving the psr enable to after intel_post_plane_update()? The move the cursor to reproduce bug sounds very weird. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_psr.c | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index d02d4e2..2be4a62 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -427,6 +427,19 @@ void intel_psr_enable(struct intel_dp *intel_dp) vlv_psr_enable_source(intel_dp); } + /* + * FIXME: Activation should happen immediately since this function + * is just called after pipe is fully trained and enabled. + * However on previous platforms we face issues when first activation + * follows a modeset so quickly. + * - On VLV/CHV we get bank screen on first activation + * - On HSW/BDW we get a recoverable frozen screen until next + * exit-activate sequence. + */ + if (INTEL_INFO(dev)-gen 9) + schedule_delayed_work(dev_priv-psr.work, + msecs_to_jiffies(500)); + dev_priv-psr.enabled = intel_dp; unlock: mutex_unlock(dev_priv-psr.lock); @@ -729,8 +742,9 @@ void intel_psr_flush(struct drm_device *dev, intel_psr_exit(dev); if (!dev_priv-psr.active !dev_priv -psr.busy_frontbuffer_bits) - schedule_delayed_work(dev_priv-psr.work, - msecs_to_jiffies(delay_ms)); + if (!work_busy(dev_priv-psr.work.work)) + schedule_delayed_work(dev_priv-psr.work, + msecs_to_jiffies(delay_ms)); mutex_unlock(dev_priv-psr.lock); } ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/1] drm/i915: Allow parsing of variable size child device entries from VBT
On Fri, 21 Aug 2015, Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Fri, Aug 21, 2015 at 04:52:01PM +0300, Jani Nikula wrote: From: David Weinehall david.weineh...@linux.intel.com VBT version 196 increased the size of common_child_dev_config. The parser code assumed that the size of this structure would not change. The modified code now copies the amount needed based on the VBT version, and emits a debug message if the VBT version is unknown (too new); since the struct config block won't shrink in newer versions it should be harmless to copy the maximum known size in such cases, so that's what we do, but emitting the warning is probably sensible anyway. In the longer run it might make sense to modify the parser code to use a version/feature mapping, rather than hardcoding things like this, but for now the variants are fairly managable. This fixes a regression introduced in commit 75067ddecf21271631bc018d2fb23ddd09b66aae Author: Antti Koskipaa antti.koski...@linux.intel.com Date: Fri Jul 10 14:10:55 2015 +0300 drm/i915: Per-DDI I_boost override since that commit changed the child device config size without updating the checks and memcpy. v2: Stricter size checks v3 by Jani: - Keep the checks strict, and warnigns verbose, but keep going anyway. - Take care to copy the max amount of child device config we can. - Fix the messages. Signed-off-by: David Weinehall david.weineh...@linux.intel.com Signed-off-by: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/intel_bios.c | 37 + drivers/gpu/drm/i915/intel_bios.h | 6 -- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 64e5b15ae0b6..be83b77aa018 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -1051,17 +1051,39 @@ parse_device_mapping(struct drm_i915_private *dev_priv, const union child_device_config *p_child; union child_device_config *child_dev_ptr; int i, child_device_num, count; -u16 block_size; +u8 expected_size; +u16 block_size; p_defs = find_section(bdb, BDB_GENERAL_DEFINITIONS); if (!p_defs) { DRM_DEBUG_KMS(No general definition block is found, no devices defined.\n); return; } -if (p_defs-child_dev_size sizeof(*p_child)) { -DRM_ERROR(General definiton block child device size is too small.\n); +if (bdb-version 195) { +expected_size = sizeof(struct old_child_dev_config); +} else if (bdb-version == 195) { +expected_size = 37; +} else if (bdb-version = 197) { +expected_size = 38; +} else { +expected_size = 38; +BUILD_BUG_ON(sizeof(*p_child) 38); +DRM_DEBUG_DRIVER(Expected child device config size for VBT version %u not known; assuming %u\n, + bdb-version, expected_size); +} + +/* The legacy sized child device config is the minimum we need. */ +if (p_defs-child_dev_size sizeof(struct old_child_dev_config)) { +DRM_ERROR(Child device config size %u is too small.\n, + p_defs-child_dev_size); return; } + +/* Flag an error for unexpected size, but continue anyway. */ +if (p_defs-child_dev_size != expected_size) +DRM_ERROR(Unexpected child device config size %u (expected %u for VBT version %u)\n, + p_defs-child_dev_size, expected_size, bdb-version); + /* get the block size of general definitions */ block_size = get_blocksize(p_defs); /* get the number of child device */ @@ -1106,7 +1128,14 @@ parse_device_mapping(struct drm_i915_private *dev_priv, child_dev_ptr = dev_priv-vbt.child_dev + count; count++; -memcpy(child_dev_ptr, p_child, sizeof(*p_child)); + +/* + * Copy as much as we know (sizeof) and is available + * (child_dev_size) of the child device. Accessing the data must + * depend on VBT version. + */ +memcpy(child_dev_ptr, p_child, + min_t(size_t, p_defs-child_dev_size, sizeof(*p_child))); Looks good. Could maybe use a BUILD_BUG_ON(sizeof(struct old_child_dev_config) != 33) somewhere to make sure people don't make a mess of things. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Pushed to drm-intel-next-fixes, thanks for the review. BR, Jani. } return; } diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h index 6d909efbf43f..06d0dbde2be6 100644 --- a/drivers/gpu/drm/i915/intel_bios.h +++ b/drivers/gpu/drm/i915/intel_bios.h @@ -203,9 +203,11 @@ struct bdb_general_features { #define DEVICE_PORT_DVOB0x01 #define
Re: [Intel-gfx] [PATCH 4/4] drm/i915: fix link rates reported for SKL
On Tue, 18 Aug 2015, Sivakumar Thulasimani sivakumar.thulasim...@intel.com wrote: From: Thulasimani,Sivakumar sivakumar.thulasim...@intel.com This patch fixes the bug that SKL SKUs before B0 might return HBR2 as supported even though it is not supposed to be enabled on such platforms. v2: optimize if else condition (Jani) Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Sivakumar Thulasimani sivakumar.thulasim...@intel.com Pushed to drm-intel-next-fixes, thanks for the patch and review. BR, Jani. --- drivers/gpu/drm/i915/intel_dp.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 8bc6361..32bf961 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1224,21 +1224,24 @@ static bool intel_dp_source_supports_hbr2(struct drm_device *dev) static int intel_dp_source_rates(struct drm_device *dev, const int **source_rates) { + int size; + if (IS_BROXTON(dev)) { *source_rates = bxt_rates; - return ARRAY_SIZE(bxt_rates); + size = ARRAY_SIZE(bxt_rates); } else if (IS_SKYLAKE(dev)) { *source_rates = skl_rates; - return ARRAY_SIZE(skl_rates); + size = ARRAY_SIZE(skl_rates); + } else { + *source_rates = default_rates; + size = ARRAY_SIZE(default_rates); } - *source_rates = default_rates; - /* This depends on the fact that 5.4 is last value in the array */ - if (intel_dp_source_supports_hbr2(dev)) - return (DP_LINK_BW_5_4 3) + 1; - else - return (DP_LINK_BW_2_7 3) + 1; + if (!intel_dp_source_supports_hbr2(dev)) + size--; + + return size; } static void -- 1.7.9.5 -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] About the iGVT-g's requirement to pin guest contexts in VM
On Mon, Aug 24, 2015 at 06:04:28PM +0800, Zhiyuan Lv wrote: Hi Chris, On Thu, Aug 20, 2015 at 09:36:00AM +0100, Chris Wilson wrote: On Thu, Aug 20, 2015 at 03:45:21PM +0800, Zhiyuan Lv wrote: Intel GVT-g will perform EXECLIST context shadowing and ring buffer shadowing. The shadow copy is created when guest creates a context. If a context changes its LRCA address, the hypervisor is hard to know whether it is a new context or not. We always pin context objects to global GTT to make life easier. Nak. Please explain why we need to workaround a bug in the host. We cannot pin the context as that breaks userspace (e.g. synmark) who can and will try to use more contexts than we have room. Could you have a look at below reasons and kindly give us your inputs? 1, Due to the GGTT partitioning, the global graphics memory available inside virtual machines is much smaller than native case. We cannot support some graphics memory intensive workloads anyway. So it looks affordable to just pin contexts which do not take much GGTT. Wrong. It exposes the guest to a trivial denial-of-service attack. A smaller GGTT does not actually limit clients (there is greater aperture pressure and some paths are less likely but an individual client will function just fine). 2, Our hypervisor needs to change i915 guest context in the shadow context implementation. That part will be tricky if the context is not always pinned. One scenario is that when a context finishes running, we need to copy shadow context, which has been updated by hardware, to guest context. The hypervisor knows context finishing by context interrupt, but that time shrinker may have unpin the context and its backing storage may have been swap-out. Such copy may fail. That is just a bug in your code. Firstly allowing swapout on an object you still are using, secondly not being able to swapin. -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 V2] drm/i915/skl+: Add YUV pixel format in Capability list
Can you please add the test case name to the commit message? Also, this should be split into two patches one addressing the divide by zero error and another one to add plane formats. Regards, Sonika -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Kumar, Mahesh Sent: Friday, July 17, 2015 7:21 PM To: intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [PATCH V2] drm/i915/skl+: Add YUV pixel format in Capability list GEN = 9 supports YUV format for all planes, but it's not exported in Capability list of primary plane. Add YUV formats in skl_primary_formats list. Don't rely on fb-bits_per_pixel as intel_framebuffer_init is not filling bits_per_pixel field of fb-struct for YUV pixel format. This leads to divide by zero error during watermark calculation. V2: Don't break NV12 case. Signed-off-by: Kumar, Mahesh mahesh1.ku...@intel.com Cc: Konduru, Chandra chandra.kond...@intel.com --- IGT changes made for testcase will be sent in separate patch. drivers/gpu/drm/i915/intel_display.c | 4 drivers/gpu/drm/i915/intel_pm.c | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index af0bcfe..d31704a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -72,6 +72,10 @@ static const uint32_t skl_primary_formats[] = { DRM_FORMAT_ABGR, DRM_FORMAT_XRGB2101010, DRM_FORMAT_XBGR2101010, + DRM_FORMAT_YUYV, + DRM_FORMAT_YVYU, + DRM_FORMAT_UYVY, + DRM_FORMAT_VYUY, }; /* Cursor formats */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 5eeddc9..5768f8c 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3164,7 +3164,8 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc, if (fb) { p-plane[0].enabled = true; p-plane[0].bytes_per_pixel = fb-pixel_format == DRM_FORMAT_NV12 ? - drm_format_plane_cpp(fb-pixel_format, 1) : fb-bits_per_pixel / 8; + drm_format_plane_cpp(fb-pixel_format, 1) : + drm_format_plane_cpp(fb-pixel_format, 0); p-plane[0].y_bytes_per_pixel = fb-pixel_format == DRM_FORMAT_NV12 ? drm_format_plane_cpp(fb-pixel_format, 0) : 0; p-plane[0].tiling = fb-modifier[0]; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V2] drm/i915/skl+: Add YUV pixel format in Capability list
Ok..., Will resubmit the patch with suggested changes. Regards, -Mahesh On 8/24/2015 3:53 PM, Jindal, Sonika wrote: Can you please add the test case name to the commit message? Also, this should be split into two patches one addressing the divide by zero error and another one to add plane formats. Regards, Sonika -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Kumar, Mahesh Sent: Friday, July 17, 2015 7:21 PM To: intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [PATCH V2] drm/i915/skl+: Add YUV pixel format in Capability list GEN = 9 supports YUV format for all planes, but it's not exported in Capability list of primary plane. Add YUV formats in skl_primary_formats list. Don't rely on fb-bits_per_pixel as intel_framebuffer_init is not filling bits_per_pixel field of fb-struct for YUV pixel format. This leads to divide by zero error during watermark calculation. V2: Don't break NV12 case. Signed-off-by: Kumar, Mahesh mahesh1.ku...@intel.com Cc: Konduru, Chandra chandra.kond...@intel.com --- IGT changes made for testcase will be sent in separate patch. drivers/gpu/drm/i915/intel_display.c | 4 drivers/gpu/drm/i915/intel_pm.c | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index af0bcfe..d31704a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -72,6 +72,10 @@ static const uint32_t skl_primary_formats[] = { DRM_FORMAT_ABGR, DRM_FORMAT_XRGB2101010, DRM_FORMAT_XBGR2101010, + DRM_FORMAT_YUYV, + DRM_FORMAT_YVYU, + DRM_FORMAT_UYVY, + DRM_FORMAT_VYUY, }; /* Cursor formats */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 5eeddc9..5768f8c 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3164,7 +3164,8 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc, if (fb) { p-plane[0].enabled = true; p-plane[0].bytes_per_pixel = fb-pixel_format == DRM_FORMAT_NV12 ? - drm_format_plane_cpp(fb-pixel_format, 1) : fb-bits_per_pixel / 8; + drm_format_plane_cpp(fb-pixel_format, 1) : + drm_format_plane_cpp(fb-pixel_format, 0); p-plane[0].y_bytes_per_pixel = fb-pixel_format == DRM_FORMAT_NV12 ? drm_format_plane_cpp(fb-pixel_format, 0) : 0; p-plane[0].tiling = fb-modifier[0]; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] Revert tests/gem_ctx_param_basic: fix invalid params
On Fri, 2015-08-21 at 16:26 +0300, Ander Conselvan De Oliveira wrote: On Fri, 2015-08-07 at 15:53 +0300, David Weinehall wrote: On Thu, Aug 06, 2015 at 11:33:00PM +0200, Daniel Vetter wrote: This reverts commit 0b45b0746f45deea11670a8b2c949776bbbef55c. The point of testing for LAST_FLAG + 1 is to catch abi extensions - despite our best efforts we really suck at properly reviewing for test coverage when extending ABI. The real bug here is that David Weinhall hasn't submitted updated igts for the NO_ZEROMAP feature yet. Imo the right course of action is to revert that feature if the testcase don't show up within a few days. The reason I never submitted it was probably because of Chris's strong opposition to the feature in the first place; I've had the testcase laying around on my computer for quite a while. Anyhow, here's a slightly modified version of that test -- hopefully not breaking anything. Signed-off-by: David Weinehall david.weineh...@linux.intel.com diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h index bc5d4bd827cf..f4deca6bd79e 100644 --- a/lib/ioctl_wrappers.h +++ b/lib/ioctl_wrappers.h @@ -102,6 +102,7 @@ struct local_i915_gem_context_param { uint32_t size; uint64_t param; #define LOCAL_CONTEXT_PARAM_BAN_PERIOD 0x1 +#define LOCAL_CONTEXT_PARAM_NO_ZEROMAP 0x2 uint64_t value; }; void gem_context_require_ban_period(int fd); diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c index b44b37cf0538..1e7e8ff40703 100644 --- a/tests/gem_ctx_param_basic.c +++ b/tests/gem_ctx_param_basic.c @@ -57,7 +57,7 @@ igt_main ctx = gem_context_create(fd); } - ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD; + ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD; igt_subtest(basic) { ctx_param.context = ctx; @@ -98,21 +98,31 @@ igt_main [...] - ctx_param.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD; + ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP; - igt_subtest(non-root-set) { + igt_subtest(non-root-set-no-zeromap) { igt_fork(child, 1) { igt_drop_root(); ctx_param.context = ctx; TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EINVAL); TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM); ctx_param.value--; TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM); (I've added the code missing from the context) Except I added the wrong code. Here's what is in i-g-t now: ctx_param.param = LOCAL_CONTEXT_PARAM_NO_ZEROMAP; igt_subtest(non-root-set-no-zeromap) { igt_fork(child, 1) { igt_drop_root(); ctx_param.context = ctx; TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM); ctx_param.value--; TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EPERM); } igt_waitchildren(); } The code in i915_gem_context_setparam_ioctl() that handles CONTEXT_PARAM_NO_ZEROMAP never returns EPERM, so this test always fails. Ander ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] About the iGVT-g's requirement to pin guest contexts in VM
Hi Chris, On Thu, Aug 20, 2015 at 09:36:00AM +0100, Chris Wilson wrote: On Thu, Aug 20, 2015 at 03:45:21PM +0800, Zhiyuan Lv wrote: Intel GVT-g will perform EXECLIST context shadowing and ring buffer shadowing. The shadow copy is created when guest creates a context. If a context changes its LRCA address, the hypervisor is hard to know whether it is a new context or not. We always pin context objects to global GTT to make life easier. Nak. Please explain why we need to workaround a bug in the host. We cannot pin the context as that breaks userspace (e.g. synmark) who can and will try to use more contexts than we have room. Could you have a look at below reasons and kindly give us your inputs? 1, Due to the GGTT partitioning, the global graphics memory available inside virtual machines is much smaller than native case. We cannot support some graphics memory intensive workloads anyway. So it looks affordable to just pin contexts which do not take much GGTT. 2, Our hypervisor needs to change i915 guest context in the shadow context implementation. That part will be tricky if the context is not always pinned. One scenario is that when a context finishes running, we need to copy shadow context, which has been updated by hardware, to guest context. The hypervisor knows context finishing by context interrupt, but that time shrinker may have unpin the context and its backing storage may have been swap-out. Such copy may fail. Look forward to your comments. Thanks! Regards, -Zhiyuan -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: Use dpcd read wake for sink crc calls.
Em Qui, 2015-08-20 às 16:23 -0700, Rodrigo Vivi escreveu: Let's use a native read with retry as suggested per spec to fix Sink CRC on SKL when PSR is enabled. With PSR enabled panel is probably taking more time to wake and dpcd read is faling. Does this commit actually fix any known problem with Sink CRC? Or is it just a try? It would be nice to have this clarified in the commit message. Anyway, it looks correct, so: Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com v2: Fix my email domain on commit message. Thanks Rafael. Cc: Rafael Antognolli rafael.antogno...@intel.com Cc: Sonika Jindal sonika.jin...@intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d32ce48..34f5e33 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4037,7 +4037,8 @@ static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp) u8 buf; int ret = 0; - if (drm_dp_dpcd_readb(intel_dp-aux, DP_TEST_SINK, buf) 0) { + if (intel_dp_dpcd_read_wake(intel_dp-aux, DP_TEST_SINK, + buf, 1) 0) { DRM_DEBUG_KMS(Sink CRC couldn't be stopped properly\n); ret = -EIO; goto out; @@ -4069,7 +4070,8 @@ static int intel_dp_sink_crc_start(struct intel_dp *intel_dp) return ret; } - if (drm_dp_dpcd_readb(intel_dp-aux, DP_TEST_SINK_MISC, buf) 0) + if (intel_dp_dpcd_read_wake(intel_dp-aux, DP_TEST_SINK_MISC, + buf, 1) 0) return -EIO; if (!(buf DP_TEST_CRC_SUPPORTED)) @@ -4077,7 +4079,7 @@ static int intel_dp_sink_crc_start(struct intel_dp *intel_dp) intel_dp-sink_crc.last_count = buf DP_TEST_COUNT_MASK; - if (drm_dp_dpcd_readb(intel_dp-aux, DP_TEST_SINK, buf) 0) + if (intel_dp_dpcd_read_wake(intel_dp-aux, DP_TEST_SINK, buf, 1) 0) return -EIO; hsw_disable_ips(intel_crtc); @@ -4109,8 +4111,8 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) do { intel_wait_for_vblank(dev, intel_crtc-pipe); - if (drm_dp_dpcd_readb(intel_dp-aux, - DP_TEST_SINK_MISC, buf) 0) { + if (intel_dp_dpcd_read_wake(intel_dp-aux, + DP_TEST_SINK_MISC, buf, 1) 0) { ret = -EIO; goto stop; } @@ -4123,7 +4125,8 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) if (count == 0) intel_dp-sink_crc.last_count = 0; - if (drm_dp_dpcd_read(intel_dp-aux, DP_TEST_CRC_R_CR, crc, 6) 0) { + if (intel_dp_dpcd_read_wake(intel_dp-aux, DP_TEST_CRC_R_CR, + crc, 6) 0) { ret = -EIO; goto stop; } ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use dpcd read wake for sink crc calls.
On Mon, 2015-08-24 at 19:54 +, Zanoni, Paulo R wrote: Em Qui, 2015-08-20 às 16:23 -0700, Rodrigo Vivi escreveu: Let's use a native read with retry as suggested per spec to fix Sink CRC on SKL when PSR is enabled. With PSR enabled panel is probably taking more time to wake and dpcd read is faling. Does this commit actually fix any known problem with Sink CRC? Or is it just a try? It would be nice to have this clarified in the commit message. It was just a try but that made sink crc working on my SKL when PSR is enabled. nothing much to add... Anyway, it looks correct, so: Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com v2: Fix my email domain on commit message. Thanks Rafael. Cc: Rafael Antognolli rafael.antogno...@intel.com Cc: Sonika Jindal sonika.jin...@intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d32ce48..34f5e33 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4037,7 +4037,8 @@ static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp) u8 buf; int ret = 0; - if (drm_dp_dpcd_readb(intel_dp-aux, DP_TEST_SINK, buf) 0) { + if (intel_dp_dpcd_read_wake(intel_dp-aux, DP_TEST_SINK, + buf, 1) 0) { DRM_DEBUG_KMS(Sink CRC couldn't be stopped properly\n); ret = -EIO; goto out; @@ -4069,7 +4070,8 @@ static int intel_dp_sink_crc_start(struct intel_dp *intel_dp) return ret; } - if (drm_dp_dpcd_readb(intel_dp-aux, DP_TEST_SINK_MISC, buf) 0) + if (intel_dp_dpcd_read_wake(intel_dp-aux, DP_TEST_SINK_MISC, + buf, 1) 0) return -EIO; if (!(buf DP_TEST_CRC_SUPPORTED)) @@ -4077,7 +4079,7 @@ static int intel_dp_sink_crc_start(struct intel_dp *intel_dp) intel_dp-sink_crc.last_count = buf DP_TEST_COUNT_MASK; - if (drm_dp_dpcd_readb(intel_dp-aux, DP_TEST_SINK, buf) 0) + if (intel_dp_dpcd_read_wake(intel_dp-aux, DP_TEST_SINK, buf, 1) 0) return -EIO; hsw_disable_ips(intel_crtc); @@ -4109,8 +4111,8 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) do { intel_wait_for_vblank(dev, intel_crtc-pipe); - if (drm_dp_dpcd_readb(intel_dp-aux, - DP_TEST_SINK_MISC, buf) 0) { + if (intel_dp_dpcd_read_wake(intel_dp-aux, + DP_TEST_SINK_MISC, buf, 1) 0) { ret = -EIO; goto stop; } @@ -4123,7 +4125,8 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) if (count == 0) intel_dp-sink_crc.last_count = 0; - if (drm_dp_dpcd_read(intel_dp-aux, DP_TEST_CRC_R_CR, crc, 6) 0) { + if (intel_dp_dpcd_read_wake(intel_dp-aux, DP_TEST_CRC_R_CR, + crc, 6) 0) { ret = -EIO; goto stop; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/7] drm/i915: Remove duplicated dpcd write on hsw_psr_enable_sink.
On Mon, 2015-08-24 at 14:04 +, Zanoni, Paulo R wrote: Em Qui, 2015-08-20 às 17:55 -0700, Rodrigo Vivi escreveu: This is wrong since my commit (89251b17). The intention of that commit was to remove this one here that is also wrong anyway, but it was forgotten. You mentioned the current code is wrong, but it would be really nice if you could describe why it is wrong and what are the implications of it being wrong. Was this causing some specific problem or was it just found by code inspection? In case it was found just by code inspection, you could try to elaborate on the possible problems brought by the bad commit. nothing critical, by wrong I meant ~DP_PSR_MAIN_LINK_ACTIVE doesn't do anything at all I don't remember if when I added I intended to be informative or I removed the val read from dpdc and forgot to remove this part... A more elaborate description helps not only the reviewers, but also the possible backporters and even helps us judging whether it should be merged by Daniel or Jani. Anyway, as far as I see the patch looks correct, so if you could provide another paragraph for Daniel to amend on the commit message: Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com Thanks Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_psr.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index a04b4dc..51f0514 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -170,9 +170,6 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp) aux_clock_divider = intel_dp -get_aux_clock_divider(intel_dp, 0); - drm_dp_dpcd_writeb(intel_dp-aux, DP_PSR_EN_CFG, - DP_PSR_ENABLE ~DP_PSR_MAIN_LINK_ACTIVE); - /* Enable AUX frame sync at sink */ if (dev_priv-psr.aux_frame_sync) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Also call frontbuffer flip when disabling planes.
We also need to call the frontbuffer flip to trigger proper invalidations when disabling planes. Otherwise we will miss screen updates when disabling sprites or cursor. On core platforms where HW tracking also works, this issue is totally masked because HW tracking triggers PSR exit however on VLV/CHV that has only SW tracking we miss screen updates when disabling planes. It was caught with kms_psr_sink_crc sprite_plane_onoff and cursor_plane_onoff subtests running on VLV/CHV. This is probably a regression since I can also get this with the manual test case, but with so many changes on atomic modeset I couldn't track exactly when this was introduced. Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f604ce1..930e5d0 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11627,7 +11627,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, intel_crtc-atomic.update_wm_pre = true; } - if (visible) + if (visible || was_visible) intel_crtc-atomic.fb_bits |= to_intel_plane(plane)-frontbuffer_bit; -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback
-Original Message- From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] Sent: Tuesday, August 25, 2015 12:27 AM To: Yang, Libin Cc: alsa-de...@alsa-project.org; ti...@suse.de; intel- g...@lists.freedesktop.org; daniel.vet...@ffwll.ch; jani.nik...@linux.intel.com Subject: Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback On Mon, Aug 24, 2015 at 03:35:33PM +, Yang, Libin wrote: -Original Message- From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] Sent: Monday, August 24, 2015 8:53 PM To: Yang, Libin Cc: alsa-de...@alsa-project.org; ti...@suse.de; intel- g...@lists.freedesktop.org; daniel.vet...@ffwll.ch; jani.nik...@linux.intel.com Subject: Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback On Mon, Aug 24, 2015 at 02:38:14AM +, Yang, Libin wrote: -Original Message- From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] Sent: Friday, August 21, 2015 11:14 PM To: Yang, Libin Cc: alsa-de...@alsa-project.org; ti...@suse.de; intel- g...@lists.freedesktop.org; daniel.vet...@ffwll.ch; jani.nik...@linux.intel.com Subject: Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: implement sync_audio_rate callback On Tue, Aug 18, 2015 at 02:51:52PM +0800, libin.y...@intel.com wrote: From: Libin Yang libin.y...@intel.com HDMI audio may not work at some frequencies with the HW provided N/CTS. This patch sets the proper N value for the given audio sample rate at the impacted frequencies. At other frequencies, it will use the N/CTS value which HW provides. Signed-off-by: Libin Yang libin.y...@intel.com --- drivers/gpu/drm/i915/intel_audio.c | 119 + 1 file changed, 119 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index dc32cf4..96b97be 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -68,6 +68,31 @@ static const struct { { 148500, AUD_CONFIG_PIXEL_CLOCK_HDMI_148500 }, }; +/* HDMI N/CTS table */ +#define TMDS_297M 297000 +#define TMDS_296M DIV_ROUND_UP(297000 * 1000, 1001) I don't really like the defines. I agree it's not a good name. Do you have some suggestions? I'd probably just have used raw numbers myself. +static const struct { + int sample_rate; + int clock; + int n; + int cts; +} aud_ncts[] = { + { 44100, TMDS_296M, 4459, 234375 }, + { 44100, TMDS_297M, 4704, 247500 }, + { 48000, TMDS_296M, 5824, 281250 }, + { 48000, TMDS_297M, 5120, 247500 }, + { 32000, TMDS_296M, 5824, 421875 }, + { 32000, TMDS_297M, 3072, 222750 }, + { 88200, TMDS_296M, 8918, 234375 }, + { 88200, TMDS_297M, 9408, 247500 }, + { 96000, TMDS_296M, 11648, 281250 }, + { 96000, TMDS_297M, 10240, 247500 }, + { 176400, TMDS_296M, 17836, 234375 }, + { 176400, TMDS_297M, 18816, 247500 }, + { 44100, TMDS_296M, 23296, 281250 }, + { 44100, TMDS_297M, 20480, 247500 }, +}; Last two should be 192 kHz. All the other values seem to match the spec. Oh, my typo. Thanks for pointing it out. These do assume 8bpc, but as the spec doesn't even specify N/CTS values for deep color modes @ 297MHz, so I suppose the expectations is that the max TMDS clock will never be so high as to allow them. If/when we start using manual programming for other TMDS clock rates we'll need to consider 12bpc as well. These values are recommended from HDMI spec. It's not related to the bpp. Am I wrong? I'm find the value from HDMISpecification 1.4: table 7-1, table 7-2 and table 7-3. There are separate tables for deep color modes in appendix D. Tables D-1 through D-3 are of interest to us since we can only do the 12bpc deep color mode. Yes, we found the D tables in the spec before, and it seems a little Complicated. And the table 7-x seems to be more general. This is the reason we use table 7-x. What's complicated? With 8bpc you use 7-x, with 12bpc you use D-x. But as stated there are no 297MHz values in the D-x tables so the assumption seems to be that the max TMDS clock will be ~300Mhz and so there's no way to to get a 297Mhz pixel clock with deep color modes. The fact that the D-x tables refer to the pixel clock instead of TMDS clock is also a bit confusing. Seems they forgot about pixel replication there. But I believe we should just consider the pixel replicated pixel clock when consulting these tables. OK. We will use D-1, D-2 and D-3 table for N/CTS. If you're
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Fix PSR disable sequence on core platforms.
On Mon, 2015-08-24 at 14:14 +, Zanoni, Paulo R wrote: Em Qui, 2015-08-20 às 17:55 -0700, Rodrigo Vivi escreveu: According to spec the disable sequence is: Driver will do the following on PSR Disable. 1. Disable PSR in PSR control register, SRD_CTL[bit 31]. 2. Poll on PSR idle 3. Wait for VBlank 4. Disable VSC DIP. Shouldn't this be done at intel_psr_exit() instead of intel_psr_disable()? don't think this is a good idea... In case it's yes (which is my guess), then the wait_for_vblank() is probably going to slow down everything, so we may need to use some sort of delayed work. delayed work wouldn't help because locks would still block the exit... In case it's no, then I don't think we need the wait_for_vblank() since the encoder disable only happens after the pipe disable. hm indeed... so I believe just ignore this patch for now since it works without this... Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_psr.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 51f0514..92e2b467 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -459,6 +459,10 @@ static void hsw_psr_disable(struct intel_dp *intel_dp) struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); struct drm_device *dev = intel_dig_port-base.base.dev; struct drm_i915_private *dev_priv = dev-dev_private; + struct drm_crtc *crtc = intel_dig_port-base.base.crtc; + enum pipe pipe = to_intel_crtc(crtc)-pipe; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc-config -cpu_transcoder); if (dev_priv-psr.active) { I915_WRITE(EDP_PSR_CTL(dev), @@ -469,6 +473,12 @@ static void hsw_psr_disable(struct intel_dp *intel_dp) EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10)) DRM_ERROR(Timed out waiting for PSR Idle State\n); + intel_wait_for_vblank(dev, pipe); + + I915_WRITE(ctl_reg, I915_READ(ctl_reg) + ~VIDEO_DIP_ENABLE_VSC_HSW); + POSTING_READ(ctl_reg); + dev_priv-psr.active = false; } else { WARN_ON(I915_READ(EDP_PSR_CTL(dev)) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/skl: Update DDI buffer translation programming.
SKL-Y can now use the same programming for all VccIO values after an adjustment to I_boost. SKL-U DP table adjustments. 1. Remove SKL Y 0.95V from SKL H and S columns in all tables. The other SKL Y column removes the 0.85V VccIO so it now applies to all voltages. 2. DP table changes SKL U 400mV+0db dword 0 value from 2016h to 201Bh. 3. DP table changes SKL U 600mv+0db dword 0 value from 2016h to 201Bh. 4. DP table increases I_boost to level 3 for SKL Y 400mv+9.5db. v2: Fix compilation warnings as pointed by Paulo. Reference: Graphics Spec Change r97962 Cc: Arthur Runyan arthur.j.run...@intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_ddi.c | 75 ++-- 1 file changed, 25 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 56d778f..3b056c4 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -128,7 +128,7 @@ static const struct ddi_buf_trans bdw_ddi_translations_hdmi[] = { { 0x80FF, 0x001B0002, 0x0 },/* 9: 100010000 */ }; -/* Skylake H, S, and Skylake Y with 0.95V VccIO */ +/* Skylake H and S */ static const struct ddi_buf_trans skl_ddi_translations_dp[] = { { 0x2016, 0x00A0, 0x0 }, { 0x5012, 0x009B, 0x0 }, @@ -143,23 +143,23 @@ static const struct ddi_buf_trans skl_ddi_translations_dp[] = { /* Skylake U */ static const struct ddi_buf_trans skl_u_ddi_translations_dp[] = { - { 0x2016, 0x00A2, 0x0 }, + { 0x201B, 0x00A2, 0x0 }, { 0x5012, 0x0088, 0x0 }, { 0x7011, 0x0087, 0x0 }, - { 0x80009010, 0x00C7, 0x1 },/* Uses I_boost */ - { 0x2016, 0x009D, 0x0 }, + { 0x80009010, 0x00C7, 0x1 },/* Uses I_boost level 0x1 */ + { 0x201B, 0x009D, 0x0 }, { 0x5012, 0x00C7, 0x0 }, { 0x7011, 0x00C7, 0x0 }, { 0x2016, 0x0088, 0x0 }, { 0x5012, 0x00C7, 0x0 }, }; -/* Skylake Y with 0.85V VccIO */ -static const struct ddi_buf_trans skl_y_085v_ddi_translations_dp[] = { +/* Skylake Y */ +static const struct ddi_buf_trans skl_y_ddi_translations_dp[] = { { 0x0018, 0x00A2, 0x0 }, { 0x5012, 0x0088, 0x0 }, { 0x7011, 0x0087, 0x0 }, - { 0x80009010, 0x00C7, 0x1 },/* Uses I_boost */ + { 0x80009010, 0x00C7, 0x3 },/* Uses I_boost level 0x3 */ { 0x0018, 0x009D, 0x0 }, { 0x5012, 0x00C7, 0x0 }, { 0x7011, 0x00C7, 0x0 }, @@ -168,7 +168,7 @@ static const struct ddi_buf_trans skl_y_085v_ddi_translations_dp[] = { }; /* - * Skylake H and S, and Skylake Y with 0.95V VccIO + * Skylake H and S * eDP 1.4 low vswing translation parameters */ static const struct ddi_buf_trans skl_ddi_translations_edp[] = { @@ -202,10 +202,10 @@ static const struct ddi_buf_trans skl_u_ddi_translations_edp[] = { }; /* - * Skylake Y with 0.95V VccIO + * Skylake Y * eDP 1.4 low vswing translation parameters */ -static const struct ddi_buf_trans skl_y_085v_ddi_translations_edp[] = { +static const struct ddi_buf_trans skl_y_ddi_translations_edp[] = { { 0x0018, 0x00A8, 0x0 }, { 0x4013, 0x00AB, 0x0 }, { 0x7011, 0x00A4, 0x0 }, @@ -218,7 +218,7 @@ static const struct ddi_buf_trans skl_y_085v_ddi_translations_edp[] = { { 0x0018, 0x008A, 0x0 }, }; -/* Skylake H, S and U, and Skylake Y with 0.95V VccIO */ +/* Skylake U, H and S */ static const struct ddi_buf_trans skl_ddi_translations_hdmi[] = { { 0x0018, 0x00AC, 0x0 }, { 0x5012, 0x009D, 0x0 }, @@ -233,8 +233,8 @@ static const struct ddi_buf_trans skl_ddi_translations_hdmi[] = { { 0x0018, 0x00C7, 0x0 }, }; -/* Skylake Y with 0.85V VccIO */ -static const struct ddi_buf_trans skl_y_085v_ddi_translations_hdmi[] = { +/* Skylake Y */ +static const struct ddi_buf_trans skl_y_ddi_translations_hdmi[] = { { 0x0018, 0x00A1, 0x0 }, { 0x5012, 0x00DF, 0x0 }, { 0x7011, 0x0084, 0x0 }, @@ -244,7 +244,7 @@ static const struct ddi_buf_trans skl_y_085v_ddi_translations_hdmi[] = { { 0x6013, 0x00C7, 0x0 }, { 0x0018, 0x008A, 0x0 }, { 0x3015, 0x00C7, 0x0 },/* Default */ - { 0x80003015, 0x00C7, 0x7 },/* Uses I_boost */ + { 0x80003015, 0x00C7, 0x7 },/* Uses I_boost level 0x7 */ { 0x0018, 0x00C7, 0x0 }, }; @@ -335,19 +335,11 @@ intel_dig_port_supports_hdmi(const struct intel_digital_port *intel_dig_port) static const struct ddi_buf_trans *skl_get_buf_trans_dp(struct drm_device *dev,
Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Adding DDI_E power well domain
On Mon, 2015-08-17 at 01:50 +, Zhang, Xiong Y wrote: Sorry, but I don't get how this enables power_well_2 as well. I just see it enabling ddi A/E as the other. Maybe Paulo or Imre are the best one to review this. On Thu, Aug 13, 2015 at 2:54 AM Xiong Zhang xiong.y.zh...@intel.com wrote: From B spec, DDI_E port belong to PowerWell 2, but DDI_E share the powerwell_req/staus register bit with DDI_A which belong to DDI_A_E_POWER_WELL. In order to communicate with the connector on DDI-E, both DDI_A_E_POWER_WELL and POWER_WELL_2 must be enabled. Currently intel_dp_power_get(DDI_E) only enable DDI_A_E_POWER_WELL, this patch will not only enable DDI_a_E_POWER_WELL but also enable POWER_WELL_2. This patch also fix the DDI-E hotplug function. Signed-off-by: Xiong Zhang xiong.y.zh...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 2 ++ drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c| 3 ++- drivers/gpu/drm/i915/intel_runtime_pm.c | 2 ++ 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 86734be..5523b6e 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2564,6 +2564,8 @@ static const char *power_domain_str(enum intel_display_power_domain domain) return PORT_DDI_D_2_LANES; case POWER_DOMAIN_PORT_DDI_D_4_LANES: return PORT_DDI_D_4_LANES; + case POWER_DOMAIN_PORT_DDI_E_2_LANES: + return PORT_DDI_E_2_LANES; case POWER_DOMAIN_PORT_DSI: return PORT_DSI; case POWER_DOMAIN_PORT_CRT: diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b157865..ee71f90 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -182,6 +182,7 @@ enum intel_display_power_domain { POWER_DOMAIN_PORT_DDI_C_4_LANES, POWER_DOMAIN_PORT_DDI_D_2_LANES, POWER_DOMAIN_PORT_DDI_D_4_LANES, + POWER_DOMAIN_PORT_DDI_E_2_LANES, POWER_DOMAIN_PORT_DSI, POWER_DOMAIN_PORT_CRT, POWER_DOMAIN_PORT_OTHER, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 801187c..ccd3f0b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5150,7 +5150,6 @@ static enum intel_display_power_domain port_to_power_domain(enum port port) { switch (port) { case PORT_A: - case PORT_E: return POWER_DOMAIN_PORT_DDI_A_4_LANES; case PORT_B: return POWER_DOMAIN_PORT_DDI_B_4_LANES; @@ -5158,6 +5157,8 @@ static enum intel_display_power_domain port_to_power_domain(enum port port) return POWER_DOMAIN_PORT_DDI_C_4_LANES; case PORT_D: return POWER_DOMAIN_PORT_DDI_D_4_LANES; + case PORT_E: + return POWER_DOMAIN_PORT_DDI_E_2_LANES; default: WARN_ON_ONCE(1); return POWER_DOMAIN_PORT_OTHER; diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 821644d..af7fdb3 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -297,6 +297,7 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv, BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) | \ BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) | \ BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) | \ + BIT(POWER_DOMAIN_PORT_DDI_E_2_LANES) [Zhang, Xiong Y] this line put DDI_E_2_LANES into SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS, but the git format-patch doesn’t show #define SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS. So it isn’t easy to review this. ops, sorry, I had missed that indeed. looks the right thing to do so, Reviewed-by: Rodrigo Vivi rodrigo.v...@intel.com | \ BIT(POWER_DOMAIN_AUX_B) | \ BIT(POWER_DOMAIN_AUX_C) | \ BIT(POWER_DOMAIN_AUX_D) | \ @@ -316,6 +317,7 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv, #define SKL_DISPLAY_DDI_A_E_POWER_DOMAINS (\ BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) | \ BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) | \ + BIT(POWER_DOMAIN_PORT_DDI_E_2_LANES) | \ BIT(POWER_DOMAIN_INIT)) #define SKL_DISPLAY_DDI_B_POWER_DOMAINS ( \ BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) | \ -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org
Re: [Intel-gfx] About the iGVT-g's requirement to pin guest contexts in VM
Hi Chris, On Mon, Aug 24, 2015 at 11:23:13AM +0100, Chris Wilson wrote: On Mon, Aug 24, 2015 at 06:04:28PM +0800, Zhiyuan Lv wrote: Hi Chris, On Thu, Aug 20, 2015 at 09:36:00AM +0100, Chris Wilson wrote: On Thu, Aug 20, 2015 at 03:45:21PM +0800, Zhiyuan Lv wrote: Intel GVT-g will perform EXECLIST context shadowing and ring buffer shadowing. The shadow copy is created when guest creates a context. If a context changes its LRCA address, the hypervisor is hard to know whether it is a new context or not. We always pin context objects to global GTT to make life easier. Nak. Please explain why we need to workaround a bug in the host. We cannot pin the context as that breaks userspace (e.g. synmark) who can and will try to use more contexts than we have room. Could you have a look at below reasons and kindly give us your inputs? 1, Due to the GGTT partitioning, the global graphics memory available inside virtual machines is much smaller than native case. We cannot support some graphics memory intensive workloads anyway. So it looks affordable to just pin contexts which do not take much GGTT. Wrong. It exposes the guest to a trivial denial-of-service attack. A Inside a VM, indeed. smaller GGTT does not actually limit clients (there is greater aperture pressure and some paths are less likely but an individual client will function just fine). 2, Our hypervisor needs to change i915 guest context in the shadow context implementation. That part will be tricky if the context is not always pinned. One scenario is that when a context finishes running, we need to copy shadow context, which has been updated by hardware, to guest context. The hypervisor knows context finishing by context interrupt, but that time shrinker may have unpin the context and its backing storage may have been swap-out. Such copy may fail. That is just a bug in your code. Firstly allowing swapout on an object you still are using, secondly not being able to swapin. As Zhi replied in another email, we do not have the knowledge of guest driver's swap operations. If we cannot pin context, we may have to ask guest driver not to swap out context pages. Do you think that would be the right way to go? Thanks! Regards, -Zhiyuan -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 mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: PSR: Let's rely more on frontbuffer tracking.
On Mon, 2015-08-24 at 14:29 +, Zanoni, Paulo R wrote: Em Qui, 2015-08-20 às 17:55 -0700, Rodrigo Vivi escreveu: Many reasons here: - Hardware tracking also has hidden corner cases Can you please elaborate more on that? I really really really really really think we should try as hard as possible to cook some IGT cases if something is affecting us :) Nothing much to be elaborated... Just that case I mentioned with unrecoverable frozen screen after dpms off that I couldn't reproduce with igt. - Frontbuffer tracking is mature and reliable now - Our sw exit by unseting bit 31 is really fast and reliable. But doesn't it trigger an automatic link retraining? afaik nothing different from what hw needs to do when entering PSR already. But if this is not true we need to stop trying SW tracking at all for all core platforms but also let userspace to control when to enable disable PSR otherwise it would broken half of the world. Also frontbuffer tracking flush means invalidate and flush. I don't know what are the implications of this in the current context. So, let's rely more and do the proper meaning of flush for all cases without any workaround. I'm really in favor of the idea that if the HW can properly handle the flips, we should rely on it, since in a lot of modern desktop environments we basically do one flip per frame. Did we study how this patch affects the PSR residency on the different cases we care about? I believe what affects the PSR residency for good is to remove the 100ms delay on re-enable. (yes, I know FBC is not relying on the HW for flips, but this is on the optimization TODO list after we finally merge the bug fixes) Due to the benefits of relying on the HW tracking, I think you'll have to bring some good arguments to sell this patch to me. But a Testcase: tag would totally do it :) I'm not convinced without this patch we can mask again the LPSP, otherwise that issue Mathew Garret faced with strange scroll on firefox on Gnome will be seen, but with LPSP we have a higher risk of unrecoverable screens. There is no test case for visual feeling. I could reproduce what Mathew had seen when opened firefox in a gnome and scroolled down and up a big page... I'm not able to create an IGT that gets this kind of feeling, sorry... Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_psr.c | 22 +++--- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 92e2b467..63bbab2 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -719,25 +719,9 @@ void intel_psr_flush(struct drm_device *dev, frontbuffer_bits = INTEL_FRONTBUFFER_ALL_MASK(pipe); dev_priv-psr.busy_frontbuffer_bits = ~frontbuffer_bits; - if (HAS_DDI(dev)) { - /* -* By definition every flush should mean invalidate + flush, -* however on core platforms let's minimize the -* disable/re-enable so we can avoid the invalidate when flip -* originated the flush. -*/ - if (frontbuffer_bits origin != ORIGIN_FLIP) - intel_psr_exit(dev); - } else { - /* -* On Valleyview and Cherryview we don't use hardware tracking -* so any plane updates or cursor moves don't result in a PSR -* invalidating. Which means we need to manually fake this in -* software for all flushes. -*/ - if (frontbuffer_bits) - intel_psr_exit(dev); - } + /* By definition flush = invalidate + flush */ + if (frontbuffer_bits) + intel_psr_exit(dev); if (!dev_priv-psr.active !dev_priv -psr.busy_frontbuffer_bits) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] drm/i915: Delay first PSR activation.
On Mon, 2015-08-24 at 17:03 +, Zanoni, Paulo R wrote: Em Qui, 2015-08-20 às 17:55 -0700, Rodrigo Vivi escreveu: This affects PSR on VLV, CHV, HSW and BDW. When debuging the frozen screen caused by HW tracking with low power state I noticed that if we keep moving the mouse non stop you will miss the screen updates for a while. At least until we stop moving the mouse for a small time and move again. The actual enabling should happen immediately after Display Port enabling sequence finished with links trained and everything enabled. However we face many issues when enabling PSR right after a modeset. On VLV/CHV we face blank screens on this scenario and on HSW+ we face a recoverable frozen screen, at least until next exit-activate sequence. Another workaround for the same issue here would be to increase re-enable idle time from 100 to 500 as we did for VLV/CHV. However this patch workaround this issue in a better way since it doesn't reduce PSR residency and also allow us to reduce the delay time between re-enables at least on VLV/CHV. It sounds like this could use a little more debugging. Have we tried moving the psr enable to after intel_post_plane_update()? I'm not sure if it is a good idea to enable psr to late directly, on some attempts to enable psr late I was having trouble with fbdev cases... but I could try... but also here we would need to be carefull to avoid multiple unecessary calls to psr_enable... The move the cursor to reproduce bug sounds very weird. yes, it is weird indeed. And hard to explain unless you are seeing it, so if you want I could upload a video somewhere to show you... Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_psr.c | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index d02d4e2..2be4a62 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -427,6 +427,19 @@ void intel_psr_enable(struct intel_dp *intel_dp) vlv_psr_enable_source(intel_dp); } + /* +* FIXME: Activation should happen immediately since this function +* is just called after pipe is fully trained and enabled. +* However on previous platforms we face issues when first activation +* follows a modeset so quickly. +* - On VLV/CHV we get bank screen on first activation +* - On HSW/BDW we get a recoverable frozen screen until next +* exit-activate sequence. +*/ + if (INTEL_INFO(dev)-gen 9) + schedule_delayed_work(dev_priv-psr.work, + msecs_to_jiffies(500)); + dev_priv-psr.enabled = intel_dp; unlock: mutex_unlock(dev_priv-psr.lock); @@ -729,8 +742,9 @@ void intel_psr_flush(struct drm_device *dev, intel_psr_exit(dev); if (!dev_priv-psr.active !dev_priv -psr.busy_frontbuffer_bits) - schedule_delayed_work(dev_priv-psr.work, - msecs_to_jiffies(delay_ms)); + if (!work_busy(dev_priv-psr.work.work)) + schedule_delayed_work(dev_priv-psr.work, + msecs_to_jiffies(delay_ms)); mutex_unlock(dev_priv-psr.lock); } ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx