[Intel-gfx] [PATCH 1/3] x86-32: Teach copy_from_user to unroll .size=6/8
Two exception handling register moves are faster to inline than a call to __copy_user_ll(). We already apply the conversion for a get_user() call, so for symmetry we should also apply the optimisation to copy_from_user. Signed-off-by: Chris Wilson Cc: Thomas Gleixner Cc: Ingo Molnar Cc: "H. Peter Anvin" --- arch/x86/include/asm/uaccess_32.h | 25 + 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/arch/x86/include/asm/uaccess_32.h b/arch/x86/include/asm/uaccess_32.h index aeda9bb8af50..44d17d1ab07c 100644 --- a/arch/x86/include/asm/uaccess_32.h +++ b/arch/x86/include/asm/uaccess_32.h @@ -23,30 +23,47 @@ static __always_inline unsigned long raw_copy_from_user(void *to, const void __user *from, unsigned long n) { if (__builtin_constant_p(n)) { - unsigned long ret; + unsigned long ret = 0; switch (n) { case 1: - ret = 0; __uaccess_begin(); __get_user_asm_nozero(*(u8 *)to, from, ret, "b", "b", "=q", 1); __uaccess_end(); return ret; case 2: - ret = 0; __uaccess_begin(); __get_user_asm_nozero(*(u16 *)to, from, ret, "w", "w", "=r", 2); __uaccess_end(); return ret; case 4: - ret = 0; __uaccess_begin(); __get_user_asm_nozero(*(u32 *)to, from, ret, "l", "k", "=r", 4); __uaccess_end(); return ret; + case 6: + __uaccess_begin(); + __get_user_asm_nozero(*(u32 *)to, from, ret, + "l", "k", "=r", 6); + if (likely(!ret)) + __get_user_asm_nozero(*(u16 *)(4 + (char *)to), + (u16 __user *)(4 + (char __user *)from), + ret, "w", "w", "=r", 2); + __uaccess_end(); + return ret; + case 8: + __uaccess_begin(); + __get_user_asm_nozero(*(u32 *)to, from, ret, + "l", "k", "=r", 8); + if (likely(!ret)) + __get_user_asm_nozero(*(u32 *)(4 + (char *)to), + (u32 __user *)(4 + (char __user *)from), + ret, "l", "k", "=r", 4); + __uaccess_end(); + return ret; } } return __copy_user_ll(to, (__force const void *)from, n); -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] x86-32: Expand static copy_to_user()
For known compile-time fixed sizes, teach x86-32 copy_to_user() to convert them to the simpler put_user and inline it similar to the optimisation applied to copy_from_user() and already used by x86-64. Signed-off-by: Chris Wilson Cc: Thomas Gleixner Cc: Ingo Molnar Cc: "H. Peter Anvin" --- arch/x86/include/asm/uaccess_32.h | 48 +++ 1 file changed, 48 insertions(+) diff --git a/arch/x86/include/asm/uaccess_32.h b/arch/x86/include/asm/uaccess_32.h index 44d17d1ab07c..a02aa9db34ed 100644 --- a/arch/x86/include/asm/uaccess_32.h +++ b/arch/x86/include/asm/uaccess_32.h @@ -16,6 +16,54 @@ unsigned long __must_check __copy_from_user_ll_nocache_nozero static __always_inline unsigned long __must_check raw_copy_to_user(void __user *to, const void *from, unsigned long n) { + if (__builtin_constant_p(n)) { + unsigned long ret = 0; + + switch (n) { + case 1: + __uaccess_begin(); + __put_user_asm(*(u8 *)from, to, ret, + "b", "b", "iq", 1); + __uaccess_end(); + return ret; + case 2: + __uaccess_begin(); + __put_user_asm(*(u16 *)from, to, ret, + "w", "w", "ir", 2); + __uaccess_end(); + return ret; + case 4: + __uaccess_begin(); + __put_user_asm(*(u32 *)from, to, ret, + "l", "k", "ir", 4); + __uaccess_end(); + return ret; + case 6: + __uaccess_begin(); + __put_user_asm(*(u32 *)from, to, ret, + "l", "k", "ir", 4); + if (likely(!ret)) { + asm("":::"memory"); + __put_user_asm(*(u16 *)(4 + (char *)from), + (u16 __user *)(4 + (char __user *)to), + ret, "w", "w", "ir", 2); + } + __uaccess_end(); + return ret; + case 8: + __uaccess_begin(); + __put_user_asm(*(u32 *)from, to, ret, + "l", "k", "ir", 4); + if (likely(!ret)) { + asm("":::"memory"); + __put_user_asm(*(u32 *)(4 + (char *)from), + (u32 __user *)(4 + (char __user *)to), + ret, "l", "k", "ir", 4); + } + __uaccess_end(); + return ret; + } + } return __copy_user_ll((__force void *)to, from, n); } -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] x86-64: Inline 6/12 byte copy_user
Extend the list of replacements for compile-time known sizes to include 6/12 byte copies. These expand to two movs (along with their exception table) and are cheaper to inline than the function call (similar to the 10 byte copy already handled). Signed-off-by: Chris Wilson Cc: Thomas Gleixner Cc: Ingo Molnar Cc: "H. Peter Anvin" --- arch/x86/include/asm/uaccess_64.h | 42 +++ 1 file changed, 42 insertions(+) diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h index c5504b9a472e..ff2d65baa988 100644 --- a/arch/x86/include/asm/uaccess_64.h +++ b/arch/x86/include/asm/uaccess_64.h @@ -71,6 +71,16 @@ raw_copy_from_user(void *dst, const void __user *src, unsigned long size) ret, "l", "k", "=r", 4); __uaccess_end(); return ret; + case 6: + __uaccess_begin(); + __get_user_asm_nozero(*(u32 *)dst, (u32 __user *)src, + ret, "l", "k", "=r", 6); + if (likely(!ret)) + __get_user_asm_nozero(*(u16 *)(4 + (char *)dst), + (u16 __user *)(4 + (char __user *)src), + ret, "w", "w", "=r", 2); + __uaccess_end(); + return ret; case 8: __uaccess_begin(); __get_user_asm_nozero(*(u64 *)dst, (u64 __user *)src, @@ -87,6 +97,16 @@ raw_copy_from_user(void *dst, const void __user *src, unsigned long size) ret, "w", "w", "=r", 2); __uaccess_end(); return ret; + case 12: + __uaccess_begin(); + __get_user_asm_nozero(*(u64 *)dst, (u64 __user *)src, + ret, "q", "", "=r", 10); + if (likely(!ret)) + __get_user_asm_nozero(*(u32 *)(8 + (char *)dst), + (u32 __user *)(8 + (char __user *)src), + ret, "l", "k", "=r", 4); + __uaccess_end(); + return ret; case 16: __uaccess_begin(); __get_user_asm_nozero(*(u64 *)dst, (u64 __user *)src, @@ -128,6 +148,17 @@ raw_copy_to_user(void __user *dst, const void *src, unsigned long size) ret, "l", "k", "ir", 4); __uaccess_end(); return ret; + case 6: + __uaccess_begin(); + __put_user_asm(*(u32 *)src, (u32 __user *)dst, + ret, "l", "k", "ir", 6); + if (likely(!ret)) { + asm("":::"memory"); + __put_user_asm(2[(u16 *)src], 2 + (u16 __user *)dst, + ret, "w", "w", "ir", 2); + } + __uaccess_end(); + return ret; case 8: __uaccess_begin(); __put_user_asm(*(u64 *)src, (u64 __user *)dst, @@ -145,6 +176,17 @@ raw_copy_to_user(void __user *dst, const void *src, unsigned long size) } __uaccess_end(); return ret; + case 12: + __uaccess_begin(); + __put_user_asm(*(u64 *)src, (u64 __user *)dst, + ret, "q", "", "er", 12); + if (likely(!ret)) { + asm("":::"memory"); + __put_user_asm(2[(u32 *)src], 2 + (u32 __user *)dst, + ret, "l", "k", "ir", 4); + } + __uaccess_end(); + return ret; case 16: __uaccess_begin(); __put_user_asm(*(u64 *)src, (u64 __user *)dst, -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] x86-32: Teach copy_from_user to unroll .size=6/8
== Series Details == Series: series starting with [1/3] x86-32: Teach copy_from_user to unroll .size=6/8 URL : https://patchwork.freedesktop.org/series/25148/ State : success == Summary == Series 25148v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/25148/revisions/1/mbox/ Test kms_busy: Subgroup basic-flip-default-a: dmesg-warn -> PASS (fi-skl-6700hq) fdo#101144 +3 Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: fail -> PASS (fi-skl-6700hq) fdo#101154 +7 fdo#101144 https://bugs.freedesktop.org/show_bug.cgi?id=101144 fdo#101154 https://bugs.freedesktop.org/show_bug.cgi?id=101154 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:453s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:432s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:568s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:508s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:488s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:482s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:437s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:415s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:415s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:496s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:466s fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 time:465s fi-kbl-7560u total:278 pass:263 dwarn:5 dfail:0 fail:0 skip:10 time:572s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:455s fi-skl-6700hqtotal:278 pass:239 dwarn:0 dfail:1 fail:17 skip:21 time:430s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:469s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:506s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:436s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:531s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:403s 923422469cbe33b47269502ce79caa6c307f41ad drm-tip: 2017y-06m-01d-06h-07m-33s UTC integration manifest 471d3b9 x86-64: Inline 6/12 byte copy_user b8b7388 x86-32: Expand static copy_to_user() 5966f1a x86-32: Teach copy_from_user to unroll .size=6/8 == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4852/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915/skl+: consider max supported plane pixel rate while scaling
Op 26-05-17 om 17:15 schreef Mahesh Kumar: > A display resolution is only supported if it meets all the restrictions > below for Maximum Pipe Pixel Rate. > > The display resolution must fit within the maximum pixel rate output > from the pipe. Make sure that the display pipe is able to feed pixels at > a rate required to support the desired resolution. > For each enabled plane on the pipe { > If plane scaling enabled { > Horizontal down scale amount = Maximum[1, plane horizontal size / > scaler horizontal window size] > Vertical down scale amount = Maximum[1, plane vertical size / > scaler vertical window size] > Plane down scale amount = Horizontal down scale amount * > Vertical down scale amount > Plane Ratio = 1 / Plane down scale amount > } > Else { > Plane Ratio = 1 > } > If plane source pixel format is 64 bits per pixel { > Plane Ratio = Plane Ratio * 8/9 > } > } > > Pipe Ratio = Minimum Plane Ratio of all enabled planes on the pipe > > If pipe scaling is enabled { > Horizontal down scale amount = Maximum[1, pipe horizontal source size / > scaler horizontal window size] > Vertical down scale amount = Maximum[1, pipe vertical source size / > scaler vertical window size] > Note: The progressive fetch - interlace display mode is equivalent to a > 2.0 vertical down scale > Pipe down scale amount = Horizontal down scale amount * > Vertical down scale amount > Pipe Ratio = Pipe Ratio / Pipe down scale amount > } > > Pipe maximum pixel rate = CDCLK frequency * Pipe Ratio > > In this patch our calculation is based on pipe downscale amount > (plane max downscale amount * pipe downscale amount) instead of Pipe > Ratio. So, > max supported crtc clock with given scaling = CDCLK / pipe downscale. > Flip will fail if, > current crtc clock > max supported crct clock with given scaling. > > Changes since V1: > - separate out fixed_16_16 wrapper API definition > Changes since V2: > - Fix buggy crtc !active condition (Maarten) > - use intel_wm_plane_visible wrapper as per Maarten's suggestion > Changes since V3: > - Change failure return from ERANGE to EINVAL > Changes since V4: > - Rebase based on previous patch changes > Changes since V5: > - return EINVAL instead of continue (Maarten) > Changes since V6: > - Improve commit message > - Address review comment > Changes since V7: > - use !enable instead of !active > - rename config variable for consistency (Maarten) > > Signed-off-by: Mahesh Kumar > Reviewed-by: Matt Roper > Reviewed-by: Maarten Lankhorst > --- > drivers/gpu/drm/i915/intel_display.c | 3 ++ > drivers/gpu/drm/i915/intel_drv.h | 2 + > drivers/gpu/drm/i915/intel_pm.c | 87 > > 3 files changed, 92 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 7fa21df5bcd7..fb2e43d3eb5b 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11177,6 +11177,9 @@ static int intel_crtc_atomic_check(struct drm_crtc > *crtc, > ret = skl_update_scaler_crtc(pipe_config); > > if (!ret) > + ret = skl_check_pipe_max_pixel_rate(intel_crtc, > + pipe_config); > + if (!ret) > ret = intel_atomic_setup_scalers(dev_priv, intel_crtc, >pipe_config); > } > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index bd500977b3fc..93afac4a83fa 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1885,6 +1885,8 @@ bool skl_ddb_allocation_overlaps(const struct > skl_ddb_entry **entries, >int ignore); > bool ilk_disable_lp_wm(struct drm_device *dev); > int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6); > +int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc, > + struct intel_crtc_state *cstate); > static inline int intel_enable_rc6(void) > { > return i915.enable_rc6; > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 46aca5cbc047..ead346f1deb2 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3868,6 +3868,93 @@ skl_plane_downscale_amount(const struct > intel_crtc_state *cstate, > return mul_fixed16(downscale_w, downscale_h); > } > > +static uint_fixed_16_16_t > +skl_pipe_downscale_amount(const struct intel_crtc_state *crtc_state) > +{ > + uint_fixed_16_16_t pipe_downscale = u32_to_fixed_16_16(1); > + > + if (!crtc_state->base.enable) > + return pipe_downscale; > + > + if (crtc_state
Re: [Intel-gfx] [PATCH] drm/i915: Guard against i915_ggtt_disable_guc() being invoked unconditionally
On Wed, May 31, 2017 at 06:01:10PM -0700, Michel Thierry wrote: > On 5/31/2017 12:05 PM, Chris Wilson wrote: > >Commit 7c3f86b6dc51 ("drm/i915: Invalidate the guc ggtt TLB upon > >insertion") added the restoration of the invalidation routine after the > >GuC was disabled, but missed that the GuC was unconditionally disabled > >when not used. This then overwrites the invalidate routine for the older > >chipsets, causing havoc and breaking resume as the most obvious victim. > > > >We place the guard inside i915_ggtt_disable_guc() to be backport > >friendly (the bug was introduced into v4.11) but it would be preferred > >to be in more control over when this was guard (i.e. do not try and > >teardown the data structures before we have enabled them). That should > >be true with the reorganisation of the guc loaders. > > > >Reported-by: Ville Syrjälä > >Signed-off-by: Chris Wilson > >Fixes: 7c3f86b6dc51 ("drm/i915: Invalidate the guc ggtt TLB upon insertion") > >Cc: Tvrtko Ursulin > >Cc: Joonas Lahtinen > >Cc: Oscar Mateo > >Cc: Daniele Ceraolo Spurio > >Cc: Michal Wajdeczko > >Cc: Arkadiusz Hiler > >Cc: # v4.11+ > Reviewed-by: Michel Thierry > > btw the bug can only happen in 4.11; in 4.12+ this safeguard is > already redundant since enable_guc_loading should be zero and > ggtt_disable_guc is never called. Thanks for the review, pushed so we can get it back to where it needs to go. I'll send a patch to convert this to an assert in a moment. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/guc: Assert that we switch between known ggtt->invalidate functions
When we enable the GuC, we enable an alternative mechanism for doing post-GGTT update invalidation. Likewise, when we disable the GuC, we restore the previous method. Assert that we change between known endpoints, so that we can catch if we accidentally clobber some other gen and if we change the invalidate routine without updating guc. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Joonas Lahtinen Cc: Oscar Mateo Cc: Daniele Ceraolo Spurio Cc: Michal Wajdeczko Cc: Arkadiusz Hiler Cc: Michel Thierry --- drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 1489c3af7145..4ff854e6413c 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -3095,13 +3095,17 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv) void i915_ggtt_enable_guc(struct drm_i915_private *i915) { + GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate); + i915->ggtt.invalidate = guc_ggtt_invalidate; } void i915_ggtt_disable_guc(struct drm_i915_private *i915) { - if (i915->ggtt.invalidate == guc_ggtt_invalidate) - i915->ggtt.invalidate = gen6_ggtt_invalidate; + /* We should only be called after i915_ggtt_enable_guc() */ + GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate); + + i915->ggtt.invalidate = gen6_ggtt_invalidate; } void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv) -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 5/6] drm/i915/gvt: Dmabuf support for GVT-g
On Sat, May 27, 2017 at 04:38:51PM +0800, Xiaoguang Chen wrote: > dmabuf for GVT-g can be exported to users who can use the dmabuf to show > the desktop of vm which use intel vgpu. > > Currently we provide query and create new dmabuf operations. > > Users of dmabuf can cache some created dmabufs and related information > such as the framebuffer's address, size, tiling mode, width, height etc. > When refresh the screen first query the currnet vgpu's frambuffer and > compare with the cached ones(address, size, tiling, width, height etc) > if found one then reuse the found dmabuf to gain performance improvment. > > If there is no dmabuf created yet or not found in the cached dmabufs then > need to create a new dmabuf. To create a dmabuf first a gem object will > be created and the backing storage of this gem object is the vgpu's > framebuffer(primary/cursor). > Set caching mode, change tiling mode and set domains of this gem object > is not supported. > Then associate this gem object to a dmabuf and export this dmabuf. > A file descriptor will be generated for this dmabuf and this file > descriptor can be sent to user space to do display. > > Signed-off-by: Xiaoguang Chen > --- > drivers/gpu/drm/i915/gvt/Makefile | 2 +- > drivers/gpu/drm/i915/gvt/dmabuf.c | 269 > + > drivers/gpu/drm/i915/gvt/dmabuf.h | 37 + > drivers/gpu/drm/i915/gvt/gvt.h | 1 + > drivers/gpu/drm/i915/i915_gem.c| 26 +++- > drivers/gpu/drm/i915/i915_gem_object.h | 9 ++ > 6 files changed, 342 insertions(+), 2 deletions(-) > create mode 100644 drivers/gpu/drm/i915/gvt/dmabuf.c > create mode 100644 drivers/gpu/drm/i915/gvt/dmabuf.h > > diff --git a/drivers/gpu/drm/i915/gvt/Makefile > b/drivers/gpu/drm/i915/gvt/Makefile > index 192ca26..e480f7d 100644 > --- a/drivers/gpu/drm/i915/gvt/Makefile > +++ b/drivers/gpu/drm/i915/gvt/Makefile > @@ -2,7 +2,7 @@ GVT_DIR := gvt > GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o > firmware.o \ > interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \ > execlist.o scheduler.o sched_policy.o render.o cmd_parser.o \ > - fb_decoder.o > + fb_decoder.o dmabuf.o > > ccflags-y+= -I$(src) -I$(src)/$(GVT_DIR) -Wall > i915-y += $(addprefix $(GVT_DIR)/, > $(GVT_SOURCE)) > diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c > b/drivers/gpu/drm/i915/gvt/dmabuf.c > new file mode 100644 > index 000..c831e91 > --- /dev/null > +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c > @@ -0,0 +1,269 @@ > +/* > + * Copyright 2017 Intel Corporation. All rights reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > + * DEALINGS IN THE SOFTWARE. > + * > + * Authors: > + *Zhiyuan Lv > + * > + * Contributors: > + *Xiaoguang Chen > + */ > + > +#include > +#include > +#include > + > +#include "i915_drv.h" > +#include "gvt.h" > + > +#define GEN8_DECODE_PTE(pte) (pte & GENMASK_ULL(63, 12)) > + > +static struct sg_table *intel_vgpu_gem_get_pages( > + struct drm_i915_gem_object *obj) > +{ > + struct drm_i915_private *dev_priv = to_i915(obj->base.dev); > + struct sg_table *st; > + struct scatterlist *sg; > + int i, ret; > + gen8_pte_t __iomem *gtt_entries; > + struct intel_vgpu_fb_info *fb_info; > + > + fb_info = (struct intel_vgpu_fb_info *)obj->gvt_info; > + if (WARN_ON(!fb_info)) > + return ERR_PTR(-ENODEV); > + > + st = kmalloc(sizeof(*st), GFP_KERNEL); > + if (!st) { > + ret = -ENOMEM; > + return ERR_PTR(ret); Tidier. > + } > + > + ret = sg_alloc_table(st, fb_info->fb_size, GFP_KERNEL); > + if (ret) { > + kfree(st); > + return ERR_PTR(ret); > + } > + gtt_entries = (gen8_pte_t __iomem *)dev_priv->ggtt.gsm + > + (fb_i
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/guc: Assert that we switch between known ggtt->invalidate functions
== Series Details == Series: drm/i915/guc: Assert that we switch between known ggtt->invalidate functions URL : https://patchwork.freedesktop.org/series/25150/ State : success == Summary == Series 25150v1 drm/i915/guc: Assert that we switch between known ggtt->invalidate functions https://patchwork.freedesktop.org/api/1.0/series/25150/revisions/1/mbox/ fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:444s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:432s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:585s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:512s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:488s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:477s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:431s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:412s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:419s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:489s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:464s fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 time:467s fi-kbl-7560u total:278 pass:263 dwarn:5 dfail:0 fail:0 skip:10 time:569s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:455s fi-skl-6700hqtotal:278 pass:239 dwarn:0 dfail:1 fail:17 skip:21 time:435s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:469s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:502s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:434s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:542s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:407s b479b5c889c24e5a7cf4beb064f0037fdc2d568d drm-tip: 2017y-06m-01d-08h-55m-16s UTC integration manifest 675b5c6 drm/i915/guc: Assert that we switch between known ggtt->invalidate functions == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4853/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 18/37] drm/exynos: Drop drm_vblank_cleanup
On Thu, Jun 01, 2017 at 03:15:13PM +0900, Inki Dae wrote: > > > 2017년 05월 31일 17:45에 Daniel Vetter 이(가) 쓴 글: > > On Tue, May 30, 2017 at 09:03:34AM +0900, Inki Dae wrote: > >> Hi Daniel, > >> > >> 2017년 05월 24일 23:51에 Daniel Vetter 이(가) 쓴 글: > >>> Only in the load failure path, where the hardware is quiet anyway. > >>> > >>> Cc: Inki Dae > >>> Cc: Joonyoung Shim > >>> Cc: Seung-Woo Kim > >>> Cc: Kyungmin Park > >>> Signed-off-by: Daniel Vetter > >>> --- > >>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 4 +--- > >>> 1 file changed, 1 insertion(+), 3 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c > >>> b/drivers/gpu/drm/exynos/exynos_drm_drv.c > >>> index 50294a7bd29d..1c814b9342af 100644 > >>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c > >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c > >>> @@ -376,7 +376,7 @@ static int exynos_drm_bind(struct device *dev) > >>> /* Probe non kms sub drivers and virtual display driver. */ > >>> ret = exynos_drm_device_subdrv_probe(drm); > >>> if (ret) > >>> - goto err_cleanup_vblank; > >>> + goto err_unbind_all; > >> > >> With this change shouldn't you post the patch to remove drm_vblank_init > >> and setup vblank stuff in drm_crtc_init together? > >> I couldn't find the relevant patch on your patch series[1]. > > > > No, drm_vblank_cleanup is already called in the core. The only reason to > > call it in the driver is to fall back from kms to ums when irq setup > > somehow failed, then you need to disable vblank support again. > > > > The only driver which ever needed this was radeon, and radeon long ago > > lost ums support. drm_vblank_cleanup is already called for you, and most > > drivers don't even do this cleanup call. But somehow a lot of people > > copied from radeon without understanding what it does. > > > > Looking at the last patch in this series might help a bit in understanding > > the history here. Can you pls reevaluate the patch? > > > Thanks for explaination. Confirmed. > > Reviewed-by: Inki Dae Thanks for the review, patch applied to drm-misc-next. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/15] drm/i915: introduce gem object page_sizes
On Wed, May 31, 2017 at 07:51:59PM +0100, Matthew Auld wrote: > + max_page_size = BIT(fls64(obj->mm.page_sizes.sg)-1); > + > + /* If were are actually dealing with a single page-size, mark it so */ > + if (IS_ALIGNED(obj->mm.page_sizes.phys, max_page_size)) > + obj->mm.page_sizes.sg = max_page_size; Pardon? If the physical page sizes is a multiple of the max sg->length (rounded to GTT page sizes), only use the max? page_sizes.sg will already express the smallest unit that can be used to for the whole object (as well as the largest one that may be used opportunistically). The current kselftests that mix sg chunk sizes are ignorant of the large pages sizes, it would be useful to add some that did mix huge pages. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/5] ACPI / APEI: Switch to use new generic UUID API
On Thu, Jun 1, 2017 at 2:56 AM, kbuild test robot wrote: > Hi Andy, > > [auto build test ERROR on pm/linux-next] > [also build test ERROR on v4.12-rc3 next-20170531] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] Dropped ahead. As cover letter says it is based on top of uuid tree. -- With Best Regards, Andy Shevchenko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/15] drm/i915: align 64K objects to 2M
On Wed, May 31, 2017 at 07:52:01PM +0100, Matthew Auld wrote: > We can't mix 64K and 4K pte's in the same page-table, so for now we > align 64K objects to 2M to avoid any potential mixing. This is > potentially wasteful but in reality shouldn't be too bad since this only > applies to the virtual address space of a 48b PPGTT. > > Suggested-by: Chris Wilson > Signed-off-by: Matthew Auld > Cc: Joonas Lahtinen > --- > drivers/gpu/drm/i915/i915_vma.c | 18 ++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c > index c355ccb01872..af950d92fa13 100644 > --- a/drivers/gpu/drm/i915/i915_vma.c > +++ b/drivers/gpu/drm/i915/i915_vma.c > @@ -479,6 +479,15 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 > alignment, u64 flags) > goto err_unpin; > } > > + /* A current limitation in our implementation is that 64K > + * objects must be aligned to 2M, and given that we can't > + * enforce this for soft pinning, we need to fallback to normal > + * pages if don't meet this restriction. > + */ > + if (obj->mm.page_sizes.sg == I915_GTT_PAGE_SIZE_64K && page_sizes.sg > I915_GTT_PAGE_SIZE_4K. > + !IS_ALIGNED(offset | size, I915_GTT_PAGE_SIZE_2M)) Abusing the semantics of PAGE_SIZE_2M. It just happens to be the value you want, but it doesn't mean what you say. > + obj->mm.page_sizes.sg = I915_GTT_PAGE_SIZE; > + > ret = i915_gem_gtt_reserve(vma->vm, &vma->node, > size, offset, obj->cache_level, > flags); > @@ -493,6 +502,15 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 > alignment, u64 flags) > if (!is_power_of_2(page_alignment)) > page_alignment = BIT(fls64(page_alignment)-1); > > + /* We can't mix 64K and 4K pte's in the same page-table > (2M > + * block), and so to avoid the ugliness and complexity > of > + * coloring we opt for just aligning 64K objects to 2M. > + */ > + if (obj->mm.page_sizes.sg == I915_GTT_PAGE_SIZE_64K) { page_sizes.sg & SIZE_64K or page_sizes.sg > SIZE_4K Also refer to earlier discussion that this should be vma->page_sizes here not obj. > + page_alignment = I915_GTT_PAGE_SIZE_2M; > + size = roundup(size, page_alignment); round_up. > + } > + > alignment = max_t(typeof(alignment), alignment, > page_alignment); > } > -- > 2.9.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/15] drm/i915: disable GTT cache for 2M/1G pages
On Wed, May 31, 2017 at 07:52:04PM +0100, Matthew Auld wrote: > When SW enables the use of 2M/1G pages, it must disable the GTT cache. > > Signed-off-by: Matthew Auld > Cc: Joonas Lahtinen > --- > drivers/gpu/drm/i915/intel_pm.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 936eef1634c7..496b64f65eb2 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -8195,10 +8195,10 @@ static void broadwell_init_clock_gating(struct > drm_i915_private *dev_priv) > > /* >* WaGttCachingOffByDefault:bdw > - * GTT cache may not work with big pages, so if those > - * are ever enabled GTT cache may need to be disabled. > + * The GTT cache must be disabled if the system is planning to use > + * 2M/1G pages. >*/ > - I915_WRITE(HSW_GTT_CACHE_EN, GTT_CACHE_EN_ALL); > + I915_WRITE(HSW_GTT_CACHE_EN, 0); > > /* WaKVMNotificationOnConfigChange:bdw */ > I915_WRITE(CHICKEN_PAR2_1, I915_READ(CHICKEN_PAR2_1) > @@ -8474,10 +8474,10 @@ static void cherryview_init_clock_gating(struct > drm_i915_private *dev_priv) > gen8_set_l3sqc_credits(dev_priv, 38, 2); > > /* > - * GTT cache may not work with big pages, so if those > - * are ever enabled GTT cache may need to be disabled. > + * The GTT cache must be disabled if the system is planning to use > + * 2M/1G pages. >*/ Cherryview doesn't support 48bit ppgtt so isn't using huge pages. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Assert that we switch between known ggtt->invalidate functions
On Thu, Jun 01, 2017 at 10:04:46AM +0100, Chris Wilson wrote: > When we enable the GuC, we enable an alternative mechanism for doing > post-GGTT update invalidation. Likewise, when we disable the GuC, we > restore the previous method. Assert that we change between known > endpoints, so that we can catch if we accidentally clobber some other > gen and if we change the invalidate routine without updating guc. > > Signed-off-by: Chris Wilson > Cc: Tvrtko Ursulin > Cc: Joonas Lahtinen > Cc: Oscar Mateo > Cc: Daniele Ceraolo Spurio > Cc: Michal Wajdeczko > Cc: Arkadiusz Hiler > Cc: Michel Thierry > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 1489c3af7145..4ff854e6413c 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -3095,13 +3095,17 @@ int i915_ggtt_enable_hw(struct drm_i915_private > *dev_priv) > > void i915_ggtt_enable_guc(struct drm_i915_private *i915) > { > + GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate); > + > i915->ggtt.invalidate = guc_ggtt_invalidate; > } > > void i915_ggtt_disable_guc(struct drm_i915_private *i915) > { > - if (i915->ggtt.invalidate == guc_ggtt_invalidate) > - i915->ggtt.invalidate = gen6_ggtt_invalidate; > + /* We should only be called after i915_ggtt_enable_guc() */ > + GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate); > + > + i915->ggtt.invalidate = gen6_ggtt_invalidate; > } While this looks correct today, it may not work in the future if we will need somethig other than gen6_ggtt_invalidate() as base invalidate function or guc_gtt_invalidate() as the one for the guc. Just a head up. Reviewed-by: Michal Wajdeczko > > void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv) > -- > 2.11.0 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Assert that we switch between known ggtt->invalidate functions
On Thu, Jun 01, 2017 at 12:03:18PM +0200, Michal Wajdeczko wrote: > On Thu, Jun 01, 2017 at 10:04:46AM +0100, Chris Wilson wrote: > > When we enable the GuC, we enable an alternative mechanism for doing > > post-GGTT update invalidation. Likewise, when we disable the GuC, we > > restore the previous method. Assert that we change between known > > endpoints, so that we can catch if we accidentally clobber some other > > gen and if we change the invalidate routine without updating guc. > > > > Signed-off-by: Chris Wilson > > Cc: Tvrtko Ursulin > > Cc: Joonas Lahtinen > > Cc: Oscar Mateo > > Cc: Daniele Ceraolo Spurio > > Cc: Michal Wajdeczko > > Cc: Arkadiusz Hiler > > Cc: Michel Thierry > > --- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > > b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index 1489c3af7145..4ff854e6413c 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -3095,13 +3095,17 @@ int i915_ggtt_enable_hw(struct drm_i915_private > > *dev_priv) > > > > void i915_ggtt_enable_guc(struct drm_i915_private *i915) > > { > > + GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate); > > + > > i915->ggtt.invalidate = guc_ggtt_invalidate; > > } > > > > void i915_ggtt_disable_guc(struct drm_i915_private *i915) > > { > > - if (i915->ggtt.invalidate == guc_ggtt_invalidate) > > - i915->ggtt.invalidate = gen6_ggtt_invalidate; > > + /* We should only be called after i915_ggtt_enable_guc() */ > > + GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate); > > + > > + i915->ggtt.invalidate = gen6_ggtt_invalidate; > > } > > While this looks correct today, it may not work in the future if we > will need somethig other than gen6_ggtt_invalidate() as base invalidate > function or guc_gtt_invalidate() as the one for the guc. Just a head up. That's one of the reasons for adding it. So that if we forget to make the change, it gets caught. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Guard against i915_ggtt_disable_guc() being invoked unconditionally
On ke, 2017-05-31 at 20:05 +0100, Chris Wilson wrote: > Commit 7c3f86b6dc51 ("drm/i915: Invalidate the guc ggtt TLB upon > insertion") added the restoration of the invalidation routine after the > GuC was disabled, but missed that the GuC was unconditionally disabled > when not used. This then overwrites the invalidate routine for the older > chipsets, causing havoc and breaking resume as the most obvious victim. > > We place the guard inside i915_ggtt_disable_guc() to be backport > friendly (the bug was introduced into v4.11) but it would be preferred > to be in more control over when this was guard (i.e. do not try and > teardown the data structures before we have enabled them). That should > be true with the reorganisation of the guc loaders. > > Reported-by: Ville Syrjälä > Signed-off-by: Chris Wilson > Fixes: 7c3f86b6dc51 ("drm/i915: Invalidate the guc ggtt TLB upon insertion") > Cc: Tvrtko Ursulin > Cc: Joonas Lahtinen > Cc: Oscar Mateo > Cc: Daniele Ceraolo Spurio > Cc: Michal Wajdeczko > Cc: Arkadiusz Hiler > Cc: # v4.11+ Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Assert that we switch between known ggtt->invalidate functions
On to, 2017-06-01 at 12:03 +0200, Michal Wajdeczko wrote: > On Thu, Jun 01, 2017 at 10:04:46AM +0100, Chris Wilson wrote: > > > > void i915_ggtt_disable_guc(struct drm_i915_private *i915) > > { > > - if (i915->ggtt.invalidate == guc_ggtt_invalidate) > > - i915->ggtt.invalidate = gen6_ggtt_invalidate; > > + /* We should only be called after i915_ggtt_enable_guc() */ > > + GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate); > > + > > + i915->ggtt.invalidate = gen6_ggtt_invalidate; > > } > > While this looks correct today, it may not work in the future if we > will need somethig other than gen6_ggtt_invalidate() as base invalidate > function or guc_gtt_invalidate() as the one for the guc. Just a head up. Currently the assignment is directly to gen6_ggtt_invalidate, no questions asked. So I don't think the assert could be much more :) Maybe GuC code should backup the invalidate function before overriding. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/15] drm/i915: enable THP for gemfs
On ke, 2017-05-31 at 19:51 +0100, Matthew Auld wrote: > Enable transparent-huge-pages through gemfs by mounting with > huge=within_size. > > Signed-off-by: Matthew Auld > Cc: Joonas Lahtinen > Cc: Chris Wilson > @@ -41,6 +42,28 @@ struct vfsmount *i915_gemfs_create(void) > > gemfs_mnt = kern_mount(type); > > +#if defined(CONFIG_TRANSPARENT_HUGE_PAGECACHE) > + if (!IS_ERR(gemfs_mnt) && has_transparent_hugepage()) { > + struct super_block *sb = gemfs_mnt->mnt_sb; > + char options[] = "huge=within_size"; > + int flags = 0; > + int ret; > + > + /* Idealy we would just pass the mount options when mounting, > + * but for some reason shmem chooses not to parse the options > + * for MS_KERNMOUNT, probably because shm_mnt is the only tmpfs > + * kernel mount other than this, where the mount options aren't > + * used. To workaround this we do a remount, which is fairly > + * inexpensive, where we know the options are never igonored. > + */ > + ret = sb->s_op->remount_fs(sb, &flags, options); This sounds like a bugfix to be sent. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/glk: Fix dotclock calculation in skl_check_pipe_max_pixel_rate
Seems that GLK has a dotclock that's twice the display clock. skl_max_scale checks for IS_GEMINILAKE, so perform the same check here. While at it, change the DRM_ERROR to DEBUG_KMS. Fixes: 73b0ca8ec76d ("drm/i915/skl+: consider max supported plane pixel rate while scaling") Cc: Mahesh Kumar Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_pm.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2042f6512e6e..88c8a3511e24 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4122,7 +4122,7 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc, struct drm_plane *plane; const struct drm_plane_state *pstate; struct intel_plane_state *intel_pstate; - int crtc_clock, cdclk; + int crtc_clock, dotclk; uint32_t pipe_max_pixel_rate; uint_fixed_16_16_t pipe_downscale; uint_fixed_16_16_t max_downscale = u32_to_fixed_16_16(1); @@ -4157,11 +4157,15 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc, pipe_downscale = mul_fixed16(pipe_downscale, max_downscale); crtc_clock = crtc_state->adjusted_mode.crtc_clock; - cdclk = to_intel_atomic_state(state)->cdclk.logical.cdclk; - pipe_max_pixel_rate = div_round_up_u32_fixed16(cdclk, pipe_downscale); + dotclk = to_intel_atomic_state(state)->cdclk.logical.cdclk; + + if (IS_GEMINILAKE(to_i915(intel_crtc->base.dev))) + dotclk *= 2; + + pipe_max_pixel_rate = div_round_up_u32_fixed16(dotclk, pipe_downscale); if (pipe_max_pixel_rate < crtc_clock) { - DRM_ERROR("Max supported pixel clock with scaling exceeded\n"); + DRM_DEBUG_KMS("Max supported pixel clock with scaling exceeded\n"); return -EINVAL; } -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Assert that we switch between known ggtt->invalidate functions
On to, 2017-06-01 at 10:04 +0100, Chris Wilson wrote: > When we enable the GuC, we enable an alternative mechanism for doing > post-GGTT update invalidation. Likewise, when we disable the GuC, we > restore the previous method. Assert that we change between known > endpoints, so that we can catch if we accidentally clobber some other > gen and if we change the invalidate routine without updating guc. > > Signed-off-by: Chris Wilson > Cc: Tvrtko Ursulin > Cc: Joonas Lahtinen > Cc: Oscar Mateo > Cc: Daniele Ceraolo Spurio > Cc: Michal Wajdeczko > Cc: Arkadiusz Hiler > Cc: Michel Thierry This was R-b'd by Michal already, a dup patch? Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: Fix dotclock calculation in skl_check_pipe_max_pixel_rate
hmm... didn't considered 2 pixels per clock. thanks. Reviewed-by: Mahesh Kumar On Thursday 01 June 2017 04:04 PM, Maarten Lankhorst wrote: Seems that GLK has a dotclock that's twice the display clock. skl_max_scale checks for IS_GEMINILAKE, so perform the same check here. While at it, change the DRM_ERROR to DEBUG_KMS. Fixes: 73b0ca8ec76d ("drm/i915/skl+: consider max supported plane pixel rate while scaling") Cc: Mahesh Kumar Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_pm.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2042f6512e6e..88c8a3511e24 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4122,7 +4122,7 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc, struct drm_plane *plane; const struct drm_plane_state *pstate; struct intel_plane_state *intel_pstate; - int crtc_clock, cdclk; + int crtc_clock, dotclk; uint32_t pipe_max_pixel_rate; uint_fixed_16_16_t pipe_downscale; uint_fixed_16_16_t max_downscale = u32_to_fixed_16_16(1); @@ -4157,11 +4157,15 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc, pipe_downscale = mul_fixed16(pipe_downscale, max_downscale); crtc_clock = crtc_state->adjusted_mode.crtc_clock; - cdclk = to_intel_atomic_state(state)->cdclk.logical.cdclk; - pipe_max_pixel_rate = div_round_up_u32_fixed16(cdclk, pipe_downscale); + dotclk = to_intel_atomic_state(state)->cdclk.logical.cdclk; + + if (IS_GEMINILAKE(to_i915(intel_crtc->base.dev))) + dotclk *= 2; + + pipe_max_pixel_rate = div_round_up_u32_fixed16(dotclk, pipe_downscale); if (pipe_max_pixel_rate < crtc_clock) { - DRM_ERROR("Max supported pixel clock with scaling exceeded\n"); + DRM_DEBUG_KMS("Max supported pixel clock with scaling exceeded\n"); return -EINVAL; } ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/15] drm/i915: really simple gemfs
On ke, 2017-05-31 at 19:51 +0100, Matthew Auld wrote: > Not a fully blown gemfs, just our very own tmpfs kernel mount. Doing so > moves us away from the shmemfs shm_mnt, and gives us the much needed > flexibility to do things like set our own mount options, namely huge= > which should allow us to enable the use of transparent-huge-pages for > our shmem backed objects. > > Signed-off-by: Matthew Auld > Cc: Joonas Lahtinen > Cc: Chris Wilson > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2227,6 +2227,9 @@ struct drm_i915_private { > DECLARE_HASHTABLE(mm_structs, 7); > struct mutex mm_lock; > > + /* Our tmpfs instance used for shmem backed objects */ > + struct vfsmount *gemfs_mnt; "gemfs" might be good enough, should not cause any confusion? > @@ -4169,4 +4172,14 @@ static inline bool i915_gem_object_is_coherent(struct > drm_i915_gem_object *obj) > HAS_LLC(to_i915(obj->base.dev))); > } > > +/* i915_gemfs.c */ i915_gemfs.h please. Lets not bloat i915_drv.h more when effort is in place to strip it down. > +struct vfsmount *i915_gemfs_create(void); Not "int gemfs_init(struct drm_i915_privat *i915)" and _fini? I doubt we should be creating more of these. > @@ -4268,7 +4286,7 @@ i915_gem_object_create(struct drm_i915_private > *dev_priv, u64 size) > if (obj == NULL) > return ERR_PTR(-ENOMEM); > > - ret = drm_gem_object_init(&dev_priv->drm, &obj->base, size); > + ret = i915_drm_gem_object_init(&dev_priv->drm, &obj->base, size); > if (ret) > goto fail; As Chris mentioned, smells bit like we could be targeting DRM scope in the future. > @@ -4383,6 +4401,9 @@ static void __i915_gem_free_objects(struct > drm_i915_private *i915, > drm_prime_gem_destroy(&obj->base, NULL); > > reservation_object_fini(&obj->__builtin_resv); > + For code below, do drop a note here that we want to do part of drm_gem_object_release's work in advance. Or rather than commenting, make it explicitly clear by having i915_gem_object_release() with this hunk of code. > + if (obj->base.filp) > + i915_gemfs_unlink(obj->base.filp); > drm_gem_object_release(&obj->base); > i915_gem_info_remove_obj(i915, obj->base.size); > > @@ -4843,6 +4864,10 @@ i915_gem_load_init(struct drm_i915_private *dev_priv) > { > int err = -ENOMEM; > > + dev_priv->gemfs_mnt = i915_gemfs_create(); > + if (IS_ERR(dev_priv->gemfs_mnt)) > + return PTR_ERR(dev_priv->gemfs_mnt); err = i915_gemfs_init(dev_priv); if (err) return err; > + > dev_priv->objects = KMEM_CACHE(drm_i915_gem_object, SLAB_HWCACHE_ALIGN); > if (!dev_priv->objects) err = -ENOMEM; goto err_gemfs; > @@ -4930,6 +4956,8 @@ void i915_gem_load_cleanup(struct drm_i915_private > *dev_priv) > > /* And ensure that our DESTROY_BY_RCU slabs are truly destroyed */ > rcu_barrier(); > + > + i915_gemfs_destroy(dev_priv->gemfs_mnt); i915_gemfs_fini(); > +struct file *i915_gemfs_file_setup(struct vfsmount *gemfs_mnt, > + const char *name, size_t size) > +{ > + > + inode = d_inode(path.dentry); > + inode->i_size = size; > + > + res = alloc_file(&path, FMODE_WRITE | FMODE_READ, inode->i_fop); shmem is passing their own fops, we don't need to? shmem_mmap seems to have some transparent huge page code at least which would be missed, no? > + if (IS_ERR(res)) > + goto unlink; > + > + return res; > + > +unlink: > + dir->i_op->unlink(dir, path.dentry); > +put_path: > + path_put(&path); Might throw newline here. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/glk: Fix dotclock calculation in skl_check_pipe_max_pixel_rate
== Series Details == Series: drm/i915/glk: Fix dotclock calculation in skl_check_pipe_max_pixel_rate URL : https://patchwork.freedesktop.org/series/25155/ State : success == Summary == Series 25155v1 drm/i915/glk: Fix dotclock calculation in skl_check_pipe_max_pixel_rate https://patchwork.freedesktop.org/api/1.0/series/25155/revisions/1/mbox/ Test kms_busy: Subgroup basic-flip-default-a: pass -> DMESG-WARN (fi-skl-6700hq) fdo#101144 +1 fdo#101144 https://bugs.freedesktop.org/show_bug.cgi?id=101144 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:443s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:434s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:576s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:515s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:493s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:476s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:430s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:410s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:416s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:490s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:464s fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 time:470s fi-kbl-7560u total:278 pass:263 dwarn:5 dfail:0 fail:0 skip:10 time:568s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:457s fi-skl-6700hqtotal:278 pass:228 dwarn:1 dfail:0 fail:27 skip:22 time:404s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:465s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:502s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:436s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:538s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:402s b0d30ff104856c5c49eee882d77aac32928f9724 drm-tip: 2017y-06m-01d-09h-45m-19s UTC integration manifest 50ad41b drm/i915/glk: Fix dotclock calculation in skl_check_pipe_max_pixel_rate == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4854/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: Fix dotclock calculation in skl_check_pipe_max_pixel_rate
Op 01-06-17 om 12:52 schreef Mahesh Kumar: > > hmm... didn't considered 2 pixels per clock. > > thanks. > > Reviewed-by: Mahesh Kumar > Thanks, fix pushed. :) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/15] drm/i915: introduce page_size_mask to dev_info
On ke, 2017-05-31 at 19:51 +0100, Matthew Auld wrote: > In preparation for huge gtt pages expose a page_size_mask as part of the > device info, to indicate the page sizes supported by the HW. Currently > only 4K is supported. > > Signed-off-by: Matthew Auld > Cc: Joonas Lahtinen > Cc: Mika Kuoppala > Cc: Chris Wilson I don't quite get why there can't be more inheritance when declaring these (Jani CC'd), but not related to this patch. > /* Keep in gen based order, and chronological order within a gen */ > + > +#define GEN_DEFAULT_PAGE_SIZES \ > + .page_size_mask = I915_GTT_PAGE_SIZE_4K > + > #define GEN2_FEATURES \ > .gen = 2, .num_pipes = 1, \ > .has_overlay = 1, .overlay_needs_physical = 1, \ > @@ -64,6 +68,7 @@ > .unfenced_needs_alignment = 1, \ > .ring_mask = RENDER_RING, \ > GEN_DEFAULT_PIPEOFFSETS, \ > + GEN_DEFAULT_PAGE_SIZES, \ > CURSOR_OFFSETS > > static const struct intel_device_info intel_i830_info = { > @@ -96,6 +101,7 @@ static const struct intel_device_info intel_i865g_info = { > .has_gmch_display = 1, \ > .ring_mask = RENDER_RING, \ > GEN_DEFAULT_PIPEOFFSETS, \ > + GEN_DEFAULT_PAGE_SIZES, \ > CURSOR_OFFSETS > > static const struct intel_device_info intel_i915g_info = { > @@ -158,6 +164,7 @@ static const struct intel_device_info intel_pineview_info > = { > .has_gmch_display = 1, \ > .ring_mask = RENDER_RING, \ > GEN_DEFAULT_PIPEOFFSETS, \ > + GEN_DEFAULT_PAGE_SIZES, \ > CURSOR_OFFSETS And goes on... Seems repetitive. Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Assert that we switch between known ggtt->invalidate functions
On Thu, Jun 01, 2017 at 01:44:07PM +0300, Joonas Lahtinen wrote: > On to, 2017-06-01 at 10:04 +0100, Chris Wilson wrote: > > When we enable the GuC, we enable an alternative mechanism for doing > > post-GGTT update invalidation. Likewise, when we disable the GuC, we > > restore the previous method. Assert that we change between known > > endpoints, so that we can catch if we accidentally clobber some other > > gen and if we change the invalidate routine without updating guc. > > > > Signed-off-by: Chris Wilson > > Cc: Tvrtko Ursulin > > Cc: Joonas Lahtinen > > Cc: Oscar Mateo > > Cc: Daniele Ceraolo Spurio > > Cc: Michal Wajdeczko > > Cc: Arkadiusz Hiler > > Cc: Michel Thierry > > This was R-b'd by Michal already, a dup patch? This is the second step to turn the v4.11 bugfix into a warning that it never happens again. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] pwm: lpss: Add intel-gfx as consumer device in lookup table
On Wednesday 31 May 2017 10:16 PM, kbuild test robot wrote: Hi Shobhit, [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on v4.12-rc3 next-20170531] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] Guess this needs to be built with CONFIG_PWM_LPSS_PLATFORM=y. Else I would have to send a patch to do EXPORT_SYMBOL_GPL for missing symbols. But then all the drivers that depend on this one are actually in-built. Any suggestions ? Regards Shobhit url: https://github.com/0day-ci/linux/commits/Shobhit-Kumar/drm-i915-Encapsulate-the-pwm_device-in-a-pwm_info-structure/20170531-222631 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-randconfig-i0-201722 (attached as .config) compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): ERROR: "pwm_add_table" [drivers/pwm/pwm-lpss-platform.ko] undefined! ERROR: "pwm_remove_table" [drivers/pwm/pwm-lpss-platform.ko] undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Assert that we switch between known ggtt->invalidate functions
On Thu, Jun 01, 2017 at 01:24:55PM +0300, Joonas Lahtinen wrote: > On to, 2017-06-01 at 12:03 +0200, Michal Wajdeczko wrote: > > On Thu, Jun 01, 2017 at 10:04:46AM +0100, Chris Wilson wrote: > > > > > > void i915_ggtt_disable_guc(struct drm_i915_private *i915) > > > { > > > - if (i915->ggtt.invalidate == guc_ggtt_invalidate) > > > - i915->ggtt.invalidate = gen6_ggtt_invalidate; > > > + /* We should only be called after i915_ggtt_enable_guc() */ > > > + GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate); > > > + > > > + i915->ggtt.invalidate = gen6_ggtt_invalidate; > > > } > > > > While this looks correct today, it may not work in the future if we > > will need somethig other than gen6_ggtt_invalidate() as base invalidate > > function or guc_gtt_invalidate() as the one for the guc. Just a head up. > > Currently the assignment is directly to gen6_ggtt_invalidate, no > questions asked. So I don't think the assert could be much more :) > > Maybe GuC code should backup the invalidate function before overriding. Possibly, but I hope the alternatives are a little better known so that we don't have to do save/restore of function pointers too often. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] i915: fix build on gcc7
With gcc7, the conditional usage of (port == PORT_A ? PORT_C : PORT_A) triggers -Werror=int-in-bool-context which breaks the build. Instead, use a temporary port_other variable that avoids hitting this error. % gcc --version gcc (SUSE Linux) 7.1.1 20170517 [gcc-7-branch revision 248152] Copyright (C) 2017 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. % make -j8 drivers/gpu/drm/i915/intel_dsi.o In file included from drivers/gpu/drm/i915/intel_dsi.c:34:0: drivers/gpu/drm/i915/intel_dsi.c: In function ‘intel_dsi_prepare’: drivers/gpu/drm/i915/intel_dsi.c:1487:23: error: ?: using integer constants in boolean context [-Werror=int-in-bool-context] PORT_A ? PORT_C : PORT_A), drivers/gpu/drm/i915/i915_drv.h:3909:76: note: in definition of macro ‘I915_WRITE’ #define I915_WRITE(reg, val) dev_priv->uncore.funcs.mmio_writel(dev_priv, (reg), (val), true) ^~~ drivers/gpu/drm/i915/i915_reg.h:8280:32: note: in expansion of macro ‘_MMIO’ #define _MMIO_MIPI(port, a, c) _MMIO(_MIPI_PORT(port, a, c)) ^ drivers/gpu/drm/i915/i915_reg.h:8280:38: note: in expansion of macro ‘_MIPI_PORT’ #define _MMIO_MIPI(port, a, c) _MMIO(_MIPI_PORT(port, a, c)) ^~ drivers/gpu/drm/i915/i915_reg.h:8624:32: note: in expansion of macro ‘_MMIO_MIPI’ #define MIPI_INIT_COUNT(port) _MMIO_MIPI(port, _MIPIA_INIT_COUNT, _MIPIC_INIT_COUNT) ^~ drivers/gpu/drm/i915/intel_dsi.c:1486:15: note: in expansion of macro ‘MIPI_INIT_COUNT’ I915_WRITE(MIPI_INIT_COUNT(port == ^~~ Signed-off-by: Aleksa Sarai --- drivers/gpu/drm/i915/intel_dsi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index 54030b68406a..53e717e7b811 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -1476,14 +1476,15 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder, txclkesc(intel_dsi->escape_clk_div, 100)); if (IS_GEN9_LP(dev_priv) && (!intel_dsi->dual_link)) { + enum port port_other = port == PORT_A ? PORT_C : PORT_A; + /* * BXT spec says write MIPI_INIT_COUNT for * both the ports, even if only one is * getting used. So write the other port * if not in dual link mode. */ - I915_WRITE(MIPI_INIT_COUNT(port == - PORT_A ? PORT_C : PORT_A), + I915_WRITE(MIPI_INIT_COUNT(port_other), intel_dsi->init_count); } -- 2.13.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Assert that we switch between known ggtt->invalidate functions
On to, 2017-06-01 at 12:01 +0100, Chris Wilson wrote: > On Thu, Jun 01, 2017 at 01:44:07PM +0300, Joonas Lahtinen wrote: > > > > On to, 2017-06-01 at 10:04 +0100, Chris Wilson wrote: > > > > > > When we enable the GuC, we enable an alternative mechanism for doing > > > post-GGTT update invalidation. Likewise, when we disable the GuC, we > > > restore the previous method. Assert that we change between known > > > endpoints, so that we can catch if we accidentally clobber some other > > > gen and if we change the invalidate routine without updating guc. > > > > > > Signed-off-by: Chris Wilson > > > Cc: Tvrtko Ursulin > > > Cc: Joonas Lahtinen > > > Cc: Oscar Mateo > > > Cc: Daniele Ceraolo Spurio > > > Cc: Michal Wajdeczko > > > Cc: Arkadiusz Hiler > > > Cc: Michel Thierry > > > > This was R-b'd by Michal already, a dup patch? > > This is the second step to turn the v4.11 bugfix into a warning that it > never happens again. Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v15 06/14] drm/i915/perf: Add OA unit support for Gen 8+
On Wed, May 31, 2017 at 01:33:47PM +0100, Lionel Landwerlin wrote: > u32 gen7_latched_oastatus1; > + u32 ctx_oactxctrl_offset; > + u32 ctx_flexeu0_offset; > + > + /* The RPT_ID/reason field for Gen8+ includes a bit > + * to determine if the CTX ID in the report is valid > + * but the specific bit differs between Gen 8 and 9 > + */ This is completely minor, but a coding-style nit nevertheless, can you go through and make sure all block commments are /* * Hello World! */ i.e. a blank first line (for symmetry with the blank last apparently), but it does pay off when mixing normal comment blocks with kerneldoc. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v15 14/14] drm/i915/perf: notify sseu configuration changes
On Wed, May 31, 2017 at 01:33:55PM +0100, Lionel Landwerlin wrote: > @@ -2493,6 +2499,44 @@ struct drm_i915_private { > const struct i915_oa_format *oa_formats; > int n_builtin_sets; > } oa; > + > + struct { > + /** > + * Buffer containing change reports of the SSEU > + * configuration. > + */ > + struct i915_vma *vma; > + u8 *vaddr; Make me void *. Kernel uses gcc so arthimetic on void * is valid, and this avoid lots of casts around the place. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] i915: fix build on gcc7
On Thu, 01 Jun 2017, Aleksa Sarai wrote: > With gcc7, the conditional usage of (port == PORT_A ? PORT_C : PORT_A) > triggers -Werror=int-in-bool-context which breaks the build. Instead, > use a temporary port_other variable that avoids hitting this error. > > % gcc --version > gcc (SUSE Linux) 7.1.1 20170517 [gcc-7-branch revision 248152] > Copyright (C) 2017 Free Software Foundation, Inc. > This is free software; see the source for copying conditions. There is NO > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > % make -j8 drivers/gpu/drm/i915/intel_dsi.o > In file included from drivers/gpu/drm/i915/intel_dsi.c:34:0: > drivers/gpu/drm/i915/intel_dsi.c: In function ‘intel_dsi_prepare’: > drivers/gpu/drm/i915/intel_dsi.c:1487:23: error: ?: using integer constants > in boolean context [-Werror=int-in-bool-context] > PORT_A ? PORT_C : PORT_A), > drivers/gpu/drm/i915/i915_drv.h:3909:76: note: in definition of macro > ‘I915_WRITE’ >#define I915_WRITE(reg, val) dev_priv->uncore.funcs.mmio_writel(dev_priv, > (reg), (val), true) > > ^~~ > drivers/gpu/drm/i915/i915_reg.h:8280:32: note: in expansion of macro ‘_MMIO’ >#define _MMIO_MIPI(port, a, c) _MMIO(_MIPI_PORT(port, a, c)) > ^ > drivers/gpu/drm/i915/i915_reg.h:8280:38: note: in expansion of macro > ‘_MIPI_PORT’ >#define _MMIO_MIPI(port, a, c) _MMIO(_MIPI_PORT(port, a, c)) > ^~ > drivers/gpu/drm/i915/i915_reg.h:8624:32: note: in expansion of macro > ‘_MMIO_MIPI’ >#define MIPI_INIT_COUNT(port) _MMIO_MIPI(port, _MIPIA_INIT_COUNT, > _MIPIC_INIT_COUNT) > ^~ > drivers/gpu/drm/i915/intel_dsi.c:1486:15: note: in expansion of macro > ‘MIPI_INIT_COUNT’ > I915_WRITE(MIPI_INIT_COUNT(port == > ^~~ > > Signed-off-by: Aleksa Sarai This is probably already fixed in drm-next by commit 0ad4dc887d4168448e8c801aa4edd8fe1e0bd534 Author: Hans de Goede Date: Thu May 18 13:06:44 2017 +0200 drm/i915: Fix new -Wint-in-bool-context gcc compiler warning which I also think is a more sensible fix than this one. BR, Jani. > --- > drivers/gpu/drm/i915/intel_dsi.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c > b/drivers/gpu/drm/i915/intel_dsi.c > index 54030b68406a..53e717e7b811 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -1476,14 +1476,15 @@ static void intel_dsi_prepare(struct intel_encoder > *intel_encoder, > txclkesc(intel_dsi->escape_clk_div, 100)); > > if (IS_GEN9_LP(dev_priv) && (!intel_dsi->dual_link)) { > + enum port port_other = port == PORT_A ? PORT_C : PORT_A; > + > /* >* BXT spec says write MIPI_INIT_COUNT for >* both the ports, even if only one is >* getting used. So write the other port >* if not in dual link mode. >*/ > - I915_WRITE(MIPI_INIT_COUNT(port == > - PORT_A ? PORT_C : PORT_A), > + I915_WRITE(MIPI_INIT_COUNT(port_other), > intel_dsi->init_count); > } -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v15 14/14] drm/i915/perf: notify sseu configuration changes
On Wed, May 31, 2017 at 01:33:55PM +0100, Lionel Landwerlin wrote: > static void > -free_oa_buffer(struct drm_i915_private *i915) > +free_sseu_buffer(struct drm_i915_private *i915) > { > - mutex_lock(&i915->drm.struct_mutex); > + i915_gem_object_unpin_map(i915->perf.sseu_buffer.vma->obj); > + i915_vma_unpin(i915->perf.sseu_buffer.vma); > + i915_gem_object_put(i915->perf.sseu_buffer.vma->obj); unpin followed by put is common enough that it is wrapped into i915_vam_unpin_and_release(). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for i915: fix build on gcc7
== Series Details == Series: i915: fix build on gcc7 URL : https://patchwork.freedesktop.org/series/25158/ State : success == Summary == Series 25158v1 i915: fix build on gcc7 https://patchwork.freedesktop.org/api/1.0/series/25158/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: pass -> FAIL (fi-snb-2600) fdo#17 Test kms_busy: Subgroup basic-flip-default-c: pass -> DMESG-WARN (fi-skl-6700hq) fdo#101144 +1 Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: pass -> FAIL (fi-skl-6700hq) fdo#101154 +7 fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fdo#101144 https://bugs.freedesktop.org/show_bug.cgi?id=101144 fdo#101154 https://bugs.freedesktop.org/show_bug.cgi?id=101154 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:445s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:435s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:577s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:515s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:490s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:477s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:431s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:414s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:417s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:496s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:467s fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 time:467s fi-kbl-7560u total:278 pass:263 dwarn:5 dfail:0 fail:0 skip:10 time:567s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:462s fi-skl-6700hqtotal:278 pass:230 dwarn:1 dfail:0 fail:25 skip:22 time:405s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:465s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:506s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:435s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:533s fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 time:403s 5f6a8e63980502f7372abb6f1e43354dcaf59280 drm-tip: 2017y-06m-01d-10h-53m-42s UTC integration manifest 0c7e7ed i915: fix build on gcc7 == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4855/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/15] drm/i915: introduce page_size_mask to dev_info
On Thu, 01 Jun 2017, Joonas Lahtinen wrote: > On ke, 2017-05-31 at 19:51 +0100, Matthew Auld wrote: >> In preparation for huge gtt pages expose a page_size_mask as part of the >> device info, to indicate the page sizes supported by the HW. Currently >> only 4K is supported. >> >> Signed-off-by: Matthew Auld >> Cc: Joonas Lahtinen >> Cc: Mika Kuoppala >> Cc: Chris Wilson > > > > I don't quite get why there can't be more inheritance when declaring > these (Jani CC'd), but not related to this patch. Yeah, could use more inheritance, but can also be follow-up. *shrug* BR, Jani. > >> /* Keep in gen based order, and chronological order within a gen */ >> + >> +#define GEN_DEFAULT_PAGE_SIZES \ >> +.page_size_mask = I915_GTT_PAGE_SIZE_4K >> + >> #define GEN2_FEATURES \ >> .gen = 2, .num_pipes = 1, \ >> .has_overlay = 1, .overlay_needs_physical = 1, \ >> @@ -64,6 +68,7 @@ >> .unfenced_needs_alignment = 1, \ >> .ring_mask = RENDER_RING, \ >> GEN_DEFAULT_PIPEOFFSETS, \ >> +GEN_DEFAULT_PAGE_SIZES, \ >> CURSOR_OFFSETS >> >> static const struct intel_device_info intel_i830_info = { >> @@ -96,6 +101,7 @@ static const struct intel_device_info intel_i865g_info = { >> .has_gmch_display = 1, \ >> .ring_mask = RENDER_RING, \ >> GEN_DEFAULT_PIPEOFFSETS, \ >> +GEN_DEFAULT_PAGE_SIZES, \ >> CURSOR_OFFSETS >> >> static const struct intel_device_info intel_i915g_info = { >> @@ -158,6 +164,7 @@ static const struct intel_device_info >> intel_pineview_info = { >> .has_gmch_display = 1, \ >> .ring_mask = RENDER_RING, \ >> GEN_DEFAULT_PIPEOFFSETS, \ >> +GEN_DEFAULT_PAGE_SIZES, \ >> CURSOR_OFFSETS > > And goes on... Seems repetitive. > > Reviewed-by: Joonas Lahtinen > > Regards, Joonas -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] i915: fix build on gcc7
This is probably already fixed in drm-next by commit 0ad4dc887d4168448e8c801aa4edd8fe1e0bd534 Author: Hans de Goede Date: Thu May 18 13:06:44 2017 +0200 drm/i915: Fix new -Wint-in-bool-context gcc compiler warning > which I also think is a more sensible fix than this one. Ah, I didn't see that patch. Yeah, you're right. -- Aleksa Sarai Software Engineer (Containers) SUSE Linux GmbH https://www.cyphar.com/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Implementing Miracast
On 01/06/17 09:02, Daniel Kasak wrote: Any news? Seems every TV I bump into these days has Miracast support ... Sorry, it must be frustrating. Some code needs to be re-implemented because it was GPL-based and we need it as MIT. Until I have some time to do this, this will probably not move forward :s ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/ddi: Avoid long delays during system suspend / eDP disabling
On Wed, May 31, 2017 at 05:34:42PM +, Patchwork wrote: > == Series Details == > > Series: drm/i915/ddi: Avoid long delays during system suspend / eDP disabling > URL : https://patchwork.freedesktop.org/series/25116/ > State : success > > == Summary == > > Series 25116v1 drm/i915/ddi: Avoid long delays during system suspend / eDP > disabling > https://patchwork.freedesktop.org/api/1.0/series/25116/revisions/1/mbox/ I pushed this to -dinq, thanks for the review. > > fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 > time:444s > fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 > time:432s > fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 > time:579s > fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 > time:510s > fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 > time:491s > fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 > time:488s > fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 > time:428s > fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 > time:412s > fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 > time:420s > fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 > time:497s > fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 > time:461s > fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 > time:458s > fi-kbl-7560u total:278 pass:263 dwarn:5 dfail:0 fail:0 skip:10 > time:572s > fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 > time:461s > fi-skl-6700hqtotal:278 pass:239 dwarn:0 dfail:1 fail:17 skip:21 > time:427s > fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 > time:469s > fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 > time:500s > fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 > time:436s > fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 > time:532s > fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 > time:402s > > 593aae9587d79f8a823a36f4e6cc12e23b547d07 drm-tip: 2017y-05m-31d-14h-36m-16s > UTC integration manifest > 1119300 drm/i915/ddi: Avoid long delays during system suspend / eDP disabling > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4845/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/15] drm/i915: really simple gemfs
On 1 June 2017 at 11:49, Joonas Lahtinen wrote: > On ke, 2017-05-31 at 19:51 +0100, Matthew Auld wrote: >> Not a fully blown gemfs, just our very own tmpfs kernel mount. Doing so >> moves us away from the shmemfs shm_mnt, and gives us the much needed >> flexibility to do things like set our own mount options, namely huge= >> which should allow us to enable the use of transparent-huge-pages for >> our shmem backed objects. >> >> Signed-off-by: Matthew Auld >> Cc: Joonas Lahtinen >> Cc: Chris Wilson > > > >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -2227,6 +2227,9 @@ struct drm_i915_private { >> DECLARE_HASHTABLE(mm_structs, 7); >> struct mutex mm_lock; >> >> + /* Our tmpfs instance used for shmem backed objects */ >> + struct vfsmount *gemfs_mnt; > > "gemfs" might be good enough, should not cause any confusion? > >> @@ -4169,4 +4172,14 @@ static inline bool i915_gem_object_is_coherent(struct >> drm_i915_gem_object *obj) >> HAS_LLC(to_i915(obj->base.dev))); >> } >> >> +/* i915_gemfs.c */ > > i915_gemfs.h please. Lets not bloat i915_drv.h more when effort is in > place to strip it down. > >> +struct vfsmount *i915_gemfs_create(void); > > Not "int gemfs_init(struct drm_i915_privat *i915)" and _fini? > > I doubt we should be creating more of these. > >> @@ -4268,7 +4286,7 @@ i915_gem_object_create(struct drm_i915_private >> *dev_priv, u64 size) >> if (obj == NULL) >> return ERR_PTR(-ENOMEM); >> >> - ret = drm_gem_object_init(&dev_priv->drm, &obj->base, size); >> + ret = i915_drm_gem_object_init(&dev_priv->drm, &obj->base, size); >> if (ret) >> goto fail; > > As Chris mentioned, smells bit like we could be targeting DRM scope in > the future. > >> @@ -4383,6 +4401,9 @@ static void __i915_gem_free_objects(struct >> drm_i915_private *i915, >> drm_prime_gem_destroy(&obj->base, NULL); >> >> reservation_object_fini(&obj->__builtin_resv); >> + > > For code below, do drop a note here that we want to do part of > drm_gem_object_release's work in advance. Or rather than commenting, > make it explicitly clear by having i915_gem_object_release() with this > hunk of code. > >> + if (obj->base.filp) >> + i915_gemfs_unlink(obj->base.filp); >> drm_gem_object_release(&obj->base); >> i915_gem_info_remove_obj(i915, obj->base.size); >> >> @@ -4843,6 +4864,10 @@ i915_gem_load_init(struct drm_i915_private *dev_priv) >> { >> int err = -ENOMEM; >> >> + dev_priv->gemfs_mnt = i915_gemfs_create(); >> + if (IS_ERR(dev_priv->gemfs_mnt)) >> + return PTR_ERR(dev_priv->gemfs_mnt); > > err = i915_gemfs_init(dev_priv); > if (err) > return err; > >> + >> dev_priv->objects = KMEM_CACHE(drm_i915_gem_object, >> SLAB_HWCACHE_ALIGN); >> if (!dev_priv->objects) > > err = -ENOMEM; > goto err_gemfs; > >> @@ -4930,6 +4956,8 @@ void i915_gem_load_cleanup(struct drm_i915_private >> *dev_priv) >> >> /* And ensure that our DESTROY_BY_RCU slabs are truly destroyed */ >> rcu_barrier(); >> + >> + i915_gemfs_destroy(dev_priv->gemfs_mnt); > > i915_gemfs_fini(); > >> +struct file *i915_gemfs_file_setup(struct vfsmount *gemfs_mnt, >> +const char *name, size_t size) >> +{ > > > >> + >> + inode = d_inode(path.dentry); >> + inode->i_size = size; >> + >> + res = alloc_file(&path, FMODE_WRITE | FMODE_READ, inode->i_fop); > > shmem is passing their own fops, we don't need to? shmem_mmap seems to > have some transparent huge page code at least which would be missed, > no? We pass the same fops, since shmem_file_operations == inode->i_fop. See shmem_get_inode. I'll try to address the other comments, thanks. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/ddi: Avoid long delays during system suspend / eDP disabling
On Wed, 31 May 2017, Ville Syrjälä wrote: > On Wed, May 31, 2017 at 08:05:35PM +0300, Imre Deak wrote: >> Atm disabling either DP or eDP outputs can generate a spurious short >> pulse interrupt. The reason is that after disabling the port the source >> will stop sending a valid stream data, while the sink expects either a >> valid stream or the idle pattern. Since neither of this is sent the sink >> assumes (after an arbitrary delay) that the link is lost and requests >> for link retraining with a short pulse. >> >> The spurious pulse is a real problem at least for eDP panels with long >> power-off / power-cycle delays: as part of disabling the output we >> disable the panel power. The subsequent spurious short pulse handling >> will have to turn the power back on, which means the driver has to do a >> redundant wait for the power-off and power-cycle delays. During system >> suspend this leads to an unnecessary delay up to ~1s on systems with >> such panels as reported by Rui. >> >> To fix this put the sink to DPMS D3 state before turning off the port. >> According to the DP spec in this state the sink should not request >> retraining. This is also what we do already on pre-ddi platforms. >> >> As an alternative I also tried configuring the port to send idle pattern >> - which is against BSPec - and leave the port in normal mode before >> turning off the port. Neither of these resolved the problem. >> >> Cc: Zhang Rui >> Cc: David Weinehall >> Cc: Ville Syrjälä >> Reported-and-tested-by: Zhang Rui >> Signed-off-by: Imre Deak > > Makes sense to me. I wonder if we should write D0 on hotplug. BR, Jani. > > Reviewed-by: Ville Syrjälä > >> --- >> drivers/gpu/drm/i915/intel_ddi.c | 10 +++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c >> b/drivers/gpu/drm/i915/intel_ddi.c >> index 0914ad9..8bac628 100644 >> --- a/drivers/gpu/drm/i915/intel_ddi.c >> +++ b/drivers/gpu/drm/i915/intel_ddi.c >> @@ -1732,12 +1732,18 @@ static void intel_ddi_post_disable(struct >> intel_encoder *intel_encoder, >> struct drm_i915_private *dev_priv = to_i915(encoder->dev); >> enum port port = intel_ddi_get_encoder_port(intel_encoder); >> struct intel_digital_port *dig_port = enc_to_dig_port(encoder); >> +struct intel_dp *intel_dp = NULL; >> int type = intel_encoder->type; >> uint32_t val; >> bool wait = false; >> >> /* old_crtc_state and old_conn_state are NULL when called from DP_MST */ >> >> +if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) { >> +intel_dp = enc_to_intel_dp(encoder); >> +intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); >> +} >> + >> val = I915_READ(DDI_BUF_CTL(port)); >> if (val & DDI_BUF_CTL_ENABLE) { >> val &= ~DDI_BUF_CTL_ENABLE; >> @@ -1753,9 +1759,7 @@ static void intel_ddi_post_disable(struct >> intel_encoder *intel_encoder, >> if (wait) >> intel_wait_ddi_buf_idle(dev_priv, port); >> >> -if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) { >> -struct intel_dp *intel_dp = enc_to_intel_dp(encoder); >> -intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); >> +if (intel_dp) { >> intel_edp_panel_vdd_on(intel_dp); >> intel_edp_panel_off(intel_dp); >> } >> -- >> 2.7.4 -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/dvo: fix debug logging on unknown DID
On Wed, 31 May 2017, Clint Taylor wrote: > On 05/31/2017 03:16 AM, Jani Nikula wrote: >> Print DID not VID on the DID error path. Looks like a copy-paste error >> from the VID error path. Clarify and clean up error logging, making them >> distinguishable from each other, while at it. > > Reviewed-by: Clinton Taylor Thanks, pushed to drm-intel-next-queued. BR, Jani. > > -Clint > >> >> Reported-by: Petru Mihancea >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101243 >> Signed-off-by: Jani Nikula >> --- >> drivers/gpu/drm/i915/dvo_ch7xxx.c | 10 -- >> 1 file changed, 4 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/dvo_ch7xxx.c >> b/drivers/gpu/drm/i915/dvo_ch7xxx.c >> index 44b3159f2fe8..7aeeffd2428b 100644 >> --- a/drivers/gpu/drm/i915/dvo_ch7xxx.c >> +++ b/drivers/gpu/drm/i915/dvo_ch7xxx.c >> @@ -217,9 +217,8 @@ static bool ch7xxx_init(struct intel_dvo_device *dvo, >> >> name = ch7xxx_get_id(vendor); >> if (!name) { >> -DRM_DEBUG_KMS("ch7xxx not detected; got 0x%02x from %s " >> -"slave %d.\n", >> - vendor, adapter->name, dvo->slave_addr); >> +DRM_DEBUG_KMS("ch7xxx not detected; got VID 0x%02x from %s >> slave %d.\n", >> + vendor, adapter->name, dvo->slave_addr); >> goto out; >> } >> >> @@ -229,9 +228,8 @@ static bool ch7xxx_init(struct intel_dvo_device *dvo, >> >> devid = ch7xxx_get_did(device); >> if (!devid) { >> -DRM_DEBUG_KMS("ch7xxx not detected; got 0x%02x from %s " >> -"slave %d.\n", >> - vendor, adapter->name, dvo->slave_addr); >> +DRM_DEBUG_KMS("ch7xxx not detected; got DID 0x%02x from %s >> slave %d.\n", >> + device, adapter->name, dvo->slave_addr); >> goto out; >> } >> > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/guc: Assert that we switch between known ggtt->invalidate functions
On Thu, Jun 01, 2017 at 09:36:58AM -, Patchwork wrote: > == Series Details == > > Series: drm/i915/guc: Assert that we switch between known ggtt->invalidate > functions > URL : https://patchwork.freedesktop.org/series/25150/ > State : success > > == Summary == > > Series 25150v1 drm/i915/guc: Assert that we switch between known > ggtt->invalidate functions > https://patchwork.freedesktop.org/api/1.0/series/25150/revisions/1/mbox/ > > fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 > time:444s > fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 > time:432s > fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 > time:585s > fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 > time:512s > fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 > time:488s > fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 > time:477s > fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 > time:431s > fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 > time:412s > fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 > time:419s > fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 > time:489s > fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 > time:464s > fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 > time:467s > fi-kbl-7560u total:278 pass:263 dwarn:5 dfail:0 fail:0 skip:10 > time:569s > fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 > time:455s > fi-skl-6700hqtotal:278 pass:239 dwarn:0 dfail:1 fail:17 skip:21 > time:435s > fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 > time:469s > fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 > time:502s > fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 > time:434s > fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 > time:542s > fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 > time:407s > > b479b5c889c24e5a7cf4beb064f0037fdc2d568d drm-tip: 2017y-06m-01d-08h-55m-16s > UTC integration manifest > 675b5c6 drm/i915/guc: Assert that we switch between known ggtt->invalidate > functions And pushed. Thanks for the review, -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Check the ring is empty when declaring the engines are idle
On Wed, May 31, 2017 at 05:01:53PM +0300, Mika Kuoppala wrote: > Chris Wilson writes: > > > On Tue, May 30, 2017 at 03:33:41PM +0300, Mika Kuoppala wrote: > >> Chris Wilson writes: > >> > >> > As another precaution when testing whether the CS engine is actually > >> > idle, also inspect the ring's HEAD/TAIL registers, which should be equal > >> > when there are no commands left to execute by the GPU. > >> > > >> > Signed-off-by: Chris Wilson > >> > Cc: Mika Kuoppala > >> > --- > >> > drivers/gpu/drm/i915/intel_engine_cs.c | 5 + > >> > 1 file changed, 5 insertions(+) > >> > > >> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > >> > b/drivers/gpu/drm/i915/intel_engine_cs.c > >> > index 699f2d3861c7..bc38bd128b76 100644 > >> > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > >> > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > >> > @@ -1212,6 +1212,11 @@ static bool ring_is_idle(struct intel_engine_cs > >> > *engine) > >> > > >> > intel_runtime_pm_get(dev_priv); > >> > > >> > +/* First check that no commands are left in the ring */ > >> > +if ((I915_READ_HEAD(engine) & HEAD_ADDR) != > >> > +(I915_READ_TAIL(engine) & TAIL_ADDR)) > >> > +idle = false; > >> > + > >> > >> You are already certain that is not idle so why not goto out? > > > > In this case I could argue that extra path for the jump is not worth it. > > It saves a mmio read, yes, but will any one notice? > and one write :P > > > It boils down to is it easier to read as: > > > > Sold. It is easier to read as is. 3/3 is > > Reviewed-by: Mika Kuoppala Thanks, now pushed. Hopefully it never spots an error! -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix logical inversion for gen4 quirking
On Mon, May 22, 2017 at 08:55:09AM +0300, Joonas Lahtinen wrote: > On su, 2017-05-21 at 13:40 +0100, Chris Wilson wrote: > > The assertion that we want to make before disabling the pin of the pages > > for the unknown swizzling quirk is that the quirk is indeed active. > > > > Fixes: 2c3a3f44dc13 ("drm/i915: Fix pages pin counting around swizzle > > quirk") > > Fixes: 957870f93412 ("drm/i915: Split out i915_gem_object_set_tiling()") > > Signed-off-by: Chris Wilson > > Cc: Chris Wilson > > Cc: Joonas Lahtinen > > Cc: Tvrtko Ursulin > > Reviewed-by: Joonas Lahtinen And pushed, thanks for the review. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Remove __GFP_NORETRY from our buffer allocator
I tried __GFP_NORETRY in the belief that __GFP_RECLAIM was effective. It struggles with handling reclaim via kswapd (through inconsistency within throttle_direct_reclaim() and even then the race between multiple allocators makes the two step of reclaim then allocate fragile), and as our buffers are always dirty (with very few exceptions), we required kswapd to perform pageout on them. The only effective means of waiting on kswapd is to retry the allocations (i.e. not set __GFP_NORETRY). That leaves us with the dilemma of invoking the oomkiller instead of propagating the allocation failure back to userspace where it can be handled more gracefully (one hopes). We cheat and note that __GFP_THISNODE has the side-effect of preventing oom and has no consequence for our final attempt at allocation. Fixes: 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations") Testcase: igt/gem_tiled_swapping Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 129515d8482a..53c51787d2ed 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2406,7 +2406,21 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) if (!*s) { /* reclaim and warn, but no oom */ gfp = mapping_gfp_mask(mapping); - gfp |= __GFP_NORETRY; + + /* Our bo are always dirty and so we require +* kswapd to reclaim our pages (direct reclaim +* performs no swapping on its own). However, +* direct reclaim is meant to wait for kswapd +* when under pressure, this is broken. As a +* result __GFP_RECLAIM is unreliable and fails +* to actually reclaim dirty pages -- unless +* you try over and over again with +* !__GFP_NORETRY. However, we still want to +* fail this allocation rather than trigger +* the out-of-memory killer and for this we +* subvert __GFP_THISNODE for that side effect. +*/ + gfp |= __GFP_THISNODE; } } while (1); -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Encourage our shrinker more when our shmemfs allocations fails
Commit 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations") made the bold decision to try and avoid the oomkiller by reporting -ENOMEM to userspace if our allocation failed after attempting to free enough buffer objects. In short, it appears we were giving up too easily (even before we start wondering if one pass of reclaim is as strong as we would like). Part of the problem is that if we only shrink just enough pages for our expected allocation, the likelihood of those pages becoming available to us is less than 100% To counter-act that we ask for twice the number of pages to be made available. Furthermore, we allow the shrinker to pull pages from the active list in later passes. Fixes: 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations") Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Daniel Vetter Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_gem.c | 52 - 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 7b676fd1f075..129515d8482a 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2337,8 +2337,8 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) struct page *page; unsigned long last_pfn = 0; /* suppress gcc warning */ unsigned int max_segment; + gfp_t noreclaim; int ret; - gfp_t gfp; /* Assert that the object is not currently in any GPU domain. As it * wasn't in the GTT, there shouldn't be any way it could have been in @@ -2367,22 +2367,33 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) * Fail silently without starting the shrinker */ mapping = obj->base.filp->f_mapping; - gfp = mapping_gfp_constraint(mapping, ~(__GFP_IO | __GFP_RECLAIM)); - gfp |= __GFP_NORETRY | __GFP_NOWARN; + noreclaim = mapping_gfp_constraint(mapping, + ~(__GFP_IO | __GFP_RECLAIM)); + noreclaim |= __GFP_NORETRY | __GFP_NOWARN; + sg = st->sgl; st->nents = 0; for (i = 0; i < page_count; i++) { - page = shmem_read_mapping_page_gfp(mapping, i, gfp); - if (unlikely(IS_ERR(page))) { - i915_gem_shrink(dev_priv, - page_count, - I915_SHRINK_BOUND | - I915_SHRINK_UNBOUND | - I915_SHRINK_PURGEABLE); + const unsigned int shrink[] = { + I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | I915_SHRINK_PURGEABLE, + I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | I915_SHRINK_PURGEABLE | I915_SHRINK_ACTIVE, + I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | I915_SHRINK_ACTIVE, + 0, + }, *s = shrink; + gfp_t gfp = noreclaim; + + do { page = shmem_read_mapping_page_gfp(mapping, i, gfp); - } - if (unlikely(IS_ERR(page))) { - gfp_t reclaim; + if (likely(!IS_ERR(page))) + break; + + if (!*s) { + ret = PTR_ERR(page); + goto err_sg; + } + + i915_gem_shrink(dev_priv, 2 * page_count, *s++); + cond_resched(); /* We've tried hard to allocate the memory by reaping * our own buffer, now let the real VM do its job and @@ -2392,15 +2403,13 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) * defer the oom here by reporting the ENOMEM back * to userspace. */ - reclaim = mapping_gfp_mask(mapping); - reclaim |= __GFP_NORETRY; /* reclaim, but no oom */ - - page = shmem_read_mapping_page_gfp(mapping, i, reclaim); - if (IS_ERR(page)) { - ret = PTR_ERR(page); - goto err_sg; + if (!*s) { + /* reclaim and warn, but no oom */ + gfp = mapping_gfp_mask(mapping); + gfp |= __GFP_NORETRY; } - } + } while (1); + if (!i || sg->length >= max_segment || page_to_pfn(page) != last_pfn + 1) { @@ -4285,6 +4294,7 @@ i915_gem_object_create(struct drm_i915_private *dev_priv, u64 size) mapping = obj->base.fil
[Intel-gfx] [PATCH 1/3] drm/i915: Allow kswapd to pause the device whilst reaping
In commit 5763ff04dc4e ("drm/i915: Avoid GPU stalls from kswapd") we stopped direct reclaim and kswapd from triggering GPU/client stalls whilst running (by restricting the objects they could reap to be idle). However with abusive GPU usage, it becomes quite easy to starve kswapd of memory and prevent it from making forward progress towards obtaining enough free memory (thus driving the system closer to swap exhaustion). Relax the previous restriction to allow kswapd (but not direct reclaim) to stall the device whilst reaping purgeable pages. v2: Also acquire the rpm wakelock to allow kswapd to unbind buffers. Signed-off-by: Chris Wilson Cc: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_gem_shrinker.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c index 0fd2b58ce475..58f27369183c 100644 --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c @@ -332,6 +332,15 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) sc->nr_to_scan - freed, I915_SHRINK_BOUND | I915_SHRINK_UNBOUND); + if (freed < sc->nr_to_scan && current_is_kswapd()) { + intel_runtime_pm_get(dev_priv); + freed += i915_gem_shrink(dev_priv, +sc->nr_to_scan - freed, +I915_SHRINK_ACTIVE | +I915_SHRINK_BOUND | +I915_SHRINK_UNBOUND); + intel_runtime_pm_put(dev_priv); + } shrinker_unlock(dev_priv, unlock); -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: Fix dotclock calculation in skl_check_pipe_max_pixel_rate
On Thu, Jun 01, 2017 at 12:34:13PM +0200, Maarten Lankhorst wrote: > Seems that GLK has a dotclock that's twice the display clock. > skl_max_scale checks for IS_GEMINILAKE, so perform the same check here. > > While at it, change the DRM_ERROR to DEBUG_KMS. > > Fixes: 73b0ca8ec76d ("drm/i915/skl+: consider max supported plane pixel > rate while scaling") > Cc: Mahesh Kumar > Signed-off-by: Maarten Lankhorst > --- > drivers/gpu/drm/i915/intel_pm.c | 12 > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 2042f6512e6e..88c8a3511e24 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4122,7 +4122,7 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc > *intel_crtc, > struct drm_plane *plane; > const struct drm_plane_state *pstate; > struct intel_plane_state *intel_pstate; > - int crtc_clock, cdclk; > + int crtc_clock, dotclk; > uint32_t pipe_max_pixel_rate; > uint_fixed_16_16_t pipe_downscale; > uint_fixed_16_16_t max_downscale = u32_to_fixed_16_16(1); > @@ -4157,11 +4157,15 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc > *intel_crtc, > pipe_downscale = mul_fixed16(pipe_downscale, max_downscale); > > crtc_clock = crtc_state->adjusted_mode.crtc_clock; > - cdclk = to_intel_atomic_state(state)->cdclk.logical.cdclk; > - pipe_max_pixel_rate = div_round_up_u32_fixed16(cdclk, pipe_downscale); > + dotclk = to_intel_atomic_state(state)->cdclk.logical.cdclk; dotclk = cdclk. That statement doesn't make sense. It should be called max_dotclk or something like that. > + > + if (IS_GEMINILAKE(to_i915(intel_crtc->base.dev))) > + dotclk *= 2; > + > + pipe_max_pixel_rate = div_round_up_u32_fixed16(dotclk, pipe_downscale); > > if (pipe_max_pixel_rate < crtc_clock) { > - DRM_ERROR("Max supported pixel clock with scaling exceeded\n"); > + DRM_DEBUG_KMS("Max supported pixel clock with scaling > exceeded\n"); > return -EINVAL; > } > > -- > 2.11.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] drm/i915/cnp: Backlight support for CNP.
On Wed, 31 May 2017, Rodrigo Vivi wrote: > Split out BXT and CNP's setup_backlight(),enable_backlight(), > disable_backlight() and hz_to_pwm() into > two separate functions instead of reusing BXT function. > > Reuse set_backlight() and get_backlight() since they have > no reference to the utility pin. > > v2: Reuse BXT functions with controller 0 instead of > redefining it. (Jani). > Use dev_priv->rawclk_freq instead of getting the value > from SFUSE_STRAP. > v3: Avoid setup backligh controller along with hooks and > fully reuse hooks setup as suggested by Jani. > v4: Clean up commit message. > v5: Implement per PCH instead per platform. > > v6: Introduce a new function for CNP.(Jani and Ville) > > v7: Squash the all CNP Backlight support patches into a > single patch. (Jani) > > v8: Correct indentation, remove unneeded blank lines and > correct mail address (Jani). > > v9: Remove unused enum pipe. (by CI) > > Reviewed-by: Jani Nikula > Suggested-by: Jani Nikula > Suggested-by: Ville Syrjala > Cc: Ville Syrjala > Cc: Jani Nikula > Signed-off-by: Anusha Srivatsa > Signed-off-by: Rodrigo Vivi > --- > drivers/gpu/drm/i915/intel_panel.c | 93 > ++ > 1 file changed, 93 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_panel.c > b/drivers/gpu/drm/i915/intel_panel.c > index c8103f8..7e34a1b 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -796,6 +796,19 @@ static void bxt_disable_backlight(struct intel_connector > *connector) > } > } > > +static void cnp_disable_backlight(struct intel_connector *connector) > +{ > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > + struct intel_panel *panel = &connector->panel; > + u32 tmp; > + > + intel_panel_actually_set_backlight(connector, 0); > + > + tmp = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller)); > + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller), > +tmp & ~BXT_BLC_PWM_ENABLE); > +} > + > static void pwm_disable_backlight(struct intel_connector *connector) > { > struct intel_panel *panel = &connector->panel; > @@ -1086,6 +1099,35 @@ static void bxt_enable_backlight(struct > intel_connector *connector) > pwm_ctl | BXT_BLC_PWM_ENABLE); > } > > +static void cnp_enable_backlight(struct intel_connector *connector) > +{ > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > + struct intel_panel *panel = &connector->panel; > + u32 pwm_ctl; > + > + pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller)); > + if (pwm_ctl & BXT_BLC_PWM_ENABLE) { > + DRM_DEBUG_KMS("backlight already enabled\n"); > + pwm_ctl &= ~BXT_BLC_PWM_ENABLE; > + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller), > +pwm_ctl); > + } > + > + I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller), > +panel->backlight.max); > + > + intel_panel_actually_set_backlight(connector, panel->backlight.level); > + > + pwm_ctl = 0; > + if (panel->backlight.active_low_pwm) > + pwm_ctl |= BXT_BLC_PWM_POLARITY; > + > + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller), pwm_ctl); > + POSTING_READ(BXT_BLC_PWM_CTL(panel->backlight.controller)); > + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller), > +pwm_ctl | BXT_BLC_PWM_ENABLE); > +} > + > static void pwm_enable_backlight(struct intel_connector *connector) > { > struct intel_panel *panel = &connector->panel; > @@ -1250,6 +1292,18 @@ void intel_backlight_device_unregister(struct > intel_connector *connector) > #endif /* CONFIG_BACKLIGHT_CLASS_DEVICE */ > > /* > + * CNP: PWM clock frequency is 19.2 MHz or 24 MHz. > + * Value is found in SFUSE_STRAP. This is confusing because it's not taken into account in the code. BR, Jani. > + * PWM increment = 1 > + */ > +static u32 cnp_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz) > +{ > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > + > + return DIV_ROUND_CLOSEST(KHz(dev_priv->rawclk_freq), pwm_freq_hz); > +} > + > +/* > * BXT: PWM clock frequency = 19.2 MHz. > */ > static u32 bxt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz) > @@ -1644,6 +1698,37 @@ static int vlv_setup_backlight(struct intel_connector > *connector, enum pipe pipe > return 0; > } > > +static int > +cnp_setup_backlight(struct intel_connector *connector, enum pipe unused) > +{ > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > + struct intel_panel *panel = &connector->panel; > + u32 pwm_ctl, val; > + > + panel->backlight.controller = dev_priv->vbt.backlight.controller; > + > + pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller)); > + > + panel->backlight.active_low_pwm = pwm_ct
Re: [Intel-gfx] [PATCH] drm/i915/ddi: Avoid long delays during system suspend / eDP disabling
On Thu, Jun 01, 2017 at 03:55:13PM +0300, Jani Nikula wrote: > On Wed, 31 May 2017, Ville Syrjälä wrote: > > On Wed, May 31, 2017 at 08:05:35PM +0300, Imre Deak wrote: > >> Atm disabling either DP or eDP outputs can generate a spurious short > >> pulse interrupt. The reason is that after disabling the port the source > >> will stop sending a valid stream data, while the sink expects either a > >> valid stream or the idle pattern. Since neither of this is sent the sink > >> assumes (after an arbitrary delay) that the link is lost and requests > >> for link retraining with a short pulse. > >> > >> The spurious pulse is a real problem at least for eDP panels with long > >> power-off / power-cycle delays: as part of disabling the output we > >> disable the panel power. The subsequent spurious short pulse handling > >> will have to turn the power back on, which means the driver has to do a > >> redundant wait for the power-off and power-cycle delays. During system > >> suspend this leads to an unnecessary delay up to ~1s on systems with > >> such panels as reported by Rui. > >> > >> To fix this put the sink to DPMS D3 state before turning off the port. > >> According to the DP spec in this state the sink should not request > >> retraining. This is also what we do already on pre-ddi platforms. > >> > >> As an alternative I also tried configuring the port to send idle pattern > >> - which is against BSPec - and leave the port in normal mode before > >> turning off the port. Neither of these resolved the problem. > >> > >> Cc: Zhang Rui > >> Cc: David Weinehall > >> Cc: Ville Syrjälä > >> Reported-and-tested-by: Zhang Rui > >> Signed-off-by: Imre Deak > > > > Makes sense to me. > > I wonder if we should write D0 on hotplug. D0 is the default power state IIRC, so when you plug something in it should automagically go into D0. That's actually a slight problem power-wise if there's no subsequent modeset to drop it into D3. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 5/5] ACPI: Switch to use generic guid_t in acpi_evaluate_dsm()
On Wed, May 31, 2017 at 10:41:52PM +0300, Andy Shevchenko wrote: > acpi_evaluate_dsm() and friends take a pointer to a raw buffer of 16 > bytes. Instead we convert them to use guid_t type. At the same time we > convert current users. > > acpi_str_to_uuid() becomes useless after the conversion and it's safe to > get rid of it. > > Cc: Borislav Petkov > Cc: Dan Williams > Cc: Amir Goldstein > Cc: Jarkko Sakkinen > Reviewed-by: Jani Nikula > Acked-by: Jani Nikula > Cc: Ben Skeggs > Acked-by: Benjamin Tissoires > Acked-by: Joerg Roedel > Cc: Adrian Hunter > Cc: Yisen Zhuang > Acked-by: Bjorn Helgaas > Acked-by: Felipe Balbi > Acked-by: Mathias Nyman > Reviewed-by: Heikki Krogerus > Cc: Liam Girdwood > Cc: Mark Brown > Signed-off-by: Andy Shevchenko Reviewed-by: Jarkko Sakkinen /Jarkko > --- > drivers/acpi/acpi_extlog.c | 4 ++-- > drivers/acpi/bus.c | 23 > -- > drivers/acpi/nfit/core.c | 6 +++--- > drivers/acpi/utils.c | 16 +++ > drivers/char/tpm/tpm_crb.c | 9 - > drivers/char/tpm/tpm_ppi.c | 20 --- > drivers/gpu/drm/i915/intel_acpi.c | 14 + > drivers/gpu/drm/nouveau/nouveau_acpi.c | 20 +-- > drivers/gpu/drm/nouveau/nvkm/subdev/mxm/base.c | 9 - > drivers/hid/i2c-hid/i2c-hid.c | 9 - > drivers/iommu/dmar.c | 11 +-- > drivers/mmc/host/sdhci-pci-core.c | 9 - > drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 15 +++--- > drivers/pci/pci-acpi.c | 13 ++-- > drivers/pci/pci-label.c| 4 ++-- > drivers/usb/dwc3/dwc3-pci.c| 10 +- > drivers/usb/host/xhci-pci.c| 9 - > drivers/usb/misc/ucsi.c| 6 +++--- > drivers/usb/typec/typec_wcove.c| 8 > include/acpi/acpi_bus.h| 11 ++- > include/linux/acpi.h | 3 +-- > include/linux/pci-acpi.h | 2 +- > sound/soc/intel/skylake/skl-nhlt.c | 7 --- > tools/testing/nvdimm/test/iomap.c | 6 +++--- > tools/testing/nvdimm/test/nfit.c | 2 +- > 25 files changed, 103 insertions(+), 143 deletions(-) > > diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c > index 6b101d595ccc..c9c1d2af9a13 100644 > --- a/drivers/acpi/acpi_extlog.c > +++ b/drivers/acpi/acpi_extlog.c > @@ -190,9 +190,9 @@ static bool __init extlog_get_l1addr(void) > return false; > if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle))) > return false; > - if (!acpi_check_dsm(handle, guid.b, EXTLOG_DSM_REV, 1 << > EXTLOG_FN_ADDR)) > + if (!acpi_check_dsm(handle, &guid, EXTLOG_DSM_REV, 1 << EXTLOG_FN_ADDR)) > return false; > - obj = acpi_evaluate_dsm_typed(handle, guid.b, EXTLOG_DSM_REV, > + obj = acpi_evaluate_dsm_typed(handle, &guid, EXTLOG_DSM_REV, > EXTLOG_FN_ADDR, NULL, ACPI_TYPE_INTEGER); > if (!obj) { > return false; > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index 042cd16265b3..5a6fbe0fcaf2 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -196,29 +196,6 @@ static void acpi_print_osc_error(acpi_handle handle, > pr_debug("\n"); > } > > -acpi_status acpi_str_to_uuid(char *str, u8 *uuid) > -{ > - int i; > - static int opc_map_to_uuid[16] = {6, 4, 2, 0, 11, 9, 16, 14, 19, 21, > - 24, 26, 28, 30, 32, 34}; > - > - if (strlen(str) != 36) > - return AE_BAD_PARAMETER; > - for (i = 0; i < 36; i++) { > - if (i == 8 || i == 13 || i == 18 || i == 23) { > - if (str[i] != '-') > - return AE_BAD_PARAMETER; > - } else if (!isxdigit(str[i])) > - return AE_BAD_PARAMETER; > - } > - for (i = 0; i < 16; i++) { > - uuid[i] = hex_to_bin(str[opc_map_to_uuid[i]]) << 4; > - uuid[i] |= hex_to_bin(str[opc_map_to_uuid[i] + 1]); > - } > - return AE_OK; > -} > -EXPORT_SYMBOL_GPL(acpi_str_to_uuid); > - > acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context > *context) > { > acpi_status status; > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c > index 50753582a0b1..56a5b2ca927a 100644 > --- a/drivers/acpi/nfit/core.c > +++ b/drivers/acpi/nfit/core.c > @@ -289,7 +289,7 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, > struct nvdimm *nvdimm, > in_buf.buffer.pointer, >
Re: [Intel-gfx] [PATCH] drm/i915/ddi: Avoid long delays during system suspend / eDP disabling
On Thu, 01 Jun 2017, Ville Syrjälä wrote: > On Thu, Jun 01, 2017 at 03:55:13PM +0300, Jani Nikula wrote: >> On Wed, 31 May 2017, Ville Syrjälä wrote: >> > On Wed, May 31, 2017 at 08:05:35PM +0300, Imre Deak wrote: >> >> Atm disabling either DP or eDP outputs can generate a spurious short >> >> pulse interrupt. The reason is that after disabling the port the source >> >> will stop sending a valid stream data, while the sink expects either a >> >> valid stream or the idle pattern. Since neither of this is sent the sink >> >> assumes (after an arbitrary delay) that the link is lost and requests >> >> for link retraining with a short pulse. >> >> >> >> The spurious pulse is a real problem at least for eDP panels with long >> >> power-off / power-cycle delays: as part of disabling the output we >> >> disable the panel power. The subsequent spurious short pulse handling >> >> will have to turn the power back on, which means the driver has to do a >> >> redundant wait for the power-off and power-cycle delays. During system >> >> suspend this leads to an unnecessary delay up to ~1s on systems with >> >> such panels as reported by Rui. >> >> >> >> To fix this put the sink to DPMS D3 state before turning off the port. >> >> According to the DP spec in this state the sink should not request >> >> retraining. This is also what we do already on pre-ddi platforms. >> >> >> >> As an alternative I also tried configuring the port to send idle pattern >> >> - which is against BSPec - and leave the port in normal mode before >> >> turning off the port. Neither of these resolved the problem. >> >> >> >> Cc: Zhang Rui >> >> Cc: David Weinehall >> >> Cc: Ville Syrjälä >> >> Reported-and-tested-by: Zhang Rui >> >> Signed-off-by: Imre Deak >> > >> > Makes sense to me. >> >> I wonder if we should write D0 on hotplug. > > D0 is the default power state IIRC, so when you plug something in it > should automagically go into D0. That's actually a slight problem > power-wise if there's no subsequent modeset to drop it into D3. Right. The thought was about branch devices that don't wake up and aren't disconnected/reconnected after D3. But then we don't receive HPD for them aither. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/ddi: Avoid long delays during system suspend / eDP disabling
On Thu, Jun 01, 2017 at 04:58:50PM +0300, Ville Syrjälä wrote: > On Thu, Jun 01, 2017 at 03:55:13PM +0300, Jani Nikula wrote: > > On Wed, 31 May 2017, Ville Syrjälä wrote: > > > On Wed, May 31, 2017 at 08:05:35PM +0300, Imre Deak wrote: > > >> Atm disabling either DP or eDP outputs can generate a spurious short > > >> pulse interrupt. The reason is that after disabling the port the source > > >> will stop sending a valid stream data, while the sink expects either a > > >> valid stream or the idle pattern. Since neither of this is sent the sink > > >> assumes (after an arbitrary delay) that the link is lost and requests > > >> for link retraining with a short pulse. > > >> > > >> The spurious pulse is a real problem at least for eDP panels with long > > >> power-off / power-cycle delays: as part of disabling the output we > > >> disable the panel power. The subsequent spurious short pulse handling > > >> will have to turn the power back on, which means the driver has to do a > > >> redundant wait for the power-off and power-cycle delays. During system > > >> suspend this leads to an unnecessary delay up to ~1s on systems with > > >> such panels as reported by Rui. > > >> > > >> To fix this put the sink to DPMS D3 state before turning off the port. > > >> According to the DP spec in this state the sink should not request > > >> retraining. This is also what we do already on pre-ddi platforms. > > >> > > >> As an alternative I also tried configuring the port to send idle pattern > > >> - which is against BSPec - and leave the port in normal mode before > > >> turning off the port. Neither of these resolved the problem. > > >> > > >> Cc: Zhang Rui > > >> Cc: David Weinehall > > >> Cc: Ville Syrjälä > > >> Reported-and-tested-by: Zhang Rui > > >> Signed-off-by: Imre Deak > > > > > > Makes sense to me. > > > > I wonder if we should write D0 on hotplug. > > D0 is the default power state IIRC, so when you plug something in it > should automagically go into D0. That's actually a slight problem > power-wise if there's no subsequent modeset to drop it into D3. There was this DP->VGA adaptor that seems to require that we set D0 explicitly: https://bugs.freedesktop.org/show_bug.cgi?id=99114#c5 But it's an odd behaviour, the DPCD read itself succeeds, only reading a certain register fails (DPCD_SINK_COUNT). Based on the DP spec a sink should respond to AUX transfers even after being put to D3, possibly with a longer wake-up delay (up to 20ms AFAIR). --Imre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Allow kswapd to pause the device whilst reaping
== Series Details == Series: series starting with [1/3] drm/i915: Allow kswapd to pause the device whilst reaping URL : https://patchwork.freedesktop.org/series/25164/ State : success == Summary == Series 25164v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/25164/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: pass -> FAIL (fi-snb-2600) fdo#17 Test kms_busy: Subgroup basic-flip-default-b: pass -> DMESG-WARN (fi-skl-6700hq) fdo#101144 +2 Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: pass -> FAIL (fi-skl-6700hq) fdo#101154 +7 fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fdo#101144 https://bugs.freedesktop.org/show_bug.cgi?id=101144 fdo#101154 https://bugs.freedesktop.org/show_bug.cgi?id=101154 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:444s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:431s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:580s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:513s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:487s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:488s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:432s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:420s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:416s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:495s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:472s fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 time:464s fi-kbl-7560u total:278 pass:263 dwarn:5 dfail:0 fail:0 skip:10 time:575s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:456s fi-skl-6700hqtotal:278 pass:229 dwarn:1 dfail:0 fail:26 skip:22 time:406s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:464s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:507s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:437s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:544s fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 time:404s 2c9abf8ec88cf4b3d3735976bb0ff7a4991946b2 drm-tip: 2017y-06m-01d-13h-29m-05s UTC integration manifest 507e749 drm/i915: Remove __GFP_NORETRY from our buffer allocator ec73274 drm/i915: Encourage our shrinker more when our shmemfs allocations fails 7247e82 drm/i915: Allow kswapd to pause the device whilst reaping == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4856/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 5/5] ACPI: Switch to use generic guid_t in acpi_evaluate_dsm()
On Wed, May 31, 2017 at 12:41 PM, Andy Shevchenko wrote: > acpi_evaluate_dsm() and friends take a pointer to a raw buffer of 16 > bytes. Instead we convert them to use guid_t type. At the same time we > convert current users. > > acpi_str_to_uuid() becomes useless after the conversion and it's safe to > get rid of it. > > Cc: Borislav Petkov > Cc: Dan Williams > Cc: Amir Goldstein > Cc: Jarkko Sakkinen > Reviewed-by: Jani Nikula > Acked-by: Jani Nikula > Cc: Ben Skeggs > Acked-by: Benjamin Tissoires > Acked-by: Joerg Roedel > Cc: Adrian Hunter > Cc: Yisen Zhuang > Acked-by: Bjorn Helgaas > Acked-by: Felipe Balbi > Acked-by: Mathias Nyman > Reviewed-by: Heikki Krogerus > Cc: Liam Girdwood > Cc: Mark Brown > Signed-off-by: Andy Shevchenko > --- > drivers/acpi/acpi_extlog.c | 4 ++-- > drivers/acpi/bus.c | 23 > -- > drivers/acpi/nfit/core.c | 6 +++--- Acked-by: Dan Williams ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/5] acpi, nfit: Switch to use new generic UUID API
On Wed, May 31, 2017 at 12:41 PM, Andy Shevchenko wrote: > There are new types and helpers that are supposed to be used in new code. > > As a preparation to get rid of legacy types and API functions do > the conversion here. > > Cc: Dan Williams > Signed-off-by: Andy Shevchenko > --- > drivers/acpi/nfit/core.c | 54 > > drivers/acpi/nfit/nfit.h | 3 +-- > include/linux/acpi.h | 1 + > 3 files changed, 29 insertions(+), 29 deletions(-) > Looks good to me. Reviewed-by: Dan Williams ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/7] drm/i915: Pipe A quirk rework
From: Ville Syrjälä This series eliminates the problematic load detect abuse for the pipe A quirk. My main motivations were to isolate these quirks more from atomic to avoid regressions, and to save a bit of extra power. I believe I cooked this up a few years ago but never posted it. In the meantine we had accumulated some more regressions in the existing code so I tossed in some fixes up front. Note that I'm entirely removing the few remaining pipe A quirks for non-830 platforms as they predate KMS and the hardware really shouldn't need them. Entire series available here: git://github.com/vsyrjala/linux.git alm_pipe_quirk_rework_4 Ville Syrjälä (7): drm/i915: Fix deadlock witha the pipe A quirk during resume drm/i915: Plumb the correct acquire ctx into intel_crtc_disable_noatomic() drm/i915: Use a loop for the "three times for luck" DPLL procedure drm/i915: Add i830 "pipes power well" drm/i915: Drop pipe A quirk for Toshiba Protege R205-S209 drm/i915: Drop pipe A quirk for Thinkapd T60 drm/i915: Remove pipe A quirk remnants drivers/gpu/drm/i915/i915_drv.h | 2 - drivers/gpu/drm/i915/intel_display.c| 209 +--- drivers/gpu/drm/i915/intel_drv.h| 2 + drivers/gpu/drm/i915/intel_overlay.c| 1 - drivers/gpu/drm/i915/intel_runtime_pm.c | 64 ++ 5 files changed, 177 insertions(+), 101 deletions(-) -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/7] drm/i915: Plumb the correct acquire ctx into intel_crtc_disable_noatomic()
From: Ville Syrjälä If intel_crtc_disable_noatomic() were to ever get called during resume e'd end up deadlocking since resume has its own acqcuire_ctx but intel_crtc_disable_noatomic() still tries to use the mode_config.acquire_ctx. Pass down the correct acquire ctx from the top. Cc: sta...@vger.kernel.org Cc: Maarten Lankhorst Fixes: e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.") Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2f76a63efe8c..4e3c64ed4131 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5839,7 +5839,8 @@ static void i9xx_crtc_disable(struct intel_crtc_state *old_crtc_state, intel_update_watermarks(intel_crtc); } -static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) +static void intel_crtc_disable_noatomic(struct drm_crtc *crtc, + struct drm_modeset_acquire_ctx *ctx) { struct intel_encoder *encoder; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); @@ -5869,7 +5870,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) return; } - state->acquire_ctx = crtc->dev->mode_config.acquire_ctx; + state->acquire_ctx = ctx; /* Everything's already locked, -EDEADLK can't happen. */ crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); @@ -15257,7 +15258,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc, plane = crtc->plane; crtc->base.primary->state->visible = true; crtc->plane = !plane; - intel_crtc_disable_noatomic(&crtc->base); + intel_crtc_disable_noatomic(&crtc->base, ctx); crtc->plane = plane; } @@ -15273,7 +15274,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc, /* Adjust the state of the output pipe according to whether we * have active connectors/encoders. */ if (crtc->active && !intel_crtc_has_encoders(crtc)) - intel_crtc_disable_noatomic(&crtc->base); + intel_crtc_disable_noatomic(&crtc->base, ctx); if (crtc->active || HAS_GMCH_DISPLAY(dev_priv)) { /* -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/7] drm/i915: Use a loop for the "three times for luck" DPLL procedure
From: Ville Syrjälä The magic "enable the DPLL three times" sequence feels like it deserves a loop. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4e3c64ed4131..2b112229b5b2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1550,6 +1550,7 @@ static void i9xx_enable_pll(struct intel_crtc *crtc) struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); i915_reg_t reg = DPLL(crtc->pipe); u32 dpll = crtc->config->dpll_hw_state.dpll; + int i; assert_pipe_disabled(dev_priv, crtc->pipe); @@ -1596,15 +1597,11 @@ static void i9xx_enable_pll(struct intel_crtc *crtc) } /* We do this three times for luck */ - I915_WRITE(reg, dpll); - POSTING_READ(reg); - udelay(150); /* wait for warmup */ - I915_WRITE(reg, dpll); - POSTING_READ(reg); - udelay(150); /* wait for warmup */ - I915_WRITE(reg, dpll); - POSTING_READ(reg); - udelay(150); /* wait for warmup */ + for (i = 0; i < 3; i++) { + I915_WRITE(reg, dpll); + POSTING_READ(reg); + udelay(150); /* wait for warmup */ + } } /** -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/7] drm/i915: Add i830 "pipes power well"
From: Ville Syrjälä 830 more or less requires both pipes and DPLLs to remain on as long as either pipe is needed. However, when neither pipe is actually needed, we can save a bit of power by turning everything off. To do that we add a new "power well" that turns both pipes and DPLLs on and off in the right order. Seems to save ~50mW on my Fujitsu-Siemens Lifebook S6010. This also avoids having to abuse the load detection to force pipe A on at init time. That was never very robust, and it only worked for one pipe, whereas 830 really needs both pipes enabled. As a bonus the 830 pipe quirk is now a bit more isolated from the rest of the mode setting infrastructure, which should mean that it's much less likely someone will accidentally break it in the future. The extra cost is of course slight code duplication, but that seems like a worthwile tradeoff here. v2; s/BIT/BIT_ULL/ Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c| 92 - drivers/gpu/drm/i915/intel_drv.h| 2 + drivers/gpu/drm/i915/intel_runtime_pm.c | 64 +++ 3 files changed, 157 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2b112229b5b2..e1e5926a40a1 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5834,6 +5834,10 @@ static void i9xx_crtc_disable(struct intel_crtc_state *old_crtc_state, if (!dev_priv->display.initial_watermarks) intel_update_watermarks(intel_crtc); + + /* clock the pipe down to 640x480@60 to potentially save power */ + if (IS_I830(dev_priv)) + i830_enable_pipe(dev_priv, pipe); } static void intel_crtc_disable_noatomic(struct drm_crtc *crtc, @@ -15130,6 +15134,91 @@ int intel_modeset_init(struct drm_device *dev) return 0; } +void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe) +{ + /* 640x480@60Hz, ~25175 kHz */ + struct dpll clock = { + .m1 = 18, + .m2 = 7, + .p1 = 13, + .p2 = 4, + .n = 2, + }; + u32 dpll, fp; + int i; + + WARN_ON(i9xx_calc_dpll_params(48000, &clock) != 25154); + + DRM_DEBUG_KMS("enabling pipe %c due to force quirk (vco=%d dot=%d)\n", + pipe_name(pipe), clock.vco, clock.dot); + + fp = i9xx_dpll_compute_fp(&clock); + dpll = (I915_READ(DPLL(pipe)) & DPLL_DVO_2X_MODE) | + DPLL_VGA_MODE_DIS | + ((clock.p1 - 2) << DPLL_FPA01_P1_POST_DIV_SHIFT) | + PLL_P2_DIVIDE_BY_4 | + PLL_REF_INPUT_DREFCLK | + DPLL_VCO_ENABLE; + + I915_WRITE(FP0(pipe), fp); + I915_WRITE(FP1(pipe), fp); + + I915_WRITE(HTOTAL(pipe), (640 - 1) | ((800 - 1) << 16)); + I915_WRITE(HBLANK(pipe), (640 - 1) | ((800 - 1) << 16)); + I915_WRITE(HSYNC(pipe), (656 - 1) | ((752 - 1) << 16)); + I915_WRITE(VTOTAL(pipe), (480 - 1) | ((525 - 1) << 16)); + I915_WRITE(VBLANK(pipe), (480 - 1) | ((525 - 1) << 16)); + I915_WRITE(VSYNC(pipe), (490 - 1) | ((492 - 1) << 16)); + I915_WRITE(PIPESRC(pipe), ((640 - 1) << 16) | (480 - 1)); + + /* +* Apparently we need to have VGA mode enabled prior to changing +* the P1/P2 dividers. Otherwise the DPLL will keep using the old +* dividers, even though the register value does change. +*/ + I915_WRITE(DPLL(pipe), dpll & ~DPLL_VGA_MODE_DIS); + I915_WRITE(DPLL(pipe), dpll); + + /* Wait for the clocks to stabilize. */ + POSTING_READ(DPLL(pipe)); + udelay(150); + + /* The pixel multiplier can only be updated once the +* DPLL is enabled and the clocks are stable. +* +* So write it again. +*/ + I915_WRITE(DPLL(pipe), dpll); + + /* We do this three times for luck */ + for (i = 0; i < 3 ; i++) { + I915_WRITE(DPLL(pipe), dpll); + POSTING_READ(DPLL(pipe)); + udelay(150); /* wait for warmup */ + } + + I915_WRITE(PIPECONF(pipe), PIPECONF_ENABLE | PIPECONF_PROGRESSIVE); + POSTING_READ(PIPECONF(pipe)); +} + +void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe) +{ + DRM_DEBUG_KMS("disabling pipe %c due to force quirk\n", + pipe_name(pipe)); + + assert_plane_disabled(dev_priv, PLANE_A); + assert_plane_disabled(dev_priv, PLANE_B); + + I915_WRITE(PIPECONF(pipe), 0); + POSTING_READ(PIPECONF(pipe)); + + if (wait_for(pipe_dsl_stopped(dev_priv, pipe), 100)) + DRM_ERROR("pipe %c off wait timed out\n", pipe_name(pipe)); + + I915_WRITE(DPLL(pipe), DPLL_VGA_MODE_DIS); + POSTING_READ(DPLL(pipe)); +} + static void intel_enable_pipe_a(struct drm_device *dev, struct drm_modeset_acquire_ct
[Intel-gfx] [PATCH 1/7] drm/i915: Fix deadlock witha the pipe A quirk during resume
From: Ville Syrjälä Pass down the correct acquire context to the pipe A quirk load detect hack during display resume. Avoids deadlocking the entire thing. Cc: sta...@vger.kernel.org Cc: Maarten Lankhorst Fixes: e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.") Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 21 - 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ed41b59ee8e3..2f76a63efe8c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -120,7 +120,8 @@ static void intel_crtc_init_scalers(struct intel_crtc *crtc, static void skylake_pfit_enable(struct intel_crtc *crtc); static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force); static void ironlake_pfit_enable(struct intel_crtc *crtc); -static void intel_modeset_setup_hw_state(struct drm_device *dev); +static void intel_modeset_setup_hw_state(struct drm_device *dev, +struct drm_modeset_acquire_ctx *ctx); static void intel_pre_disable_primary_noatomic(struct drm_crtc *crtc); struct intel_limit { @@ -3461,7 +3462,7 @@ __intel_display_resume(struct drm_device *dev, struct drm_crtc *crtc; int i, ret; - intel_modeset_setup_hw_state(dev); + intel_modeset_setup_hw_state(dev, ctx); i915_redisable_vga(to_i915(dev)); if (!state) @@ -15094,7 +15095,7 @@ int intel_modeset_init(struct drm_device *dev) intel_setup_outputs(dev_priv); drm_modeset_lock_all(dev); - intel_modeset_setup_hw_state(dev); + intel_modeset_setup_hw_state(dev, dev->mode_config.acquire_ctx); drm_modeset_unlock_all(dev); for_each_intel_crtc(dev, crtc) { @@ -15131,13 +15132,13 @@ int intel_modeset_init(struct drm_device *dev) return 0; } -static void intel_enable_pipe_a(struct drm_device *dev) +static void intel_enable_pipe_a(struct drm_device *dev, + struct drm_modeset_acquire_ctx *ctx) { struct intel_connector *connector; struct drm_connector_list_iter conn_iter; struct drm_connector *crt = NULL; struct intel_load_detect_pipe load_detect_temp; - struct drm_modeset_acquire_ctx *ctx = dev->mode_config.acquire_ctx; int ret; /* We can't just switch on the pipe A, we need to set things up with a @@ -15209,7 +15210,8 @@ static bool has_pch_trancoder(struct drm_i915_private *dev_priv, (HAS_PCH_LPT_H(dev_priv) && pch_transcoder == TRANSCODER_A); } -static void intel_sanitize_crtc(struct intel_crtc *crtc) +static void intel_sanitize_crtc(struct intel_crtc *crtc, + struct drm_modeset_acquire_ctx *ctx) { struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = to_i915(dev); @@ -15265,7 +15267,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) * resume. Force-enable the pipe to fix this, the update_dpms * call below we restore the pipe to the right state, but leave * the required bits on. */ - intel_enable_pipe_a(dev); + intel_enable_pipe_a(dev, ctx); } /* Adjust the state of the output pipe according to whether we @@ -15568,7 +15570,8 @@ get_encoder_power_domains(struct drm_i915_private *dev_priv) * and sanitizes it to the current state */ static void -intel_modeset_setup_hw_state(struct drm_device *dev) +intel_modeset_setup_hw_state(struct drm_device *dev, +struct drm_modeset_acquire_ctx *ctx) { struct drm_i915_private *dev_priv = to_i915(dev); enum pipe pipe; @@ -15588,7 +15591,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev) for_each_pipe(dev_priv, pipe) { crtc = intel_get_crtc_for_pipe(dev_priv, pipe); - intel_sanitize_crtc(crtc); + intel_sanitize_crtc(crtc, ctx); intel_dump_pipe_config(crtc, crtc->config, "[setup_hw_state]"); } -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/7] drm/i915: Drop pipe A quirk for Toshiba Protege R205-S209
From: Ville Syrjälä The pipe A force quirk shouldn't needed except on 830. So let's nuke it for the Toshiba Protege R-205/S-209 945 machines. This quirk pre-dates KMS so it's usefulness is doubtful at best now. Unfortunately the original bug report [1] isn't very helpful since it doesn't describe the symptoms. And the commit message in xf86-video-intel commit ecdb5963ef68 ("Add pipe A force enable quirk for Toshiba Portege R205-S209") is not much help either. However, if we assume the problem was the typical "closing the lid hangs the box" type of thing, we already nuked the quirk for another 945 machine in commit 736a69ca8c99 ("drm/i915: Drop PIPE-A quirk for 945GSE HP Mini") and so I hope we can drop this one as well. [1] https://bugs.freedesktop.org/show_bug.cgi?id=14944 Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e1e5926a40a1..31e534c893ce 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14802,9 +14802,6 @@ static const struct intel_dmi_quirk intel_dmi_quirks[] = { }; static struct intel_quirk intel_quirks[] = { - /* Toshiba Protege R-205, S-209 needs pipe A force quirk */ - { 0x2592, 0x1179, 0x0001, quirk_pipea_force }, - /* ThinkPad T60 needs pipe A force quirk (bug #16494) */ { 0x2782, 0x17aa, 0x201a, quirk_pipea_force }, -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/7] drm/i915: Remove pipe A quirk remnants
From: Ville Syrjälä With 830 the only thing needing pipe quirks, we can just drop the quirk defines and replace the checks with IS_I830() checks. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.h | 2 - drivers/gpu/drm/i915/intel_display.c | 92 drivers/gpu/drm/i915/intel_overlay.c | 1 - 3 files changed, 10 insertions(+), 85 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index bde554eb2257..4e5db51803f8 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1159,11 +1159,9 @@ enum intel_sbi_destination { SBI_MPHY, }; -#define QUIRK_PIPEA_FORCE (1<<0) #define QUIRK_LVDS_SSC_DISABLE (1<<1) #define QUIRK_INVERT_BRIGHTNESS (1<<2) #define QUIRK_BACKLIGHT_PRESENT (1<<3) -#define QUIRK_PIPEB_FORCE (1<<4) #define QUIRK_PIN_SWIZZLED_PAGES (1<<5) struct intel_fbdev; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 935e600ab179..774a6e523980 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1193,9 +1193,8 @@ void assert_pipe(struct drm_i915_private *dev_priv, pipe); enum intel_display_power_domain power_domain; - /* if we need the pipe quirk it must be always on */ - if ((pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) || - (pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE)) + /* we keep both pipes enabled on 830 */ + if (IS_I830(dev_priv)) state = true; power_domain = POWER_DOMAIN_TRANSCODER(cpu_transcoder); @@ -1629,8 +1628,7 @@ static void i9xx_disable_pll(struct intel_crtc *crtc) } /* Don't disable pipe or pipe PLLs if needed */ - if ((pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) || - (pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE)) + if (IS_I830(dev_priv)) return; /* Make sure the pipe isn't still relying on us */ @@ -1913,8 +1911,8 @@ static void intel_enable_pipe(struct intel_crtc *crtc) reg = PIPECONF(cpu_transcoder); val = I915_READ(reg); if (val & PIPECONF_ENABLE) { - WARN_ON(!((pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) || - (pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))); + /* we keep both pipes enabled on 830 */ + WARN_ON(!IS_I830(dev_priv)); return; } @@ -1974,8 +1972,7 @@ static void intel_disable_pipe(struct intel_crtc *crtc) val &= ~PIPECONF_DOUBLE_WIDE; /* Don't disable pipe or pipe PLLs if needed */ - if (!(pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) && - !(pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE)) + if (!IS_I830(dev_priv)) val &= ~PIPECONF_ENABLE; I915_WRITE(reg, val); @@ -7049,8 +7046,8 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc) pipeconf = 0; - if ((intel_crtc->pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) || - (intel_crtc->pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE)) + /* we keep both pipes enabled on 830 */ + if (IS_I830(dev_priv)) pipeconf |= I915_READ(PIPECONF(intel_crtc->pipe)) & PIPECONF_ENABLE; if (intel_crtc->config->double_wide) @@ -12220,9 +12217,8 @@ verify_crtc_state(struct drm_crtc *crtc, active = dev_priv->display.get_pipe_config(intel_crtc, pipe_config); - /* hw state is inconsistent with the pipe quirk */ - if ((intel_crtc->pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) || - (intel_crtc->pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE)) + /* we keep both pipes enabled on 830 */ + if (IS_I830(dev_priv)) active = new_crtc_state->active; I915_STATE_WARN(new_crtc_state->active != active, @@ -14717,27 +14713,6 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv) } /* - * Some BIOSes insist on assuming the GPU's pipe A is enabled at suspend, - * resume, or other times. This quirk makes sure that's the case for - * affected systems. - */ -static void quirk_pipea_force(struct drm_device *dev) -{ - struct drm_i915_private *dev_priv = to_i915(dev); - - dev_priv->quirks |= QUIRK_PIPEA_FORCE; - DRM_INFO("applying pipe a force quirk\n"); -} - -static void quirk_pipeb_force(struct drm_device *dev) -{ - struct drm_i915_private *dev_priv = to_i915(dev); - - dev_priv->quirks |= QUIRK_PIPEB_FORCE; - DRM_INFO("applying pipe b force quirk\n"); -} - -/* * Some machines (Lenovo U160) do not work with SSC on LVDS for some reason */ static void quirk_ssc_force_disable(struct drm_device *dev) @@ -14802,12 +14777,6 @@ stat
[Intel-gfx] [PATCH 6/7] drm/i915: Drop pipe A quirk for Thinkapd T60
From: Ville Syrjälä The pipe A force quirk shouldn't needed except on 830. So let's nuke it for the IBM Thinkpad T60 945 machines. This quirk pre-dates KMS so it's usefulness is doubtful at best now. The original bug report [1] describes the symptoms as "system hang on closing T60 panel lid", and we already dropped a similar quirk for another 945 machine in commit 736a69ca8c99 ("drm/i915: Drop PIPE-A quirk for 945GSE HP Mini") so I'm hopeful we can drop this one as well. The quirk was added into xf86-video-intel in commit 08903abe4dc0 ("Add pipe a force enable quirk for Lenovo T60") [1] https://bugs.freedesktop.org/show_bug.cgi?id=16494 Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 31e534c893ce..935e600ab179 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14802,9 +14802,6 @@ static const struct intel_dmi_quirk intel_dmi_quirks[] = { }; static struct intel_quirk intel_quirks[] = { - /* ThinkPad T60 needs pipe A force quirk (bug #16494) */ - { 0x2782, 0x17aa, 0x201a, quirk_pipea_force }, - /* 830 needs to leave pipe A & dpll A up */ { 0x3577, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force }, -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/ddi: Avoid long delays during system suspend / eDP disabling
On Thu, 01 Jun 2017, Imre Deak wrote: > On Thu, Jun 01, 2017 at 04:58:50PM +0300, Ville Syrjälä wrote: >> On Thu, Jun 01, 2017 at 03:55:13PM +0300, Jani Nikula wrote: >> > On Wed, 31 May 2017, Ville Syrjälä wrote: >> > > On Wed, May 31, 2017 at 08:05:35PM +0300, Imre Deak wrote: >> > >> Atm disabling either DP or eDP outputs can generate a spurious short >> > >> pulse interrupt. The reason is that after disabling the port the source >> > >> will stop sending a valid stream data, while the sink expects either a >> > >> valid stream or the idle pattern. Since neither of this is sent the sink >> > >> assumes (after an arbitrary delay) that the link is lost and requests >> > >> for link retraining with a short pulse. >> > >> >> > >> The spurious pulse is a real problem at least for eDP panels with long >> > >> power-off / power-cycle delays: as part of disabling the output we >> > >> disable the panel power. The subsequent spurious short pulse handling >> > >> will have to turn the power back on, which means the driver has to do a >> > >> redundant wait for the power-off and power-cycle delays. During system >> > >> suspend this leads to an unnecessary delay up to ~1s on systems with >> > >> such panels as reported by Rui. >> > >> >> > >> To fix this put the sink to DPMS D3 state before turning off the port. >> > >> According to the DP spec in this state the sink should not request >> > >> retraining. This is also what we do already on pre-ddi platforms. >> > >> >> > >> As an alternative I also tried configuring the port to send idle pattern >> > >> - which is against BSPec - and leave the port in normal mode before >> > >> turning off the port. Neither of these resolved the problem. >> > >> >> > >> Cc: Zhang Rui >> > >> Cc: David Weinehall >> > >> Cc: Ville Syrjälä >> > >> Reported-and-tested-by: Zhang Rui >> > >> Signed-off-by: Imre Deak >> > > >> > > Makes sense to me. >> > >> > I wonder if we should write D0 on hotplug. >> >> D0 is the default power state IIRC, so when you plug something in it >> should automagically go into D0. That's actually a slight problem >> power-wise if there's no subsequent modeset to drop it into D3. > > There was this DP->VGA adaptor that seems to require that we set D0 > explicitly: > https://bugs.freedesktop.org/show_bug.cgi?id=99114#c5 > > But it's an odd behaviour, the DPCD read itself succeeds, only reading a > certain register fails (DPCD_SINK_COUNT). Based on the DP spec a sink > should respond to AUX transfers even after being put to D3, possibly > with a longer wake-up delay (up to 20ms AFAIR). Also [1] where AFAICT there isn't even a hotplug after D3. BR, Jani. [1] https://bugs.freedesktop.org/show_bug.cgi?id=101044 -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/7] drm/i915: Pipe A quirk rework
On Thu, Jun 01, 2017 at 05:36:12PM +0300, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Note that I'm entirely removing the few remaining pipe A quirks > for non-830 platforms as they predate KMS and the hardware > really shouldn't need them. > drm/i915: Drop pipe A quirk for Toshiba Protege R205-S209 > drm/i915: Drop pipe A quirk for Thinkapd T60 > drm/i915: Remove pipe A quirk remnants Acked-by: Chris Wilson I've wanted to do that for a long time. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Plumb the correct acquire ctx into intel_crtc_disable_noatomic()
On Thu, 01 Jun 2017, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > If intel_crtc_disable_noatomic() were to ever get called during resume > e'd end up deadlocking since resume has its own acqcuire_ctx but > intel_crtc_disable_noatomic() still tries to use the > mode_config.acquire_ctx. Pass down the correct acquire ctx from the top. Same here, have we seen this, or "just" theoretical? BR, Jani. > > Cc: sta...@vger.kernel.org > Cc: Maarten Lankhorst > Fixes: e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.") > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_display.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 2f76a63efe8c..4e3c64ed4131 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5839,7 +5839,8 @@ static void i9xx_crtc_disable(struct intel_crtc_state > *old_crtc_state, > intel_update_watermarks(intel_crtc); > } > > -static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) > +static void intel_crtc_disable_noatomic(struct drm_crtc *crtc, > + struct drm_modeset_acquire_ctx *ctx) > { > struct intel_encoder *encoder; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > @@ -5869,7 +5870,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc > *crtc) > return; > } > > - state->acquire_ctx = crtc->dev->mode_config.acquire_ctx; > + state->acquire_ctx = ctx; > > /* Everything's already locked, -EDEADLK can't happen. */ > crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); > @@ -15257,7 +15258,7 @@ static void intel_sanitize_crtc(struct intel_crtc > *crtc, > plane = crtc->plane; > crtc->base.primary->state->visible = true; > crtc->plane = !plane; > - intel_crtc_disable_noatomic(&crtc->base); > + intel_crtc_disable_noatomic(&crtc->base, ctx); > crtc->plane = plane; > } > > @@ -15273,7 +15274,7 @@ static void intel_sanitize_crtc(struct intel_crtc > *crtc, > /* Adjust the state of the output pipe according to whether we >* have active connectors/encoders. */ > if (crtc->active && !intel_crtc_has_encoders(crtc)) > - intel_crtc_disable_noatomic(&crtc->base); > + intel_crtc_disable_noatomic(&crtc->base, ctx); > > if (crtc->active || HAS_GMCH_DISPLAY(dev_priv)) { > /* -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/7] drm/i915: Fix deadlock witha the pipe A quirk during resume
On Thu, 01 Jun 2017, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Pass down the correct acquire context to the pipe A quirk load detect > hack during display resume. Avoids deadlocking the entire thing. Have we seen this in the wild? References? BR, Jani. > > Cc: sta...@vger.kernel.org > Cc: Maarten Lankhorst > Fixes: e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.") > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_display.c | 21 - > 1 file changed, 12 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index ed41b59ee8e3..2f76a63efe8c 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -120,7 +120,8 @@ static void intel_crtc_init_scalers(struct intel_crtc > *crtc, > static void skylake_pfit_enable(struct intel_crtc *crtc); > static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force); > static void ironlake_pfit_enable(struct intel_crtc *crtc); > -static void intel_modeset_setup_hw_state(struct drm_device *dev); > +static void intel_modeset_setup_hw_state(struct drm_device *dev, > + struct drm_modeset_acquire_ctx *ctx); > static void intel_pre_disable_primary_noatomic(struct drm_crtc *crtc); > > struct intel_limit { > @@ -3461,7 +3462,7 @@ __intel_display_resume(struct drm_device *dev, > struct drm_crtc *crtc; > int i, ret; > > - intel_modeset_setup_hw_state(dev); > + intel_modeset_setup_hw_state(dev, ctx); > i915_redisable_vga(to_i915(dev)); > > if (!state) > @@ -15094,7 +15095,7 @@ int intel_modeset_init(struct drm_device *dev) > intel_setup_outputs(dev_priv); > > drm_modeset_lock_all(dev); > - intel_modeset_setup_hw_state(dev); > + intel_modeset_setup_hw_state(dev, dev->mode_config.acquire_ctx); > drm_modeset_unlock_all(dev); > > for_each_intel_crtc(dev, crtc) { > @@ -15131,13 +15132,13 @@ int intel_modeset_init(struct drm_device *dev) > return 0; > } > > -static void intel_enable_pipe_a(struct drm_device *dev) > +static void intel_enable_pipe_a(struct drm_device *dev, > + struct drm_modeset_acquire_ctx *ctx) > { > struct intel_connector *connector; > struct drm_connector_list_iter conn_iter; > struct drm_connector *crt = NULL; > struct intel_load_detect_pipe load_detect_temp; > - struct drm_modeset_acquire_ctx *ctx = dev->mode_config.acquire_ctx; > int ret; > > /* We can't just switch on the pipe A, we need to set things up with a > @@ -15209,7 +15210,8 @@ static bool has_pch_trancoder(struct drm_i915_private > *dev_priv, > (HAS_PCH_LPT_H(dev_priv) && pch_transcoder == TRANSCODER_A); > } > > -static void intel_sanitize_crtc(struct intel_crtc *crtc) > +static void intel_sanitize_crtc(struct intel_crtc *crtc, > + struct drm_modeset_acquire_ctx *ctx) > { > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = to_i915(dev); > @@ -15265,7 +15267,7 @@ static void intel_sanitize_crtc(struct intel_crtc > *crtc) >* resume. Force-enable the pipe to fix this, the update_dpms >* call below we restore the pipe to the right state, but leave >* the required bits on. */ > - intel_enable_pipe_a(dev); > + intel_enable_pipe_a(dev, ctx); > } > > /* Adjust the state of the output pipe according to whether we > @@ -15568,7 +15570,8 @@ get_encoder_power_domains(struct drm_i915_private > *dev_priv) > * and sanitizes it to the current state > */ > static void > -intel_modeset_setup_hw_state(struct drm_device *dev) > +intel_modeset_setup_hw_state(struct drm_device *dev, > + struct drm_modeset_acquire_ctx *ctx) > { > struct drm_i915_private *dev_priv = to_i915(dev); > enum pipe pipe; > @@ -15588,7 +15591,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev) > for_each_pipe(dev_priv, pipe) { > crtc = intel_get_crtc_for_pipe(dev_priv, pipe); > > - intel_sanitize_crtc(crtc); > + intel_sanitize_crtc(crtc, ctx); > intel_dump_pipe_config(crtc, crtc->config, > "[setup_hw_state]"); > } -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Use a loop for the "three times for luck" DPLL procedure
On Thu, 01 Jun 2017, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > The magic "enable the DPLL three times" sequence feels like it > deserves a loop. I think the copy-paste unrolled loop is more fun. ;) Reviewed-by: Jani Nikula > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_display.c | 15 ++- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 4e3c64ed4131..2b112229b5b2 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -1550,6 +1550,7 @@ static void i9xx_enable_pll(struct intel_crtc *crtc) > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > i915_reg_t reg = DPLL(crtc->pipe); > u32 dpll = crtc->config->dpll_hw_state.dpll; > + int i; > > assert_pipe_disabled(dev_priv, crtc->pipe); > > @@ -1596,15 +1597,11 @@ static void i9xx_enable_pll(struct intel_crtc *crtc) > } > > /* We do this three times for luck */ > - I915_WRITE(reg, dpll); > - POSTING_READ(reg); > - udelay(150); /* wait for warmup */ > - I915_WRITE(reg, dpll); > - POSTING_READ(reg); > - udelay(150); /* wait for warmup */ > - I915_WRITE(reg, dpll); > - POSTING_READ(reg); > - udelay(150); /* wait for warmup */ > + for (i = 0; i < 3; i++) { > + I915_WRITE(reg, dpll); > + POSTING_READ(reg); > + udelay(150); /* wait for warmup */ > + } > } > > /** -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915: Add i830 "pipes power well"
On Thu, Jun 01, 2017 at 05:36:16PM +0300, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > 830 more or less requires both pipes and DPLLs to remain on as long > as either pipe is needed. However, when neither pipe is actually needed, > we can save a bit of power by turning everything off. To do that we add > a new "power well" that turns both pipes and DPLLs on and off in the > right order. Seems to save ~50mW on my Fujitsu-Siemens Lifebook S6010. > > This also avoids having to abuse the load detection to force pipe A on > at init time. That was never very robust, and it only worked for one > pipe, whereas 830 really needs both pipes enabled. As a bonus the 830 > pipe quirk is now a bit more isolated from the rest of the mode setting > infrastructure, which should mean that it's much less likely someone > will accidentally break it in the future. The extra cost is of course > slight code duplication, but that seems like a worthwile tradeoff here. Defining it as a powerwell is an interesting way to do it, and seems quite apt. My only nit is a request not to add to intel_display.c if we can place it elsewhere, intel_gen2_pm.c ? gen2_runtime_pm.c? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Use a loop for the "three times for luck" DPLL procedure
On Thu, Jun 01, 2017 at 05:36:15PM +0300, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > The magic "enable the DPLL three times" sequence feels like it > deserves a loop. Hmm, I thought once upon a time we figured out what the magic was about. Or was that another loop? Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Pipe A quirk rework
== Series Details == Series: drm/i915: Pipe A quirk rework URL : https://patchwork.freedesktop.org/series/25169/ State : success == Summary == Series 25169v1 drm/i915: Pipe A quirk rework https://patchwork.freedesktop.org/api/1.0/series/25169/revisions/1/mbox/ fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:441s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:434s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:588s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:519s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:490s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:489s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:432s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:414s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:417s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:495s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:462s fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 time:463s fi-kbl-7560u total:278 pass:263 dwarn:5 dfail:0 fail:0 skip:10 time:567s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:454s fi-skl-6700hqtotal:278 pass:239 dwarn:0 dfail:1 fail:17 skip:21 time:432s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:464s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:501s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:435s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:533s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:411s 2c9abf8ec88cf4b3d3735976bb0ff7a4991946b2 drm-tip: 2017y-06m-01d-13h-29m-05s UTC integration manifest ff4fbbc drm/i915: Remove pipe A quirk remnants 7b6bcec drm/i915: Drop pipe A quirk for Thinkapd T60 24fa310 drm/i915: Drop pipe A quirk for Toshiba Protege R205-S209 32f154c drm/i915: Add i830 "pipes power well" 132e825 drm/i915: Use a loop for the "three times for luck" DPLL procedure 15ac5e0 drm/i915: Plumb the correct acquire ctx into intel_crtc_disable_noatomic() 7efca7d drm/i915: Fix deadlock witha the pipe A quirk during resume == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4857/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RESEND] drm: i915: Preserve old FBC status for update without new planes
If the atomic commit doesn't include any new plane, there is no need to choose a new CRTC for FBC, and the intel_fbc_choose_crtc() will bail out early. Although, if the FBC setup failed in the previous commit, if the current commit doesn't include new plane update, it tries to overwrite no_fbc_reason to "no suitable CRTC for FBC". For that scenario, it is better that we simply keep the old message in-place to make debugging easier. A scenario where this happens is with the igt@kms_frontbuffer_tracking@fbc-suspend testcase when executed on a Haswell system with not enough stolen memory. When enabling the CRTC, the FBC setup will be correctly initialized to a specific CRTC, but won't be enabled, since there is not enough memory. The testcase will then enable CRC checking, which requires a quirk for Haswell, which issues a new atomic commit that doesn't update the planes. Since that update doesn't include any new planes (and the FBC wasn't enabled), intel_fbc_choose_crtc() will not find any suitable CRTC, but update the error message, hiding the lack of memory information, which is the actual cause of the initialization failure. As a result, this causes that test, which should skip, to fail on Haswell. Changes since v1: - Update commit message (Manasi) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100020 Fixes: f7e9b004b8a3 ("drm/i915/fbc: inline intel_fbc_can_choose()") Reported-by: Dorota Czaplejewicz Signed-off-by: Gabriel Krisman Bertazi Cc: Paulo Zanoni --- drivers/gpu/drm/i915/intel_fbc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index ded2add18b26..0c99c9b731ee 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -1045,6 +1045,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv, struct drm_plane *plane; struct drm_plane_state *plane_state; bool crtc_chosen = false; + bool new_planes = false; int i; mutex_lock(&fbc->lock); @@ -1066,6 +1067,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv, to_intel_plane_state(plane_state); struct intel_crtc_state *intel_crtc_state; struct intel_crtc *crtc = to_intel_crtc(plane_state->crtc); + new_planes = true; if (!intel_plane_state->base.visible) continue; @@ -1084,7 +1086,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv, break; } - if (!crtc_chosen) + if (new_planes && !crtc_chosen) fbc->no_fbc_reason = "no suitable CRTC for FBC"; out: -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 30/37] drm/sti: Drop drm_vblank_cleanup
On 05/24/2017 04:52 PM, Daniel Vetter wrote: > Seems entirely cargo-culted. > > Cc: Benjamin Gaignard > Cc: Vincent Abriou > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/sti/sti_drv.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c > index a4b574283269..06ef1e3886cf 100644 > --- a/drivers/gpu/drm/sti/sti_drv.c > +++ b/drivers/gpu/drm/sti/sti_drv.c > @@ -237,7 +237,6 @@ static void sti_cleanup(struct drm_device *ddev) > } > > drm_kms_helper_poll_fini(ddev); > - drm_vblank_cleanup(ddev); > component_unbind_all(ddev->dev, ddev); > kfree(private); > ddev->dev_private = NULL; > Acked-by: Vincent Abriou BR Vincent ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm: i915: Preserve old FBC status for update without new planes (rev2)
== Series Details == Series: drm: i915: Preserve old FBC status for update without new planes (rev2) URL : https://patchwork.freedesktop.org/series/24635/ State : success == Summary == Series 24635v2 drm: i915: Preserve old FBC status for update without new planes https://patchwork.freedesktop.org/api/1.0/series/24635/revisions/2/mbox/ Test kms_busy: Subgroup basic-flip-default-b: pass -> DMESG-WARN (fi-skl-6700hq) fdo#101144 +2 Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: pass -> FAIL (fi-skl-6700hq) fdo#101154 +7 fdo#101144 https://bugs.freedesktop.org/show_bug.cgi?id=101144 fdo#101154 https://bugs.freedesktop.org/show_bug.cgi?id=101154 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:439s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:430s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:574s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:507s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:485s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:489s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:434s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:414s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:419s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:490s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:460s fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 time:469s fi-kbl-7560u total:278 pass:263 dwarn:5 dfail:0 fail:0 skip:10 time:570s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:469s fi-skl-6700hqtotal:278 pass:229 dwarn:1 dfail:0 fail:26 skip:22 time:408s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:464s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:502s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:440s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:532s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:400s 2c9abf8ec88cf4b3d3735976bb0ff7a4991946b2 drm-tip: 2017y-06m-01d-13h-29m-05s UTC integration manifest 72395e7 drm: i915: Preserve old FBC status for update without new planes == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4858/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/7] drm/i915: Fix deadlock witha the pipe A quirk during resume
On Thu, Jun 01, 2017 at 05:47:45PM +0300, Jani Nikula wrote: > On Thu, 01 Jun 2017, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > Pass down the correct acquire context to the pipe A quirk load detect > > hack during display resume. Avoids deadlocking the entire thing. > > Have we seen this in the wild? References? My 830. > > BR, > Jani. > > > > > Cc: sta...@vger.kernel.org > > Cc: Maarten Lankhorst > > Fixes: e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.") > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/intel_display.c | 21 - > > 1 file changed, 12 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index ed41b59ee8e3..2f76a63efe8c 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -120,7 +120,8 @@ static void intel_crtc_init_scalers(struct intel_crtc > > *crtc, > > static void skylake_pfit_enable(struct intel_crtc *crtc); > > static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force); > > static void ironlake_pfit_enable(struct intel_crtc *crtc); > > -static void intel_modeset_setup_hw_state(struct drm_device *dev); > > +static void intel_modeset_setup_hw_state(struct drm_device *dev, > > +struct drm_modeset_acquire_ctx *ctx); > > static void intel_pre_disable_primary_noatomic(struct drm_crtc *crtc); > > > > struct intel_limit { > > @@ -3461,7 +3462,7 @@ __intel_display_resume(struct drm_device *dev, > > struct drm_crtc *crtc; > > int i, ret; > > > > - intel_modeset_setup_hw_state(dev); > > + intel_modeset_setup_hw_state(dev, ctx); > > i915_redisable_vga(to_i915(dev)); > > > > if (!state) > > @@ -15094,7 +15095,7 @@ int intel_modeset_init(struct drm_device *dev) > > intel_setup_outputs(dev_priv); > > > > drm_modeset_lock_all(dev); > > - intel_modeset_setup_hw_state(dev); > > + intel_modeset_setup_hw_state(dev, dev->mode_config.acquire_ctx); > > drm_modeset_unlock_all(dev); > > > > for_each_intel_crtc(dev, crtc) { > > @@ -15131,13 +15132,13 @@ int intel_modeset_init(struct drm_device *dev) > > return 0; > > } > > > > -static void intel_enable_pipe_a(struct drm_device *dev) > > +static void intel_enable_pipe_a(struct drm_device *dev, > > + struct drm_modeset_acquire_ctx *ctx) > > { > > struct intel_connector *connector; > > struct drm_connector_list_iter conn_iter; > > struct drm_connector *crt = NULL; > > struct intel_load_detect_pipe load_detect_temp; > > - struct drm_modeset_acquire_ctx *ctx = dev->mode_config.acquire_ctx; > > int ret; > > > > /* We can't just switch on the pipe A, we need to set things up with a > > @@ -15209,7 +15210,8 @@ static bool has_pch_trancoder(struct > > drm_i915_private *dev_priv, > > (HAS_PCH_LPT_H(dev_priv) && pch_transcoder == TRANSCODER_A); > > } > > > > -static void intel_sanitize_crtc(struct intel_crtc *crtc) > > +static void intel_sanitize_crtc(struct intel_crtc *crtc, > > + struct drm_modeset_acquire_ctx *ctx) > > { > > struct drm_device *dev = crtc->base.dev; > > struct drm_i915_private *dev_priv = to_i915(dev); > > @@ -15265,7 +15267,7 @@ static void intel_sanitize_crtc(struct intel_crtc > > *crtc) > > * resume. Force-enable the pipe to fix this, the update_dpms > > * call below we restore the pipe to the right state, but leave > > * the required bits on. */ > > - intel_enable_pipe_a(dev); > > + intel_enable_pipe_a(dev, ctx); > > } > > > > /* Adjust the state of the output pipe according to whether we > > @@ -15568,7 +15570,8 @@ get_encoder_power_domains(struct drm_i915_private > > *dev_priv) > > * and sanitizes it to the current state > > */ > > static void > > -intel_modeset_setup_hw_state(struct drm_device *dev) > > +intel_modeset_setup_hw_state(struct drm_device *dev, > > +struct drm_modeset_acquire_ctx *ctx) > > { > > struct drm_i915_private *dev_priv = to_i915(dev); > > enum pipe pipe; > > @@ -15588,7 +15591,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev) > > for_each_pipe(dev_priv, pipe) { > > crtc = intel_get_crtc_for_pipe(dev_priv, pipe); > > > > - intel_sanitize_crtc(crtc); > > + intel_sanitize_crtc(crtc, ctx); > > intel_dump_pipe_config(crtc, crtc->config, > >"[setup_hw_state]"); > > } > > -- > Jani Nikula, Intel Open Source Technology Center -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Plumb the correct acquire ctx into intel_crtc_disable_noatomic()
On Thu, Jun 01, 2017 at 05:48:16PM +0300, Jani Nikula wrote: > On Thu, 01 Jun 2017, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > If intel_crtc_disable_noatomic() were to ever get called during resume > > e'd end up deadlocking since resume has its own acqcuire_ctx but > > intel_crtc_disable_noatomic() still tries to use the > > mode_config.acquire_ctx. Pass down the correct acquire ctx from the top. > > Same here, have we seen this, or "just" theoretical? Happens on my 830 after adding the pipes powerwell. Before that the machine resumed with both pipes off and the pipe A quirk left crtc->active as false so intel_crtc_disable_noatomic() didn't get called. Hmm. Actually, now that I think about it more, I believe this should happen for S4 even without the new powerwell since both pipes should have been enabled by the BIOS when resuming from S4. I'll have to run a proper to test to verify that. > > BR, > Jani. > > > > > Cc: sta...@vger.kernel.org > > Cc: Maarten Lankhorst > > Fixes: e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.") > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/intel_display.c | 9 + > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 2f76a63efe8c..4e3c64ed4131 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -5839,7 +5839,8 @@ static void i9xx_crtc_disable(struct intel_crtc_state > > *old_crtc_state, > > intel_update_watermarks(intel_crtc); > > } > > > > -static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) > > +static void intel_crtc_disable_noatomic(struct drm_crtc *crtc, > > + struct drm_modeset_acquire_ctx *ctx) > > { > > struct intel_encoder *encoder; > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > @@ -5869,7 +5870,7 @@ static void intel_crtc_disable_noatomic(struct > > drm_crtc *crtc) > > return; > > } > > > > - state->acquire_ctx = crtc->dev->mode_config.acquire_ctx; > > + state->acquire_ctx = ctx; > > > > /* Everything's already locked, -EDEADLK can't happen. */ > > crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); > > @@ -15257,7 +15258,7 @@ static void intel_sanitize_crtc(struct intel_crtc > > *crtc, > > plane = crtc->plane; > > crtc->base.primary->state->visible = true; > > crtc->plane = !plane; > > - intel_crtc_disable_noatomic(&crtc->base); > > + intel_crtc_disable_noatomic(&crtc->base, ctx); > > crtc->plane = plane; > > } > > > > @@ -15273,7 +15274,7 @@ static void intel_sanitize_crtc(struct intel_crtc > > *crtc, > > /* Adjust the state of the output pipe according to whether we > > * have active connectors/encoders. */ > > if (crtc->active && !intel_crtc_has_encoders(crtc)) > > - intel_crtc_disable_noatomic(&crtc->base); > > + intel_crtc_disable_noatomic(&crtc->base, ctx); > > > > if (crtc->active || HAS_GMCH_DISPLAY(dev_priv)) { > > /* > > -- > Jani Nikula, Intel Open Source Technology Center -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests/kms_setmode: increase MAX_CRTCS to 6
Please ignore what I wrote. Looks like I was a bit hasty in my judgement. The code just doesn't read easily but seems correct, other than the array size. Harry On 2017-05-31 04:17 PM, Harry Wentland wrote: On 2017-05-31 09:32 AM, Harry Wentland wrote: On 2017-05-31 05:37 AM, Daniel Vetter wrote: On Tue, May 30, 2017 at 04:01:40PM -0400, Harry Wentland wrote: AMD GPUs can have 6 CRTCs. This requires us to allocate the combinations on the heap. Signed-off-by: Harry Wentland I think just dynamically allocating stuff directly and dropping the #define would be even neater ... GetResources can tell us how much of each exists. -Daniel Agreed. I'll send out a v3 later. About to send v3 but this code is quite majorly wrong as-is. In particular I see these two issues: 1) In iterate_combinations we assign a stack object to our list of combinations which is then popped off the stack as soon as get_combinations returns. Later on in test_combinations we then try to access it with connector_idxs = &connector_combs.items[i].elems[0]; ... int *crtc_idxs = &crtc_combs.items[j].elems[0]; 2) This for loop in iterate_combinations will simply override comb->elems[depth] with new values: for (v = base; v < n; v++) { comb->elems[depth] = v; [...] } It looks like whoever wrote the code tried to do some k choose n algorithm to find all possible sets of crtcs and connectors but then only ended up picking one crtc and connector out of each list anyways (after the item popped from the stack). If I find time I might rewrite the test_combinations function with a simple nested for loop that tries all crtcs with all connectors one-to-one as this seems to be currently the intention of this function. Harry Harry --- tests/kms_setmode.c | 25 +++-- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c index 430568a1c24e..847ad650d27f 100644 --- a/tests/kms_setmode.c +++ b/tests/kms_setmode.c @@ -35,11 +35,13 @@ #include "intel_bufmgr.h" #define MAX_CONNECTORS 10 -#define MAX_CRTCS 3 +#define MAX_CRTCS 6 /* max combinations with repetitions */ +/* MAX_CONNECTORS ^ MAX_CRTCS */ +/* TODO should really be MAX_CONNECTORS ^ MAX_CONNECTORS ??? */ #define MAX_COMBINATION_COUNT \ -(MAX_CONNECTORS * MAX_CONNECTORS * MAX_CONNECTORS) +(MAX_CONNECTORS * MAX_CONNECTORS * MAX_CONNECTORS * MAX_CONNECTORS * MAX_CONNECTORS * MAX_CONNECTORS) #define MAX_COMBINATION_ELEMS MAX_CRTCS static int drm_fd; @@ -743,22 +745,25 @@ static void get_combinations(int n, int k, bool allow_repetitions, static void test_combinations(const struct test_config *tconf, int connector_count) { -struct combination_set connector_combs; -struct combination_set crtc_combs; +struct combination_set *connector_combs; +struct combination_set *crtc_combs; struct connector_config *cconfs; int i; if (connector_count > 2 && (tconf->flags & TEST_STEALING)) return; +connector_combs = malloc(sizeof(*connector_combs)); +crtc_combs = malloc(sizeof(*crtc_combs)); + get_combinations(tconf->resources->count_connectors, connector_count, - false, &connector_combs); + false, connector_combs); get_combinations(tconf->resources->count_crtcs, connector_count, - true, &crtc_combs); + true, crtc_combs); igt_info("Testing: %s %d connector combinations\n", tconf->name, connector_count); -for (i = 0; i < connector_combs.count; i++) { +for (i = 0; i < connector_combs->count; i++) { int *connector_idxs; int ret; int j; @@ -766,14 +771,14 @@ static void test_combinations(const struct test_config *tconf, cconfs = malloc(sizeof(*cconfs) * connector_count); igt_assert(cconfs); -connector_idxs = &connector_combs.items[i].elems[0]; +connector_idxs = &connector_combs->items[i].elems[0]; ret = get_connectors(tconf->resources, connector_idxs, connector_count, cconfs); if (ret < 0) goto free_cconfs; -for (j = 0; j < crtc_combs.count; j++) { -int *crtc_idxs = &crtc_combs.items[j].elems[0]; +for (j = 0; j < crtc_combs->count; j++) { +int *crtc_idxs = &crtc_combs->items[j].elems[0]; ret = assign_crtc_to_connectors(tconf, crtc_idxs, connector_count, cconfs); -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ amd-gfx mailing list amd-...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx __
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Use a loop for the "three times for luck" DPLL procedure
On Thu, Jun 01, 2017 at 03:48:51PM +0100, Chris Wilson wrote: > On Thu, Jun 01, 2017 at 05:36:15PM +0300, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > The magic "enable the DPLL three times" sequence feels like it > > deserves a loop. > > Hmm, I thought once upon a time we figured out what the magic was about. > Or was that another loop? At least I've never tried to understand this particular loop. The VGA_MODE_DISABLE thing I did manage to figure out. Whether that might have been a contributing factor for adding this loop is unclear. We actually have a couple more DPLL writes just before the loop, so we might want consider folding those into the loop as well. > > Reviewed-by: Chris Wilson > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915: Add i830 "pipes power well"
On Thu, Jun 01, 2017 at 03:46:43PM +0100, Chris Wilson wrote: > On Thu, Jun 01, 2017 at 05:36:16PM +0300, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > 830 more or less requires both pipes and DPLLs to remain on as long > > as either pipe is needed. However, when neither pipe is actually needed, > > we can save a bit of power by turning everything off. To do that we add > > a new "power well" that turns both pipes and DPLLs on and off in the > > right order. Seems to save ~50mW on my Fujitsu-Siemens Lifebook S6010. > > > > This also avoids having to abuse the load detection to force pipe A on > > at init time. That was never very robust, and it only worked for one > > pipe, whereas 830 really needs both pipes enabled. As a bonus the 830 > > pipe quirk is now a bit more isolated from the rest of the mode setting > > infrastructure, which should mean that it's much less likely someone > > will accidentally break it in the future. The extra cost is of course > > slight code duplication, but that seems like a worthwile tradeoff here. > > Defining it as a powerwell is an interesting way to do it, and seems > quite apt. My only nit is a request not to add to intel_display.c if we > can place it elsewhere, intel_gen2_pm.c ? gen2_runtime_pm.c? Not sure a new file for just these two functions is warranted. I was actually thinking of keeping them reasonably close by to the normal pipe/dpll enable/disable code so that if people make a change to one they would realize that they should perhaps change the other one as well. Although I suppose intel_display.c is so big that even having them in the same file won't really guarantee that. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/13] drm/i915/cnp: Backlight support for CNP.
On Thu, 2017-06-01 at 02:15 +, Pandiyan, Dhinakaran wrote: > On Tue, 2017-05-30 at 15:42 -0700, Rodrigo Vivi wrote: > > Split out BXT and CNP's setup_backlight(),enable_backlight(), > > disable_backlight() and hz_to_pwm() into > > two separate functions instead of reusing BXT function. > > > > Reuse set_backlight() and get_backlight() since they have > > no reference to the utility pin. > > > > v2: Reuse BXT functions with controller 0 instead of > > redefining it. (Jani). > > Use dev_priv->rawclk_freq instead of getting the value > > from SFUSE_STRAP. > > v3: Avoid setup backligh controller along with hooks and > > fully reuse hooks setup as suggested by Jani. > > v4: Clean up commit message. > > v5: Implement per PCH instead per platform. > > > > v6: Introduce a new function for CNP.(Jani and Ville) > > > > v7: Squash the all CNP Backlight support patches into a > > single patch. (Jani) > > > > v8: Correct indentation, remove unneeded blank lines and > > correct mail address (Jani). > > > > v9: Remove unused enum pipe. (by CI) > > > > Reviewed-by: Jani Nikula > > Suggested-by: Jani Nikula > > Suggested-by: Ville Syrjala > > Cc: Ville Syrjala > > Cc: Jani Nikula > > Signed-off-by: Anusha Srivatsa > > Signed-off-by: Rodrigo Vivi > > --- > > drivers/gpu/drm/i915/intel_panel.c | 93 > > ++ > > 1 file changed, 93 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_panel.c > > b/drivers/gpu/drm/i915/intel_panel.c > > index c8103f8..7e34a1b 100644 > > --- a/drivers/gpu/drm/i915/intel_panel.c > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > @@ -796,6 +796,19 @@ static void bxt_disable_backlight(struct > > intel_connector *connector) > > } > > } > > > > +static void cnp_disable_backlight(struct intel_connector *connector) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > > + struct intel_panel *panel = &connector->panel; > > + u32 tmp; > > + > > + intel_panel_actually_set_backlight(connector, 0); > > + > > + tmp = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller)); > > + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller), > > + tmp & ~BXT_BLC_PWM_ENABLE); > > +} > > + > > static void pwm_disable_backlight(struct intel_connector *connector) > > { > > struct intel_panel *panel = &connector->panel; > > @@ -1086,6 +1099,35 @@ static void bxt_enable_backlight(struct > > intel_connector *connector) > > pwm_ctl | BXT_BLC_PWM_ENABLE); > > } > > > > +static void cnp_enable_backlight(struct intel_connector *connector) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > > + struct intel_panel *panel = &connector->panel; > > + u32 pwm_ctl; > > + > > + pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller)); > > Shouldn't this be BLC_PWM_PCH_CTL1? Not sure. Are we going to 1) redefine everything? including all registers and bit definitions that are identical? 2) reuse part of CPU and part of old PCH? > > I think reusing CPU register definitions for PCH is confusing. Even more > so, when we already have separate definitions for PCH. Actually the BXT backlight implementation was used on CNP. So all of this was moved from CPU to PCH. > BSpec specifically refers to these registers as SBLC_PWM_CTL1, > SBLC_PWM_FREQ and SBLC_PWM_DUTY. I believe we traditionally try to reuse registers definitions that are there already instead of redefine everytime that spec changes the name. For me all of this implementation is more like BXT so we should proceed with this from this point and on and reusing as much as we can. However during the review it was decided to not reuse directly the bxt functions... So now I'm not sure in which point we should stop duplicating code anymore... > > > > > > + if (pwm_ctl & BXT_BLC_PWM_ENABLE) { > > + DRM_DEBUG_KMS("backlight already enabled\n"); > > + pwm_ctl &= ~BXT_BLC_PWM_ENABLE; > > + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller), > > + pwm_ctl); > > + } > > + > > + I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller), > > + panel->backlight.max); > > + > > + intel_panel_actually_set_backlight(connector, panel->backlight.level); > > + > > + pwm_ctl = 0; > > + if (panel->backlight.active_low_pwm) > > + pwm_ctl |= BXT_BLC_PWM_POLARITY; > > + > > + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller), pwm_ctl); > > + POSTING_READ(BXT_BLC_PWM_CTL(panel->backlight.controller)); > > + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller), > > + pwm_ctl | BXT_BLC_PWM_ENABLE); > > +} > > + > > static void pwm_enable_backlight(struct intel_connector *connector) > > { > > struct intel_panel *panel = &connector->panel; > > @@ -1250,6 +1292,18 @@ void intel_backlight_device_unregister(struct > > intel_connector *connector) > > #endif /* CONFIG
Re: [Intel-gfx] [PATCH v6 4/6] vfio: Define vfio based vgpu's dma-buf operations
On Thu, 1 Jun 2017 03:01:28 + "Chen, Xiaoguang" wrote: > Hi Kirti, > > >-Original Message- > >From: Kirti Wankhede [mailto:kwankh...@nvidia.com] > >Sent: Thursday, June 01, 2017 1:23 AM > >To: Chen, Xiaoguang ; Gerd Hoffmann > >; alex.william...@redhat.com; ch...@chris-wilson.co.uk; > >intel-gfx@lists.freedesktop.org; linux-ker...@vger.kernel.org; > >zhen...@linux.intel.com; Lv, Zhiyuan ; intel-gvt- > >d...@lists.freedesktop.org; Wang, Zhi A ; Tian, Kevin > > > >Subject: Re: [PATCH v6 4/6] vfio: Define vfio based vgpu's dma-buf operations > > > > > > > >On 5/31/2017 11:48 AM, Chen, Xiaoguang wrote: > >> Hi, > >> > >>> -Original Message- > >>> From: Gerd Hoffmann [mailto:kra...@redhat.com] > >>> Sent: Monday, May 29, 2017 3:20 PM > >>> To: Chen, Xiaoguang ; > >>> alex.william...@redhat.com; ch...@chris-wilson.co.uk; intel- > >>> g...@lists.freedesktop.org; linux-ker...@vger.kernel.org; > >>> zhen...@linux.intel.com; Lv, Zhiyuan ; > >>> intel-gvt- d...@lists.freedesktop.org; Wang, Zhi A > >>> ; Tian, Kevin > >>> Subject: Re: [PATCH v6 4/6] vfio: Define vfio based vgpu's dma-buf > >>> operations > >>> > +struct vfio_vgpu_dmabuf_info { > +__u32 argsz; > +__u32 flags; > +struct vfio_vgpu_plane_info plane_info; > +__s32 fd; > +__u32 pad; > +}; > >>> > >>> Hmm, now you have argsz and flags twice in vfio_vgpu_dmabuf_info ... > >>> > >>> I think we should have something like this: > >>> > >>> struct vfio_vgpu_plane_info { > >>> __u64 start; > >>> __u64 drm_format_mod; > >>> __u32 drm_format; > >>> __u32 width; > >>> __u32 height; > >>> __u32 stride; > >>> __u32 size; > >>> __u32 x_pos; > >>> __u32 y_pos; > >>>__u32 padding; > >>> }; > >>> > >>> struct vfio_vgpu_query_plane { > >>> __u32 argsz; > >>> __u32 flags; > >>> struct vfio_vgpu_plane_info plane_info; > >>>__u32 plane_id; > >>>__u32 padding; > >>> }; > >>> > >>> struct vfio_vgpu_create_dmabuf { > >>> __u32 argsz; > >>> __u32 flags; > >>> struct vfio_vgpu_plane_info plane_info; > >>>__u32 plane_id; > >>>__s32 fd; > >>> }; > >> Good suggestion will apply in the next version. > >> Thanks for review :) > >> > > > >Can you define what are the expected values of 'flags' would be? > Flags is not used in this case. It is defined to follow the rules of vfio > ioctls. An important note about flags, the vendor driver must validate it. If they don't and the user passes an arbitrary value there, then we have a backwards compatibility issue with ever attempting to use the flags field. The user passing in a flag unknown to the vendor driver should return an -EINVAL response. In this case, we haven't defined any flags, so the vendor driver needs to force the user to pass zero. Thanks, Alex ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Store i915_gem_object_is_coherent() as a bit next to cache-dirty
For ease of use (i.e. avoiding a few checks and function calls), store the object's cache coherency next to the cache is dirty bit. Signed-off-by: Chris Wilson Cc: Dongwon Kim Cc: Matt Roper Tested-by: Dongwon Kim --- drivers/gpu/drm/i915/i915_gem.c | 14 +++--- drivers/gpu/drm/i915/i915_gem_clflush.c | 2 +- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- drivers/gpu/drm/i915/i915_gem_internal.c | 3 ++- drivers/gpu/drm/i915/i915_gem_object.h | 1 + drivers/gpu/drm/i915/i915_gem_stolen.c | 1 + drivers/gpu/drm/i915/i915_gem_userptr.c | 3 ++- drivers/gpu/drm/i915/selftests/huge_gem_object.c | 3 ++- 8 files changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 9fa571523641..bd1be8f8ce9d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -52,7 +52,7 @@ static bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj) if (obj->cache_dirty) return false; - if (!i915_gem_object_is_coherent(obj)) + if (!obj->cache_coherent) return true; return obj->pin_display; @@ -253,7 +253,7 @@ __i915_gem_object_release_shmem(struct drm_i915_gem_object *obj, if (needs_clflush && (obj->base.read_domains & I915_GEM_DOMAIN_CPU) == 0 && - !i915_gem_object_is_coherent(obj)) + !obj->cache_coherent) drm_clflush_sg(pages); __start_cpu_write(obj); @@ -856,8 +856,7 @@ int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj, if (ret) return ret; - if (i915_gem_object_is_coherent(obj) || - !static_cpu_has(X86_FEATURE_CLFLUSH)) { + if (obj->cache_coherent || !static_cpu_has(X86_FEATURE_CLFLUSH)) { ret = i915_gem_object_set_to_cpu_domain(obj, false); if (ret) goto err_unpin; @@ -909,8 +908,7 @@ int i915_gem_obj_prepare_shmem_write(struct drm_i915_gem_object *obj, if (ret) return ret; - if (i915_gem_object_is_coherent(obj) || - !static_cpu_has(X86_FEATURE_CLFLUSH)) { + if (obj->cache_coherent || !static_cpu_has(X86_FEATURE_CLFLUSH)) { ret = i915_gem_object_set_to_cpu_domain(obj, true); if (ret) goto err_unpin; @@ -3688,6 +3686,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj, list_for_each_entry(vma, &obj->vma_list, obj_link) vma->node.color = cache_level; obj->cache_level = cache_level; + obj->cache_coherent = i915_gem_object_is_coherent(obj); obj->cache_dirty = true; /* Always invalidate stale cachelines */ return 0; @@ -4348,7 +4347,8 @@ i915_gem_object_create(struct drm_i915_private *dev_priv, u64 size) } else obj->cache_level = I915_CACHE_NONE; - obj->cache_dirty = !i915_gem_object_is_coherent(obj); + obj->cache_coherent = i915_gem_object_is_coherent(obj); + obj->cache_dirty = !obj->cache_coherent; trace_i915_gem_object_create(obj); diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c b/drivers/gpu/drm/i915/i915_gem_clflush.c index 17b207e963c2..152f16c11878 100644 --- a/drivers/gpu/drm/i915/i915_gem_clflush.c +++ b/drivers/gpu/drm/i915/i915_gem_clflush.c @@ -139,7 +139,7 @@ void i915_gem_clflush_object(struct drm_i915_gem_object *obj, * snooping behaviour occurs naturally as the result of our domain * tracking. */ - if (!(flags & I915_CLFLUSH_FORCE) && i915_gem_object_is_coherent(obj)) + if (!(flags & I915_CLFLUSH_FORCE) && obj->cache_coherent) return; trace_i915_gem_object_clflush(obj); diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 87505cfb75c2..3d34badc4838 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1129,7 +1129,7 @@ i915_gem_execbuffer_move_to_gpu(struct drm_i915_gem_request *req, if (vma->exec_entry->flags & EXEC_OBJECT_ASYNC) continue; - if (obj->cache_dirty) + if (obj->cache_dirty & ~obj->cache_coherent) i915_gem_clflush_object(obj, 0); ret = i915_gem_request_await_object diff --git a/drivers/gpu/drm/i915/i915_gem_internal.c b/drivers/gpu/drm/i915/i915_gem_internal.c index 58e93e87d573..568bf83af1f5 100644 --- a/drivers/gpu/drm/i915/i915_gem_internal.c +++ b/drivers/gpu/drm/i915/i915_gem_internal.c @@ -191,7 +191,8 @@ i915_gem_object_create_internal(struct drm_i915_private *i915, obj->base.read_domains = I915_GEM_DOMAIN_CPU; obj->base.write_domain = I915_GEM_DOMAIN_CPU; obj->cache_level = HAS_LLC(i915) ? I915_CAC
[Intel-gfx] [PATCH 1/2] drm/i915: Mark CPU cache as dirty on every transition for CPU writes
Currently, we only mark the CPU cache as dirty if we skip a clflush. This leads to some confusion where we have to ask if the object is in the write domain or missed a clflush. If we always mark the cache as dirty, this becomes a much simply question to answer. The goal remains to do as few clflushes as required and to do them as late as possible, in the hope of deferring the work to a kthread and not block the caller (e.g. execbuf, flips). v2: Always call clflush before GPU execution when the cache_dirty flag is set. This may cause some extra work on llc systems that migrate dirty buffers back and forth - but we do try to limit that by only setting cache_dirty at the end of the gpu sequence. v3: Always mark the cache as dirty upon a level change, as we need to invalidate any stale cachelines due to external writes. Reported-by: Dongwon Kim Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning the scanout") Signed-off-by: Chris Wilson Cc: Dongwon Kim Cc: Matt Roper Tested-by: Dongwon Kim --- drivers/gpu/drm/i915/i915_gem.c | 76 ++-- drivers/gpu/drm/i915/i915_gem_clflush.c | 15 +++-- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 21 +++ drivers/gpu/drm/i915/i915_gem_internal.c | 3 +- drivers/gpu/drm/i915/i915_gem_userptr.c | 5 +- drivers/gpu/drm/i915/selftests/huge_gem_object.c | 3 +- 6 files changed, 67 insertions(+), 56 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 53c51787d2ed..9fa571523641 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -49,7 +49,7 @@ static void i915_gem_flush_free_objects(struct drm_i915_private *i915); static bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj) { - if (obj->base.write_domain == I915_GEM_DOMAIN_CPU) + if (obj->cache_dirty) return false; if (!i915_gem_object_is_coherent(obj)) @@ -233,6 +233,14 @@ i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj) return st; } +static void __start_cpu_write(struct drm_i915_gem_object *obj) +{ + obj->base.read_domains = I915_GEM_DOMAIN_CPU; + obj->base.write_domain = I915_GEM_DOMAIN_CPU; + if (cpu_write_needs_clflush(obj)) + obj->cache_dirty = true; +} + static void __i915_gem_object_release_shmem(struct drm_i915_gem_object *obj, struct sg_table *pages, @@ -248,8 +256,7 @@ __i915_gem_object_release_shmem(struct drm_i915_gem_object *obj, !i915_gem_object_is_coherent(obj)) drm_clflush_sg(pages); - obj->base.read_domains = I915_GEM_DOMAIN_CPU; - obj->base.write_domain = I915_GEM_DOMAIN_CPU; + __start_cpu_write(obj); } static void @@ -684,6 +691,12 @@ i915_gem_dumb_create(struct drm_file *file, args->size, &args->handle); } +static bool gpu_write_needs_clflush(struct drm_i915_gem_object *obj) +{ + return !(obj->cache_level == I915_CACHE_NONE || +obj->cache_level == I915_CACHE_WT); +} + /** * Creates a new mm object and returns a handle to it. * @dev: drm device pointer @@ -753,6 +766,11 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains) case I915_GEM_DOMAIN_CPU: i915_gem_clflush_object(obj, I915_CLFLUSH_SYNC); break; + + case I915_GEM_DOMAIN_RENDER: + if (gpu_write_needs_clflush(obj)) + obj->cache_dirty = true; + break; } obj->base.write_domain = 0; @@ -854,7 +872,8 @@ int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj, * optimizes for the case when the gpu will dirty the data * anyway again before the next pread happens. */ - if (!(obj->base.read_domains & I915_GEM_DOMAIN_CPU)) + if (!obj->cache_dirty && + !(obj->base.read_domains & I915_GEM_DOMAIN_CPU)) *needs_clflush = CLFLUSH_BEFORE; out: @@ -906,14 +925,16 @@ int i915_gem_obj_prepare_shmem_write(struct drm_i915_gem_object *obj, * This optimizes for the case when the gpu will use the data * right away and we therefore have to clflush anyway. */ - if (obj->base.write_domain != I915_GEM_DOMAIN_CPU) + if (!obj->cache_dirty) { *needs_clflush |= CLFLUSH_AFTER; - /* Same trick applies to invalidate partially written cachelines read -* before writing. -*/ - if (!(obj->base.read_domains & I915_GEM_DOMAIN_CPU)) - *needs_clflush |= CLFLUSH_BEFORE; + /* +* Same trick applies to invalidate partially written +* cachelines read before writing. +*/ + if (!(obj->base.read_domains & I915_GEM_DOMAIN_CPU)) + *needs_clflush |= CLF
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Plumb the correct acquire ctx into intel_crtc_disable_noatomic()
On Thu, Jun 01, 2017 at 07:05:41PM +0300, Ville Syrjälä wrote: > On Thu, Jun 01, 2017 at 05:48:16PM +0300, Jani Nikula wrote: > > On Thu, 01 Jun 2017, ville.syrj...@linux.intel.com wrote: > > > From: Ville Syrjälä > > > > > > If intel_crtc_disable_noatomic() were to ever get called during resume > > > e'd end up deadlocking since resume has its own acqcuire_ctx but > > > intel_crtc_disable_noatomic() still tries to use the > > > mode_config.acquire_ctx. Pass down the correct acquire ctx from the top. > > > > Same here, have we seen this, or "just" theoretical? > > Happens on my 830 after adding the pipes powerwell. Before that the > machine resumed with both pipes off and the pipe A quirk left > crtc->active as false so intel_crtc_disable_noatomic() didn't get > called. > > Hmm. Actually, now that I think about it more, I believe this should > happen for S4 even without the new powerwell since both pipes should > have been enabled by the BIOS when resuming from S4. I'll have to > run a proper to test to verify that. Indeed. In fact it deadlocks already during the thaw phase between creating the hibernation image and writing to it disk. > > > > > BR, > > Jani. > > > > > > > > Cc: sta...@vger.kernel.org > > > Cc: Maarten Lankhorst > > > Fixes: e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.") > > > Signed-off-by: Ville Syrjälä > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 9 + > > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > b/drivers/gpu/drm/i915/intel_display.c > > > index 2f76a63efe8c..4e3c64ed4131 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -5839,7 +5839,8 @@ static void i9xx_crtc_disable(struct > > > intel_crtc_state *old_crtc_state, > > > intel_update_watermarks(intel_crtc); > > > } > > > > > > -static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) > > > +static void intel_crtc_disable_noatomic(struct drm_crtc *crtc, > > > + struct drm_modeset_acquire_ctx *ctx) > > > { > > > struct intel_encoder *encoder; > > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > > @@ -5869,7 +5870,7 @@ static void intel_crtc_disable_noatomic(struct > > > drm_crtc *crtc) > > > return; > > > } > > > > > > - state->acquire_ctx = crtc->dev->mode_config.acquire_ctx; > > > + state->acquire_ctx = ctx; > > > > > > /* Everything's already locked, -EDEADLK can't happen. */ > > > crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); > > > @@ -15257,7 +15258,7 @@ static void intel_sanitize_crtc(struct intel_crtc > > > *crtc, > > > plane = crtc->plane; > > > crtc->base.primary->state->visible = true; > > > crtc->plane = !plane; > > > - intel_crtc_disable_noatomic(&crtc->base); > > > + intel_crtc_disable_noatomic(&crtc->base, ctx); > > > crtc->plane = plane; > > > } > > > > > > @@ -15273,7 +15274,7 @@ static void intel_sanitize_crtc(struct intel_crtc > > > *crtc, > > > /* Adjust the state of the output pipe according to whether we > > >* have active connectors/encoders. */ > > > if (crtc->active && !intel_crtc_has_encoders(crtc)) > > > - intel_crtc_disable_noatomic(&crtc->base); > > > + intel_crtc_disable_noatomic(&crtc->base, ctx); > > > > > > if (crtc->active || HAS_GMCH_DISPLAY(dev_priv)) { > > > /* > > > > -- > > Jani Nikula, Intel Open Source Technology Center > > -- > Ville Syrjälä > Intel OTC > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Mark CPU cache as dirty on every transition for CPU writes
== Series Details == Series: series starting with [1/2] drm/i915: Mark CPU cache as dirty on every transition for CPU writes URL : https://patchwork.freedesktop.org/series/25173/ State : success == Summary == Series 25173v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/25173/revisions/1/mbox/ Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-legacy: pass -> FAIL (fi-snb-2600) fdo#100215 fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:444s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:432s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:573s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:515s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:482s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:489s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:426s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:406s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:424s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:490s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:464s fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 time:465s fi-kbl-7560u total:278 pass:263 dwarn:5 dfail:0 fail:0 skip:10 time:568s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:460s fi-skl-6700hqtotal:278 pass:239 dwarn:0 dfail:1 fail:17 skip:21 time:427s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:468s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:498s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:436s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:539s fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 time:406s 2c9abf8ec88cf4b3d3735976bb0ff7a4991946b2 drm-tip: 2017y-06m-01d-13h-29m-05s UTC integration manifest 77ab5e1 drm/i915: Store i915_gem_object_is_coherent() as a bit next to cache-dirty 9823c27 drm/i915: Mark CPU cache as dirty on every transition for CPU writes == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4859/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: Fix dotclock calculation in skl_check_pipe_max_pixel_rate
Op 01-06-17 om 15:52 schreef Ville Syrjälä: > On Thu, Jun 01, 2017 at 12:34:13PM +0200, Maarten Lankhorst wrote: >> Seems that GLK has a dotclock that's twice the display clock. >> skl_max_scale checks for IS_GEMINILAKE, so perform the same check here. >> >> While at it, change the DRM_ERROR to DEBUG_KMS. >> >> Fixes: 73b0ca8ec76d ("drm/i915/skl+: consider max supported plane pixel >> rate while scaling") >> Cc: Mahesh Kumar >> Signed-off-by: Maarten Lankhorst >> --- >> drivers/gpu/drm/i915/intel_pm.c | 12 >> 1 file changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c >> b/drivers/gpu/drm/i915/intel_pm.c >> index 2042f6512e6e..88c8a3511e24 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -4122,7 +4122,7 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc >> *intel_crtc, >> struct drm_plane *plane; >> const struct drm_plane_state *pstate; >> struct intel_plane_state *intel_pstate; >> -int crtc_clock, cdclk; >> +int crtc_clock, dotclk; >> uint32_t pipe_max_pixel_rate; >> uint_fixed_16_16_t pipe_downscale; >> uint_fixed_16_16_t max_downscale = u32_to_fixed_16_16(1); >> @@ -4157,11 +4157,15 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc >> *intel_crtc, >> pipe_downscale = mul_fixed16(pipe_downscale, max_downscale); >> >> crtc_clock = crtc_state->adjusted_mode.crtc_clock; >> -cdclk = to_intel_atomic_state(state)->cdclk.logical.cdclk; >> -pipe_max_pixel_rate = div_round_up_u32_fixed16(cdclk, pipe_downscale); >> +dotclk = to_intel_atomic_state(state)->cdclk.logical.cdclk; > dotclk = cdclk. That statement doesn't make sense. It should be called > max_dotclk or something like that. R-B if you want to rename it to max_dotclk. :) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/13] drm/i915/cnp: Backlight support for CNP.
Based on your clarification the second option feels like the right choice, with a relevant comment in code. Like you said, we get to retain BXT register definitions and clarify that the register is on a PCH for CNP. -DK -Original Message- From: Vivi, Rodrigo Sent: Thursday, June 1, 2017 9:29 AM To: Pandiyan, Dhinakaran Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani Subject: Re: [Intel-gfx] [PATCH 04/13] drm/i915/cnp: Backlight support for CNP. On Thu, 2017-06-01 at 02:15 +, Pandiyan, Dhinakaran wrote: > On Tue, 2017-05-30 at 15:42 -0700, Rodrigo Vivi wrote: > > Split out BXT and CNP's setup_backlight(),enable_backlight(), > > disable_backlight() and hz_to_pwm() into two separate functions > > instead of reusing BXT function. > > > > Reuse set_backlight() and get_backlight() since they have no > > reference to the utility pin. > > > > v2: Reuse BXT functions with controller 0 instead of > > redefining it. (Jani). > > Use dev_priv->rawclk_freq instead of getting the value > > from SFUSE_STRAP. > > v3: Avoid setup backligh controller along with hooks and > > fully reuse hooks setup as suggested by Jani. > > v4: Clean up commit message. > > v5: Implement per PCH instead per platform. > > > > v6: Introduce a new function for CNP.(Jani and Ville) > > > > v7: Squash the all CNP Backlight support patches into a single > > patch. (Jani) > > > > v8: Correct indentation, remove unneeded blank lines and correct > > mail address (Jani). > > > > v9: Remove unused enum pipe. (by CI) > > > > Reviewed-by: Jani Nikula > > Suggested-by: Jani Nikula > > Suggested-by: Ville Syrjala > > Cc: Ville Syrjala > > Cc: Jani Nikula > > Signed-off-by: Anusha Srivatsa > > Signed-off-by: Rodrigo Vivi > > --- > > drivers/gpu/drm/i915/intel_panel.c | 93 > > ++ > > 1 file changed, 93 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_panel.c > > b/drivers/gpu/drm/i915/intel_panel.c > > index c8103f8..7e34a1b 100644 > > --- a/drivers/gpu/drm/i915/intel_panel.c > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > @@ -796,6 +796,19 @@ static void bxt_disable_backlight(struct > > intel_connector *connector) > > } > > } > > > > +static void cnp_disable_backlight(struct intel_connector > > +*connector) { > > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > > + struct intel_panel *panel = &connector->panel; > > + u32 tmp; > > + > > + intel_panel_actually_set_backlight(connector, 0); > > + > > + tmp = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller)); > > + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller), > > + tmp & ~BXT_BLC_PWM_ENABLE); > > +} > > + > > static void pwm_disable_backlight(struct intel_connector > > *connector) { > > struct intel_panel *panel = &connector->panel; @@ -1086,6 +1099,35 > > @@ static void bxt_enable_backlight(struct intel_connector *connector) > > pwm_ctl | BXT_BLC_PWM_ENABLE); > > } > > > > +static void cnp_enable_backlight(struct intel_connector *connector) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > > + struct intel_panel *panel = &connector->panel; > > + u32 pwm_ctl; > > + > > + pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller)); > > Shouldn't this be BLC_PWM_PCH_CTL1? Not sure. Are we going to 1) redefine everything? including all registers and bit definitions that are identical? 2) reuse part of CPU and part of old PCH? > > I think reusing CPU register definitions for PCH is confusing. Even > more so, when we already have separate definitions for PCH. Actually the BXT backlight implementation was used on CNP. So all of this was moved from CPU to PCH. > BSpec specifically refers to these registers as SBLC_PWM_CTL1, > SBLC_PWM_FREQ and SBLC_PWM_DUTY. I believe we traditionally try to reuse registers definitions that are there already instead of redefine everytime that spec changes the name. For me all of this implementation is more like BXT so we should proceed with this from this point and on and reusing as much as we can. However during the review it was decided to not reuse directly the bxt functions... So now I'm not sure in which point we should stop duplicating code anymore... > > > > > > + if (pwm_ctl & BXT_BLC_PWM_ENABLE) { > > + DRM_DEBUG_KMS("backlight already enabled\n"); > > + pwm_ctl &= ~BXT_BLC_PWM_ENABLE; > > + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller), > > + pwm_ctl); > > + } > > + > > + I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller), > > + panel->backlight.max); > > + > > + intel_panel_actually_set_backlight(connector, > > +panel->backlight.level); > > + > > + pwm_ctl = 0; > > + if (panel->backlight.active_low_pwm) > > + pwm_ctl |= BXT_BLC_PWM_POLARITY; > > + > > + I915_WRITE(BXT_BLC_PWM_CTL(panel->backli
[Intel-gfx] [PATCH] drm/i915: Implement fbc_status "Compressing" info for all platforms
From: Ville Syrjälä The number of compressed segments has been available ever since FBC2 was introduced in g4x, it just moved from the STATUS register into STATUS2 on IVB. For FBC1 if we really wanted the number of compressed segments we'd have to trawl through the tags, but in this case since the code just uses the number of compressed segments as an indicator whether compression has occurred we can just check the state of the COMPRESSING and COMPRESSED bits. IIRC the hardware will try to periodically recompress all uncompressed lines even if they haven't changed and the COMPRESSED bit will be cleared while the compressor is running, so just checking the COMPRESSED bit might not give us the right answer. Hence it seems better to check for both COMPRESSED and COMPRESSING as that should tell us that the compressor is at least trying to do something. While at it move the IVB+ register define to the right place, unify the naming convention of the compressed segment count masks, and fix up the mask for g4x. Cc: Paulo Zanoni Cc: Gabriel Krisman Bertazi Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_debugfs.c | 22 -- drivers/gpu/drm/i915/i915_reg.h | 10 +- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 3b088685a553..126f1e8a9d0b 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1670,12 +1670,22 @@ static int i915_fbc_status(struct seq_file *m, void *unused) seq_printf(m, "FBC disabled: %s\n", dev_priv->fbc.no_fbc_reason); - if (intel_fbc_is_active(dev_priv) && INTEL_GEN(dev_priv) >= 7) { - uint32_t mask = INTEL_GEN(dev_priv) >= 8 ? - BDW_FBC_COMPRESSION_MASK : - IVB_FBC_COMPRESSION_MASK; - seq_printf(m, "Compressing: %s\n", - yesno(I915_READ(FBC_STATUS2) & mask)); + if (intel_fbc_is_active(dev_priv)) { + u32 mask; + + if (INTEL_GEN(dev_priv) >= 8) + mask = I915_READ(ILK_DPFC_STATUS2) & BDW_DPFC_COMP_SEG_MASK; + else if (INTEL_GEN(dev_priv) >= 7) + mask = I915_READ(ILK_DPFC_STATUS2) & IVB_DPFC_COMP_SEG_MASK; + else if (INTEL_GEN(dev_priv) >= 5) + mask = I915_READ(ILK_DPFC_STATUS) & ILK_DPFC_COMP_SEG_MASK; + else if (IS_G4X(dev_priv)) + mask = I915_READ(DPFC_STATUS) & DPFC_COMP_SEG_MASK; + else + mask = I915_READ(FBC_STATUS) & (FBC_STAT_COMPRESSING | + FBC_STAT_COMPRESSED); + + seq_printf(m, "Compressing: %s\n", yesno(mask)); } mutex_unlock(&dev_priv->fbc.lock); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 231ee86625cd..23bdc079575c 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2507,10 +2507,6 @@ enum skl_disp_power_wells { #define FBC_FENCE_OFF _MMIO(0x3218) /* BSpec typo has 321Bh */ #define FBC_TAG(i) _MMIO(0x3300 + (i) * 4) -#define FBC_STATUS2_MMIO(0x43214) -#define IVB_FBC_COMPRESSION_MASK 0x7ff -#define BDW_FBC_COMPRESSION_MASK 0xfff - #define FBC_LL_SIZE(1536) #define FBC_LLC_READ_CTRL _MMIO(0x9044) @@ -2539,7 +2535,7 @@ enum skl_disp_power_wells { #define DPFC_INVAL_SEG_SHIFT (16) #define DPFC_INVAL_SEG_MASK (0x07ff) #define DPFC_COMP_SEG_SHIFT (0) -#define DPFC_COMP_SEG_MASK (0x03ff) +#define DPFC_COMP_SEG_MASK (0x07ff) #define DPFC_STATUS2 _MMIO(0x3214) #define DPFC_FENCE_YOFF_MMIO(0x3218) #define DPFC_CHICKEN _MMIO(0x3224) @@ -2553,6 +2549,10 @@ enum skl_disp_power_wells { #define DPFC_RESERVED(0x1F00) #define ILK_DPFC_RECOMP_CTL_MMIO(0x4320c) #define ILK_DPFC_STATUS_MMIO(0x43210) +#define ILK_DPFC_COMP_SEG_MASK0x7ff +#define ILK_DPFC_STATUS2 _MMIO(0x43214) +#define IVB_DPFC_COMP_SEG_MASK0x7ff +#define BDW_DPFC_COMP_SEG_MASK0xfff #define ILK_DPFC_FENCE_YOFF_MMIO(0x43218) #define ILK_DPFC_CHICKEN _MMIO(0x43224) #define ILK_DPFC_DISABLE_DUMMY0 (1<<8) -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 6/6] drm/i915/gvt: Adding interface so user space can get the dma-buf
On Sat, 27 May 2017 16:38:52 +0800 Xiaoguang Chen wrote: > User space should create the management fd for the dma-buf operation first. > Then user can query the plane information and create dma-buf if necessary > using the management fd. > > Signed-off-by: Xiaoguang Chen > --- > drivers/gpu/drm/i915/gvt/dmabuf.c | 12 > drivers/gpu/drm/i915/gvt/dmabuf.h | 5 ++ > drivers/gpu/drm/i915/gvt/gvt.c| 2 + > drivers/gpu/drm/i915/gvt/gvt.h| 5 ++ > drivers/gpu/drm/i915/gvt/kvmgt.c | 144 > ++ > drivers/gpu/drm/i915/gvt/vgpu.c | 1 + > 6 files changed, 169 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c > b/drivers/gpu/drm/i915/gvt/dmabuf.c > index c831e91..9759e9a 100644 > --- a/drivers/gpu/drm/i915/gvt/dmabuf.c > +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c > @@ -226,6 +226,7 @@ int intel_vgpu_create_dmabuf(struct intel_vgpu *vgpu, > void *args) > struct vfio_vgpu_dmabuf_info *gvt_dmabuf = args; > struct intel_vgpu_fb_info *fb_info; > int ret; > + struct intel_vgpu_dmabuf_obj *dmabuf_obj; > > ret = intel_vgpu_get_plane_info(dev, vgpu, &gvt_dmabuf->plane_info); > if (ret != 0) > @@ -263,6 +264,17 @@ int intel_vgpu_create_dmabuf(struct intel_vgpu *vgpu, > void *args) > gvt_vgpu_err("create dma-buf fd failed ret:%d\n", ret); > return ret; > } > + dmabuf_obj = kmalloc(sizeof(*dmabuf_obj), GFP_KERNEL); > + if (dmabuf_obj == NULL) { > + kfree(fb_info); > + i915_gem_object_put(obj); > + gvt_vgpu_err("alloc dmabuf_obj failed\n"); > + return -ENOMEM; > + } > + dmabuf_obj->obj = obj; > + INIT_LIST_HEAD(&dmabuf_obj->list); > + list_add_tail(&dmabuf_obj->list, &vgpu->dmabuf_obj_list_head); > + > gvt_dmabuf->fd = ret; > > return 0; > diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.h > b/drivers/gpu/drm/i915/gvt/dmabuf.h > index 8be9979..cafa781 100644 > --- a/drivers/gpu/drm/i915/gvt/dmabuf.h > +++ b/drivers/gpu/drm/i915/gvt/dmabuf.h > @@ -31,6 +31,11 @@ struct intel_vgpu_fb_info { > uint32_t fb_size; > }; > > +struct intel_vgpu_dmabuf_obj { > + struct drm_i915_gem_object *obj; > + struct list_head list; > +}; > + > int intel_vgpu_query_plane(struct intel_vgpu *vgpu, void *args); > int intel_vgpu_create_dmabuf(struct intel_vgpu *vgpu, void *args); > > diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c > index 2032917..dbc3f86 100644 > --- a/drivers/gpu/drm/i915/gvt/gvt.c > +++ b/drivers/gpu/drm/i915/gvt/gvt.c > @@ -54,6 +54,8 @@ static const struct intel_gvt_ops intel_gvt_ops = { > .vgpu_reset = intel_gvt_reset_vgpu, > .vgpu_activate = intel_gvt_activate_vgpu, > .vgpu_deactivate = intel_gvt_deactivate_vgpu, > + .vgpu_query_plane = intel_vgpu_query_plane, > + .vgpu_create_dmabuf = intel_vgpu_create_dmabuf, > }; > > /** > diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h > index 763a8c5..a855797 100644 > --- a/drivers/gpu/drm/i915/gvt/gvt.h > +++ b/drivers/gpu/drm/i915/gvt/gvt.h > @@ -185,8 +185,11 @@ struct intel_vgpu { > struct kvm *kvm; > struct work_struct release_work; > atomic_t released; > + struct vfio_device *vfio_device; > } vdev; > #endif > + int dmabuf_mgr_fd; > + struct list_head dmabuf_obj_list_head; > }; > > struct intel_gvt_gm { > @@ -467,6 +470,8 @@ struct intel_gvt_ops { > void (*vgpu_reset)(struct intel_vgpu *); > void (*vgpu_activate)(struct intel_vgpu *); > void (*vgpu_deactivate)(struct intel_vgpu *); > + int (*vgpu_query_plane)(struct intel_vgpu *vgpu, void *); > + int (*vgpu_create_dmabuf)(struct intel_vgpu *vgpu, void *); > }; > > > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c > b/drivers/gpu/drm/i915/gvt/kvmgt.c > index 389f072..a079080 100644 > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c > @@ -41,6 +41,7 @@ > #include > #include > #include > +#include > > #include "i915_drv.h" > #include "gvt.h" > @@ -524,6 +525,125 @@ static int intel_vgpu_reg_init_opregion(struct > intel_vgpu *vgpu) > return ret; > } > > +static int kvmgt_get_vfio_device(struct intel_vgpu *vgpu) > +{ > + struct vfio_device *device; > + > + device = vfio_device_get_from_dev(mdev_dev(vgpu->vdev.mdev)); > + if (device == NULL) > + return -ENODEV; > + vgpu->vdev.vfio_device = device; > + > + return 0; > +} > + > +static void kvmgt_put_vfio_device(struct intel_vgpu *vgpu) > +{ > + vfio_device_put(vgpu->vdev.vfio_device); > +} > + > +static int intel_vgpu_dmabuf_mgr_fd_mmap(struct file *file, > + struct vm_area_struct *vma) > +{ > + return -EPERM; > +} > + > +static int intel_vgpu_dmabuf_mgr_fd_release(struct inode *inode, > + struct file *filp) > +{ > + struct intel_vgpu *vgpu
[Intel-gfx] [PATCH] drm/i915: Remove dead code from runtime resume handler
From: Ville Syrjälä Remove the SNB PCH refclock init call from the runtime resume handler. I don't think it was actually needed even when we had SNB runtime PM, and if definitely isn't needed ever since SNB runtime PM was nuked in commit d4c5636e7447 ("drm/i915: Remove runtime PM for SNB"). Cc: Rodrigo Vivi Cc: Paulo Zanoni Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 2fdfaf135ea9..13735bc69c8c 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2461,9 +2461,6 @@ static int intel_runtime_resume(struct device *kdev) intel_guc_resume(dev_priv); - if (IS_GEN6(dev_priv)) - intel_init_pch_refclk(dev_priv); - if (IS_GEN9_LP(dev_priv)) { bxt_disable_dc9(dev_priv); bxt_display_core_init(dev_priv, true); -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Implement fbc_status "Compressing" info for all platforms
== Series Details == Series: drm/i915: Implement fbc_status "Compressing" info for all platforms URL : https://patchwork.freedesktop.org/series/25176/ State : success == Summary == Series 25176v1 drm/i915: Implement fbc_status "Compressing" info for all platforms https://patchwork.freedesktop.org/api/1.0/series/25176/revisions/1/mbox/ fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:447s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:435s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:580s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:508s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:487s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:483s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:432s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:410s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:416s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:499s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:467s fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 time:463s fi-kbl-7560u total:278 pass:263 dwarn:5 dfail:0 fail:0 skip:10 time:568s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:457s fi-skl-6700hqtotal:278 pass:239 dwarn:0 dfail:1 fail:17 skip:21 time:427s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:470s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:504s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:436s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:532s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:400s 2c9abf8ec88cf4b3d3735976bb0ff7a4991946b2 drm-tip: 2017y-06m-01d-13h-29m-05s UTC integration manifest c5201c2 drm/i915: Implement fbc_status "Compressing" info for all platforms == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4860/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 4/6] vfio: Define vfio based vgpu's dma-buf operations
On 6/1/2017 10:08 PM, Alex Williamson wrote: > On Thu, 1 Jun 2017 03:01:28 + > "Chen, Xiaoguang" wrote: > >> Hi Kirti, >> >>> -Original Message- >>> From: Kirti Wankhede [mailto:kwankh...@nvidia.com] >>> Sent: Thursday, June 01, 2017 1:23 AM >>> To: Chen, Xiaoguang ; Gerd Hoffmann >>> ; alex.william...@redhat.com; ch...@chris-wilson.co.uk; >>> intel-gfx@lists.freedesktop.org; linux-ker...@vger.kernel.org; >>> zhen...@linux.intel.com; Lv, Zhiyuan ; intel-gvt- >>> d...@lists.freedesktop.org; Wang, Zhi A ; Tian, Kevin >>> >>> Subject: Re: [PATCH v6 4/6] vfio: Define vfio based vgpu's dma-buf >>> operations >>> >>> >>> >>> On 5/31/2017 11:48 AM, Chen, Xiaoguang wrote: Hi, > -Original Message- > From: Gerd Hoffmann [mailto:kra...@redhat.com] > Sent: Monday, May 29, 2017 3:20 PM > To: Chen, Xiaoguang ; > alex.william...@redhat.com; ch...@chris-wilson.co.uk; intel- > g...@lists.freedesktop.org; linux-ker...@vger.kernel.org; > zhen...@linux.intel.com; Lv, Zhiyuan ; > intel-gvt- d...@lists.freedesktop.org; Wang, Zhi A > ; Tian, Kevin > Subject: Re: [PATCH v6 4/6] vfio: Define vfio based vgpu's dma-buf > operations > >> +struct vfio_vgpu_dmabuf_info { >> +__u32 argsz; >> +__u32 flags; >> +struct vfio_vgpu_plane_info plane_info; >> +__s32 fd; >> +__u32 pad; >> +}; > > Hmm, now you have argsz and flags twice in vfio_vgpu_dmabuf_info ... > > I think we should have something like this: > > struct vfio_vgpu_plane_info { > __u64 start; > __u64 drm_format_mod; > __u32 drm_format; > __u32 width; > __u32 height; > __u32 stride; > __u32 size; > __u32 x_pos; > __u32 y_pos; >__u32 padding; > }; > > struct vfio_vgpu_query_plane { > __u32 argsz; > __u32 flags; > struct vfio_vgpu_plane_info plane_info; >__u32 plane_id; >__u32 padding; > }; > > struct vfio_vgpu_create_dmabuf { > __u32 argsz; > __u32 flags; > struct vfio_vgpu_plane_info plane_info; >__u32 plane_id; >__s32 fd; > }; Good suggestion will apply in the next version. Thanks for review :) >>> >>> Can you define what are the expected values of 'flags' would be? >> Flags is not used in this case. It is defined to follow the rules of vfio >> ioctls. > > An important note about flags, the vendor driver must validate it. If > they don't and the user passes an arbitrary value there, then we have a > backwards compatibility issue with ever attempting to use the flags > field. The user passing in a flag unknown to the vendor driver should > return an -EINVAL response. In this case, we haven't defined any > flags, so the vendor driver needs to force the user to pass zero. There are two ways QEMU can get surface for console: 1. adding a region using region capability 2. dmabuf In both the above case surface parameters need to be queried from vendor driver are same. The structure would be : struct vfio_vgpu_surface_info { __u64 start; __u32 width; __u32 height; __u32 stride; __u32 size; __u32 x_pos; __u32 y_pos; __u32 padding; /* Only used when VFIO_VGPU_SURFACE_DMABUF_* flags set */ __u64 drm_format_mod; __u32 drm_format; }; We can use one ioctl to query surface information from vendor driver, structure would look like: struct vfio_vgpu_get_surface_info{ __u32 argsz; __u32 flags; #define VFIO_VGPU_SURFACE_DMABUF_CREATE (1 << 0) /* Create dmabuf */ #define VFIO_VGPU_SURFACE_DMABUF_QUERY (1 << 1) /* Query surface info for dmabuf */ #define VFIO_VGPU_SURFACE_REGION_QUERY (1 << 2) /* Query surface info for REGION type */ struct vfio_vgpu_surface_info surface; __u32 plane_id; __s32 fd; }; #define VFIO_DEVICE_SURFACE_INFO _IO(VFIO_TYPE, VFIO_BASE + 15) Vendor driver should return -EINVAL, if that type of query is not supported. I would like to design this interface to support both type, region cap and dmabuf. Thanks, Kirti ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Remove dead code from runtime resume handler
== Series Details == Series: drm/i915: Remove dead code from runtime resume handler URL : https://patchwork.freedesktop.org/series/25182/ State : success == Summary == Series 25182v1 drm/i915: Remove dead code from runtime resume handler https://patchwork.freedesktop.org/api/1.0/series/25182/revisions/1/mbox/ fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:451s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:432s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:581s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:513s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:488s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:479s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:438s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:412s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:417s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:493s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:460s fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 time:472s fi-kbl-7560u total:278 pass:263 dwarn:5 dfail:0 fail:0 skip:10 time:571s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:460s fi-skl-6700hqtotal:278 pass:239 dwarn:0 dfail:1 fail:17 skip:21 time:428s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:461s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:508s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:435s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:534s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:408s 2c9abf8ec88cf4b3d3735976bb0ff7a4991946b2 drm-tip: 2017y-06m-01d-13h-29m-05s UTC integration manifest 64b6005 drm/i915: Remove dead code from runtime resume handler == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4861/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tests/kms_cursor_crc: Test non-square cursors on IVB+
From: Ville Syrjälä IVB+ have the cursor "FBC" feature, meaning they support a somewhat limited form of non-square cursors. Let's test that. Signed-off-by: Ville Syrjälä --- tests/kms_cursor_crc.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c index 206f85268c9e..0940752b8c40 100644 --- a/tests/kms_cursor_crc.c +++ b/tests/kms_cursor_crc.c @@ -416,7 +416,13 @@ static bool has_nonsquare_cursors(uint32_t devid) * Test non-square cursors a bit on the platforms * that support such things. */ - return devid == PCI_CHIP_845_G || devid == PCI_CHIP_I865_G; + if (devid == PCI_CHIP_845_G || devid == PCI_CHIP_I865_G) + return true; + + if (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid)) + return false; + + return intel_gen(devid) >= 7; } static void test_cursor_size(data_t *data) -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 01/11] drm: Add HDMI 2.0 VIC support for AVI info-frames
On Wed, May 31, 2017 at 09:06:12PM +0530, Sharma, Shashank wrote: > Regards > > Shashank > > > On 5/31/2017 6:11 PM, Ville Syrjälä wrote: > > On Tue, May 30, 2017 at 10:00:12PM +0530, Sharma, Shashank wrote: > >> Regards > >> > >> Shashank > >> > >> > >> On 5/30/2017 9:43 PM, Ville Syrjälä wrote: > >>> On Tue, May 30, 2017 at 05:43:40PM +0530, Shashank Sharma wrote: > HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC > 1-64). > For any other mode, the VIC filed in AVI infoframes should be 0. > HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is > extended to (VIC 1-107). > > This patch adds a bool input variable, which indicates if the connected > sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a > HDMI 2.0 VIC to a HDMI 1.4 sink. > > This patch touches all drm drivers, who are callers of this function > drm_hdmi_avi_infoframe_from_display_mode but to make sure there is > no change in current behavior, is_hdmi2 is kept as false. > > In case of I915 driver, this patch checks the connector->display_info > to check if the connected display is HDMI 2.0. > > Cc: Ville Syrjala > Cc: Jose Abreu > Cc: Andrzej Hajda > Cc: Alex Deucher > Cc: Daniel Vetter > > PS: This patch touches a few lines in few files, which were > already above 80 char, so checkpatch gives 80 char warning again. > - gpu/drm/omapdrm/omap_encoder.c > - gpu/drm/i915/intel_sdvo.c > > V2: Rebase, Added r-b from Andrzej > > Reviewed-by: Andrzej Hajda > Signed-off-by: Shashank Sharma > --- > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 2 +- > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 2 +- > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- > drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++- > drivers/gpu/drm/bridge/sii902x.c | 2 +- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- > drivers/gpu/drm/drm_edid.c| 12 +++- > drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- > drivers/gpu/drm/i2c/tda998x_drv.c | 2 +- > drivers/gpu/drm/i915/intel_hdmi.c | 5 - > drivers/gpu/drm/i915/intel_sdvo.c | 3 ++- > drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +- > drivers/gpu/drm/omapdrm/omap_encoder.c| 3 ++- > drivers/gpu/drm/radeon/radeon_audio.c | 2 +- > drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +- > drivers/gpu/drm/sti/sti_hdmi.c| 2 +- > drivers/gpu/drm/tegra/hdmi.c | 2 +- > drivers/gpu/drm/tegra/sor.c | 2 +- > drivers/gpu/drm/vc4/vc4_hdmi.c| 2 +- > drivers/gpu/drm/zte/zx_hdmi.c | 2 +- > include/drm/drm_edid.h| 3 ++- > 21 files changed, 38 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > index 3c62c45..4923ddc 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > @@ -1864,7 +1864,7 @@ static void dce_v10_0_afmt_setmode(struct > drm_encoder *encoder, > dce_v10_0_audio_write_sad_regs(encoder); > dce_v10_0_audio_write_latency_fields(encoder, mode); > > -err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); > +err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, > false); > if (err < 0) { > DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); > return; > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > index c8ed0fa..4101684 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > @@ -1848,7 +1848,7 @@ static void dce_v11_0_afmt_setmode(struct > drm_encoder *encoder, > dce_v11_0_audio_write_sad_regs(encoder); > dce_v11_0_audio_write_latency_fields(encoder, mode); > > -err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); > +err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, > false); > if (err < 0) { > DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); > return; > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > index 3e90c19..a7f6b32 100644 > --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > @@ -1747,7 +1747,7 @@ static void dce_v8_0_afmt_setmode(struct > drm_encoder *encod
[Intel-gfx] [PATCH 2/6] drm-tip: 2017y-06m-01d-13h-29m-05s UTC integration manifest
From: Chris Wilson --- integration-manifest | 26 ++ 1 file changed, 26 insertions(+) create mode 100644 integration-manifest diff --git a/integration-manifest b/integration-manifest new file mode 100644 index 000..bb20fc4 --- /dev/null +++ b/integration-manifest @@ -0,0 +1,26 @@ +drm-intel drm-intel-fixes 9bd9590997b92fbd79fd028f704f6c584b4439d7 + drm/i915: Stop pretending to mask/unmask LPE audio interrupts +drm-upstream drm-fixes 400129f0a3ae989c30b37104bbc23b35c9d7a9a4 + Merge tag 'exynos-drm-fixes-for-v4.12' of git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos into drm-fixes +drm-intel drm-intel-next-fixes 88326ef05b262f681d837ecf65db10a7edb609f1 + drm/i915: Confirm the request is still active before adding it to the await +drm-intel drm-intel-next-queued 20bb377106af69d16269b1837e9a945b9f508a2e + drm/i915: Fix logical inversion for gen4 quirking +drm-upstream drm-next 2a1720376adda5ecf8e636fbfb05339c7dad1c55 + Backmerge tag 'v4.12-rc3' into drm-next +sound-upstream for-next 7421a1671abe5bc07cc7a09f5a1be45acc403db7 + ALSA: pcm: include pcm_local.h and remove some extraneous tabs +sound-upstream for-linus d2c3b14e1f0dcebdb695617c0c1342a36b914a47 + ALSA: hda - Fix applying MSI dual-codec mobo quirk +drm-intel topic/core-for-CI 081d1f1a327affcd9449e92257fc75fa83449832 + perf/core: Avoid removing shared pmu_context on unregister +drm-misc drm-misc-next 991dca01dd6719d1dd1bd4c39f5a12680c74b15e + drm/exynos: Drop drm_vblank_cleanup +drm-misc drm-misc-next-fixes 2d7b56378d32b0cf006f8944cbba4046df45dd25 + drm/rockchip: gem: add the lacks lock and trivial changes +drm-misc drm-misc-fixes 869e188a35c93f1bad7bf16c32f34a6feb5f79f1 + drm: Fix locking in drm_atomic_helper_resume +drm-intel topic/dp-quirks b31e85eda38c58cae986162ae2c462b53b0a2065 + drm/i915: Detect USB-C specific dongles before reducing M and N +drm-intel topic/e1000e-fix 4e5684f930587bd22565f404eb3c5e417a994ccc + e1000e: Undo e1000e_pm_freeze if __e1000_shutdown fails -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/6] drm/i915: Fix logical inversion for gen4 quirking
From: Chris Wilson The assertion that we want to make before disabling the pin of the pages for the unknown swizzling quirk is that the quirk is indeed active, and that the quirk is disabled before we do apply it to the pages. Fixes: 2c3a3f44dc13 ("drm/i915: Fix pages pin counting around swizzle quirk") Fixes: 957870f93412 ("drm/i915: Split out i915_gem_object_set_tiling()") Signed-off-by: Chris Wilson Cc: Chris Wilson Cc: Joonas Lahtinen Cc: Tvrtko Ursulin Link: http://patchwork.freedesktop.org/patch/msgid/20170521124014.27678-1-ch...@chris-wilson.co.uk Reviewed-bhy: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_gem_tiling.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c index a0d6d43..fb5231f 100644 --- a/drivers/gpu/drm/i915/i915_gem_tiling.c +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c @@ -278,7 +278,7 @@ static bool i915_vma_fence_prepare(struct i915_vma *vma, obj->mm.quirked = false; } if (!i915_gem_object_is_tiled(obj)) { - GEM_BUG_ON(!obj->mm.quirked); + GEM_BUG_ON(obj->mm.quirked); __i915_gem_object_pin_pages(obj); obj->mm.quirked = true; } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx