Re: [Intel-gfx] [PATCH] drm/i915: Avoid double mutex lock applying pipe A quirk during sanitize_crtc()
On Mon, Jun 09, 2014 at 07:47:10AM +0100, Chris Wilson wrote: > Thomas found that his machine would deadlock reloading the i915.ko > module after resume. He identified that this was caused by the > reacquisition of the connection mutex inside intel_enable_pipe_a() > during the CRTC sanitization routine. This will only affect machines > that quirk PIPE A, i.e. the original 830m chipsets. > > This patch move the locking into a wrapper function so that > intel_enable_pipe_a() can bypass the locking knowing that it already > holds the correct locks. > > Reported-by: Thomas Richter > Signed-off-by: Chris Wilson > Cc: Thomas Richter > Cc: Daniel Vetter > Cc: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_display.c | 54 > ++ > 1 file changed, 35 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index c1f79a1..26d3424 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8387,9 +8387,10 @@ mode_fits_in_fbdev(struct drm_device *dev, > #endif > } > > -bool intel_get_load_detect_pipe(struct drm_connector *connector, > - struct drm_display_mode *mode, > - struct intel_load_detect_pipe *old) > +static bool > +__intel_get_load_detect_pipe(struct drm_connector *connector, > + struct drm_display_mode *mode, > + struct intel_load_detect_pipe *old) > { > struct intel_crtc *intel_crtc; > struct intel_encoder *intel_encoder = > @@ -8405,7 +8406,7 @@ bool intel_get_load_detect_pipe(struct drm_connector > *connector, > connector->base.id, connector->name, > encoder->base.id, encoder->name); > > - mutex_lock(&dev->mode_config.connection_mutex); > + lockdep_assert_held(&dev->mode_config.connection_mutex); > > /* >* Algorithm gets a little messy: > @@ -8449,7 +8450,7 @@ bool intel_get_load_detect_pipe(struct drm_connector > *connector, >*/ > if (!crtc) { > DRM_DEBUG_KMS("no pipe available for load-detect\n"); > - goto fail_unlock_connector; > + return false; > } > > mutex_lock(&crtc->mutex); > @@ -8503,14 +8504,13 @@ bool intel_get_load_detect_pipe(struct drm_connector > *connector, > else > intel_crtc->new_config = NULL; > mutex_unlock(&crtc->mutex); > -fail_unlock_connector: > - mutex_unlock(&dev->mode_config.connection_mutex); > > return false; > } > > -void intel_release_load_detect_pipe(struct drm_connector *connector, > - struct intel_load_detect_pipe *old) > +static void > +__intel_release_load_detect_pipe(struct drm_connector *connector, > + struct intel_load_detect_pipe *old) > { > struct intel_encoder *intel_encoder = > intel_attached_encoder(connector); > @@ -8518,6 +8518,8 @@ void intel_release_load_detect_pipe(struct > drm_connector *connector, > struct drm_crtc *crtc = encoder->crtc; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > + lockdep_assert_held(&connector->dev->mode_config.connection_mutex); > + > DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n", > connector->base.id, connector->name, > encoder->base.id, encoder->name); > @@ -8533,17 +8535,32 @@ void intel_release_load_detect_pipe(struct > drm_connector *connector, > drm_framebuffer_unregister_private(old->release_fb); > drm_framebuffer_unreference(old->release_fb); > } > + } else { > + /* Switch crtc and encoder back off if necessary */ > + if (old->dpms_mode != DRM_MODE_DPMS_ON) > + connector->funcs->dpms(connector, old->dpms_mode); > + } This part looks like an unrelated change? -Daniel > + mutex_unlock(&crtc->mutex); > +} > > - mutex_unlock(&crtc->mutex); > +bool intel_get_load_detect_pipe(struct drm_connector *connector, > + struct drm_display_mode *mode, > + struct intel_load_detect_pipe *old) > +{ > + mutex_lock(&connector->dev->mode_config.connection_mutex); > + if (!__intel_get_load_detect_pipe(connector, mode, old)) { > mutex_unlock(&connector->dev->mode_config.connection_mutex); > - return; > + return false; > } > > - /* Switch crtc and encoder back off if necessary */ > - if (old->dpms_mode != DRM_MODE_DPMS_ON) > - connector->funcs->dpms(connector, old->dpms_mode); > + /* lock will be released by intel_release_load_detect_pipe() */ > + return true; > +} > > - mutex_unlock(&crtc->mutex); > +void intel_release_load_detect_pipe(struct drm_connector *connect
Re: [Intel-gfx] [PATCH] drm/i915: Avoid double mutex lock applying pipe A quirk during sanitize_crtc()
On Mon, Jun 09, 2014 at 11:30:26AM +0300, Ville Syrjälä wrote: > On Mon, Jun 09, 2014 at 07:47:10AM +0100, Chris Wilson wrote: > > Thomas found that his machine would deadlock reloading the i915.ko > > module after resume. He identified that this was caused by the > > reacquisition of the connection mutex inside intel_enable_pipe_a() > > during the CRTC sanitization routine. This will only affect machines > > that quirk PIPE A, i.e. the original 830m chipsets. > > > > This patch move the locking into a wrapper function so that > > intel_enable_pipe_a() can bypass the locking knowing that it already > > holds the correct locks. > > It can still try to grab crtc->mutex twice. Looks like Danial undid my > fix to not take all the modeset locks around > intel_modeset_setup_hw_state(). Hm, I didn't find anything in git logs and we've had places where we used modeset_lock_all since a long time. And I don't see how Chris' patch wouldn't address this here. Can you please explain? Thanks, Daniel > > > > > Reported-by: Thomas Richter > > Signed-off-by: Chris Wilson > > Cc: Thomas Richter > > Cc: Daniel Vetter > > Cc: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/intel_display.c | 54 > > ++ > > 1 file changed, 35 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index c1f79a1..26d3424 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -8387,9 +8387,10 @@ mode_fits_in_fbdev(struct drm_device *dev, > > #endif > > } > > > > -bool intel_get_load_detect_pipe(struct drm_connector *connector, > > - struct drm_display_mode *mode, > > - struct intel_load_detect_pipe *old) > > +static bool > > +__intel_get_load_detect_pipe(struct drm_connector *connector, > > +struct drm_display_mode *mode, > > +struct intel_load_detect_pipe *old) > > { > > struct intel_crtc *intel_crtc; > > struct intel_encoder *intel_encoder = > > @@ -8405,7 +8406,7 @@ bool intel_get_load_detect_pipe(struct drm_connector > > *connector, > > connector->base.id, connector->name, > > encoder->base.id, encoder->name); > > > > - mutex_lock(&dev->mode_config.connection_mutex); > > + lockdep_assert_held(&dev->mode_config.connection_mutex); > > > > /* > > * Algorithm gets a little messy: > > @@ -8449,7 +8450,7 @@ bool intel_get_load_detect_pipe(struct drm_connector > > *connector, > > */ > > if (!crtc) { > > DRM_DEBUG_KMS("no pipe available for load-detect\n"); > > - goto fail_unlock_connector; > > + return false; > > } > > > > mutex_lock(&crtc->mutex); > > @@ -8503,14 +8504,13 @@ bool intel_get_load_detect_pipe(struct > > drm_connector *connector, > > else > > intel_crtc->new_config = NULL; > > mutex_unlock(&crtc->mutex); > > -fail_unlock_connector: > > - mutex_unlock(&dev->mode_config.connection_mutex); > > > > return false; > > } > > > > -void intel_release_load_detect_pipe(struct drm_connector *connector, > > - struct intel_load_detect_pipe *old) > > +static void > > +__intel_release_load_detect_pipe(struct drm_connector *connector, > > +struct intel_load_detect_pipe *old) > > { > > struct intel_encoder *intel_encoder = > > intel_attached_encoder(connector); > > @@ -8518,6 +8518,8 @@ void intel_release_load_detect_pipe(struct > > drm_connector *connector, > > struct drm_crtc *crtc = encoder->crtc; > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > > > + lockdep_assert_held(&connector->dev->mode_config.connection_mutex); > > + > > DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n", > > connector->base.id, connector->name, > > encoder->base.id, encoder->name); > > @@ -8533,17 +8535,32 @@ void intel_release_load_detect_pipe(struct > > drm_connector *connector, > > drm_framebuffer_unregister_private(old->release_fb); > > drm_framebuffer_unreference(old->release_fb); > > } > > + } else { > > + /* Switch crtc and encoder back off if necessary */ > > + if (old->dpms_mode != DRM_MODE_DPMS_ON) > > + connector->funcs->dpms(connector, old->dpms_mode); > > + } > > + mutex_unlock(&crtc->mutex); > > +} > > > > - mutex_unlock(&crtc->mutex); > > +bool intel_get_load_detect_pipe(struct drm_connector *connector, > > + struct drm_display_mode *mode, > > + struct intel_load_detect_pipe *old) > > +{ > > + mutex_lock(&connector->dev->mode_config.connection_mutex); > > + if (!__intel_get_load_detect_pipe(connector, mode, old)) { > > mute
Re: [Intel-gfx] [PATCH] drm/i915: Fix VLV CRC reading.
On Thu, Jun 05, 2014 at 02:28:17PM -0700, Rodrigo Vivi wrote: > Adding missing Display mmio reg offset. > > Credits-to: Laws, Philip > Cc: He, Shuang > Signed-off-by: Rodrigo Vivi Is there no bugzilla that pipe crc tests on DP/eDP ports aren't working on byt? Can you please chase this down with QA? Patch is queued for -next. Thanks, Daniel > --- > drivers/gpu/drm/i915/i915_reg.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 286f05c..05e2541 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -2627,7 +2627,7 @@ enum punit_power_well { > > #define PORT_DFT_I9XX0x61150 > #define DC_BALANCE_RESET (1 << 25) > -#define PORT_DFT2_G4X0x61154 > +#define PORT_DFT2_G4X(dev_priv->info.display_mmio_offset + > 0x61154) > #define DC_BALANCE_RESET_VLV (1 << 31) > #define PIPE_SCRAMBLE_RESET_MASK (0x3 << 0) > #define PIPE_B_SCRAMBLE_RESET (1 << 1) > -- > 1.9.3 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Disable FBC on Haswell
On Mon, Jun 09, 2014 at 10:06:29PM +0300, Jani Nikula wrote: > On Mon, 09 Jun 2014, Ville Syrjälä wrote: > > On Mon, Jun 09, 2014 at 09:12:18PM +0300, Jani Nikula wrote: > >> On Fri, 06 Jun 2014, Chris Wilson wrote: > >> > It causes black screen on bootup and is approximately 100x slower than > >> > running with FBC disabled, so the GPU runs at a high frequency for much > >> > longer - completely contrary to the power saving claims. It also still > >> > has mutex deadlocks in multi-head scenarios, which can lead to a > >> > system/X lockup. These bugs were known before FBC was enabled by default > >> > on Haswell and still have not been fixed. > >> > > >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79716 > >> > Reported-and-tested-by: Jon Kristensen > >> > Signed-off-by: Chris Wilson > >> > Cc: sta...@vger.kernel.org > >> > --- > >> > drivers/gpu/drm/i915/intel_pm.c | 3 +-- > >> > 1 file changed, 1 insertion(+), 2 deletions(-) > >> > > >> > diff --git a/drivers/gpu/drm/i915/intel_pm.c > >> > b/drivers/gpu/drm/i915/intel_pm.c > >> > index e403010540a5..0b8a6010427e 100644 > >> > --- a/drivers/gpu/drm/i915/intel_pm.c > >> > +++ b/drivers/gpu/drm/i915/intel_pm.c > >> > @@ -511,8 +511,7 @@ void intel_update_fbc(struct drm_device *dev) > >> > obj = intel_fb->obj; > >> > adjusted_mode = &intel_crtc->config.adjusted_mode; > >> > > >> > -if (i915.enable_fbc < 0 && > >> > -INTEL_INFO(dev)->gen <= 7 && !IS_HASWELL(dev)) { > >> > +if (i915.enable_fbc < 0) { > >> > >> Not only does this disable FBC by default on Haswell but also on all > >> current and future platforms, including Broadwell. Shouldn't you leave > >> the INTEL_INFO(dev)->gen <= 7 part intact? > > > > The current FBC code is universally broken. > > Well, then the commit message should reflect that! :p s/Haswell/Haswell+/ and it's correct ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Avoid div-by-zero when pixel_multiplier is zero
On Mon, Jun 09, 2014 at 04:20:46PM +0300, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > On certain platforms pixel_multiplier is read out in > .get_pipe_config(), but it also gets used to calculate the > pixel clock in intel_sdvo_get_config(). If the pipe is disable > but some SDVO outputs are active, we may end up dividing by zero > in intel_sdvo_get_config(). > > To avoid the problem simply check for zero pixel_multiplier and skip > the division. Another attempt at fixing this involved populating > pixel_multiplier to 1 even for disabled pipes, but that triggered a > WARN because SDVO_CMD_GET_CLOCK_RATE_MULT command failed and thus > encoder_pixel_multiplier was left at zero and didn't match > pipe_config->pixel_multiplier. > > The "divide by pixel_multiplier" operation got introduced here: > commit 18442d08786472c63a0a80c27f92b033dffc26de > Author: Ville Syrjälä > Date: Fri Sep 13 16:00:08 2013 +0300 > > drm/i915: Fix port_clock and adjusted_mode.clock readout all over > > and it has caused a regression on certain machines since they would > hit the div-by-zero during resume. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76520 > Cc: # 3.13+ > Tested-by: Tim Richardson > Signed-off-by: Ville Syrjälä Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_sdvo.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c > b/drivers/gpu/drm/i915/intel_sdvo.c > index 6a4d5bc..20375cc 100644 > --- a/drivers/gpu/drm/i915/intel_sdvo.c > +++ b/drivers/gpu/drm/i915/intel_sdvo.c > @@ -1385,7 +1385,9 @@ static void intel_sdvo_get_config(struct intel_encoder > *encoder, >>> SDVO_PORT_MULTIPLY_SHIFT) + 1; > } > > - dotclock = pipe_config->port_clock / pipe_config->pixel_multiplier; > + dotclock = pipe_config->port_clock; > + if (pipe_config->pixel_multiplier) > + dotclock /= pipe_config->pixel_multiplier; > > if (HAS_PCH_SPLIT(dev)) > ironlake_check_encoder_dotclock(pipe_config, dotclock); > -- > 1.8.5.5 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Avoid double mutex lock applying pipe A quirk during sanitize_crtc()
On Tue, Jun 10, 2014 at 08:59:42AM +0200, Daniel Vetter wrote: > On Mon, Jun 09, 2014 at 07:47:10AM +0100, Chris Wilson wrote: > > + } else { > > + /* Switch crtc and encoder back off if necessary */ > > + if (old->dpms_mode != DRM_MODE_DPMS_ON) > > + connector->funcs->dpms(connector, old->dpms_mode); > > + } > > This part looks like an unrelated change? It's just diff. The change was to rejig the braces to remove the duplicated unlock + return. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] "sna: Do not allow imported buffers to be cached" broke my X11
Lately current git for xf86-video-intel broke for me. Bisecting led me to "sna: Do not allow imported buffers to be cached" and then reverting that on top(with one conflict) of current git restored my X. The symptom is that drawing starts with lots of random pixels and then it clears after 1 second. Hard to describe properly. Here is my Xorg log [336440.143] X.Org X Server 1.15.1 Release Date: 2014-04-13 [336440.143] X Protocol Version 11, Revision 0 [336440.143] Build Operating System: Linux 3.10.40 x86_64 Gentoo [336440.143] Current Operating System: Linux jocke 3.10.40 #3 SMP Tue Jun 3 20:39:59 CEST 2014 x86_64 [336440.143] Kernel command line: root=/dev/sda2 rootflags=discard net.ifnames=0 [336440.143] Build Date: 03 June 2014 10:50:57PM [336440.143] [336440.143] Current version of pixman: 0.32.4 [336440.143]Before reporting problems, check http://wiki.x.org to make sure that you have the latest version. [336440.143] Markers: (--) probed, (**) from config file, (==) default setting, (++) from command line, (!!) notice, (II) informational, (WW) warning, (EE) error, (NI) not implemented, (??) unknown. [336440.143] (==) Log file: "/var/log/Xorg.0.log", Time: Sat Jun 7 18:12:05 2014 [336440.143] (==) Using config directory: "/etc/X11/xorg.conf.d" [336440.143] (==) Using system config directory "/usr/share/X11/xorg.conf.d" [336440.143] (==) No Layout section. Using the first Screen section. [336440.143] (==) No screen section available. Using defaults. [336440.143] (**) |-->Screen "Default Screen Section" (0) [336440.143] (**) | |-->Monitor "" [336440.143] (==) No device specified for screen "Default Screen Section". Using the first device section listed. [336440.143] (**) | |-->Device "card0" [336440.144] (==) No monitor specified for screen "Default Screen Section". Using a default monitor configuration. [336440.144] (==) Automatically adding devices [336440.144] (==) Automatically enabling devices [336440.144] (==) Automatically adding GPU devices [336440.144] (WW) The directory "/usr/share/fonts/OTF/" does not exist. [336440.144]Entry deleted from font path. [336440.144] (WW) The directory "/usr/share/fonts/Type1/" does not exist. [336440.144]Entry deleted from font path. [336440.144] (WW) `fonts.dir' not found (or not valid) in "/usr/share/fonts/75dpi/". [336440.144]Entry deleted from font path. [336440.144](Run 'mkfontdir' on "/usr/share/fonts/75dpi/"). [336440.144] (==) FontPath set to: /usr/share/fonts/misc/, /usr/share/fonts/TTF/, /usr/share/fonts/100dpi/ [336440.144] (==) ModulePath set to "/usr/lib64/xorg/modules" [336440.144] (II) The server relies on udev to provide the list of input devices. If no devices become available, reconfigure udev or disable AutoAddDevices. [336440.144] (II) Loader magic: 0x807c60 [336440.144] (II) Module ABI versions: [336440.144]X.Org ANSI C Emulation: 0.4 [336440.144]X.Org Video Driver: 15.0 [336440.144]X.Org XInput driver : 20.0 [336440.144]X.Org Server Extension : 8.0 [336440.144] (II) xfree86: Adding drm device (/dev/dri/card0) [336440.144] (--) PCI:*(0:0:2:0) 8086:0162:1043:844d rev 9, Mem @ 0xf780/4194304, 0xe000/268435456, I/O @ 0xf000/64 [336440.145] Initializing built-in extension Generic Event Extension [336440.145] Initializing built-in extension SHAPE [336440.145] Initializing built-in extension MIT-SHM [336440.145] Initializing built-in extension XInputExtension [336440.145] Initializing built-in extension XTEST [336440.145] Initializing built-in extension BIG-REQUESTS [336440.145] Initializing built-in extension SYNC [336440.145] Initializing built-in extension XKEYBOARD [336440.145] Initializing built-in extension XC-MISC [336440.145] Initializing built-in extension XINERAMA [336440.145] Initializing built-in extension XFIXES [336440.145] Initializing built-in extension RENDER [336440.145] Initializing built-in extension RANDR [336440.145] Initializing built-in extension COMPOSITE [336440.145] Initializing built-in extension DAMAGE [336440.145] Initializing built-in extension MIT-SCREEN-SAVER [336440.145] Initializing built-in extension DOUBLE-BUFFER [336440.145] Initializing built-in extension RECORD [336440.145] Initializing built-in extension DPMS [336440.145] Initializing built-in extension Present [336440.145] Initializing built-in extension DRI3 [336440.145] Initializing built-in extension X-Resource [336440.145] Initializing built-in extension XVideo [336440.145] Initializing built-in extension XVideo-MotionCompensation [336440.145] Initializing built-in extension XFree86-VidModeExtension [336440.145] Initializing built-in extension XFree86-DGA [336440.145] Initializing built-in extension XFree86-DRI [336440.145] Initializing built-in extension DRI2 [336440.145] (II) LoadModule: "glx" [336440.145] (II) Loading /usr/lib64/xorg/modules/extensions/libglx.so [336440.145] (II) Module glx: vendor="X.Org Foundatio
[Intel-gfx] [PATCH] drm/i915 : Removed the unconditional cross engine/ring update of MBOX registers
From: Akash Goel Removed the unconditional cross engine/ring update of MBOX registers. The MBox update will done only when needed when the actual inter ring dependency has been ascertained. Although this late sync could affect the Media performance slightly but it shall improve the residency time of individual power wells in C6 state. Signed-off-by: Sourab Gupta --- drivers/gpu/drm/i915/intel_ringbuffer.c | 53 - drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +-- 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 279488a..c684b47 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -672,43 +672,35 @@ static void render_ring_cleanup(struct intel_engine_cs *ring) } static int gen6_signal(struct intel_engine_cs *signaller, - unsigned int num_dwords) + struct intel_engine_cs *waiter, + u32 seqno) { - struct drm_device *dev = signaller->dev; - struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_engine_cs *useless; - int i, ret; + u32 mbox_reg = signaller->semaphore.mbox.signal[waiter->id]; + int ret; /* NB: In order to be able to do semaphore MBOX updates for varying * number of rings, it's easiest if we round up each individual update * to a multiple of 2 (since ring updates must always be a multiple of * 2) even though the actual update only requires 3 dwords. */ -#define MBOX_UPDATE_DWORDS 4 - if (i915_semaphore_is_enabled(dev)) - num_dwords += ((I915_NUM_RINGS-1) * MBOX_UPDATE_DWORDS); - else - return intel_ring_begin(signaller, num_dwords); - ret = intel_ring_begin(signaller, num_dwords); +#define MBOX_UPDATE_DWORDS 4 + ret = intel_ring_begin(signaller, MBOX_UPDATE_DWORDS); +#undef MBOX_UPDATE_DWORDS if (ret) return ret; -#undef MBOX_UPDATE_DWORDS - - for_each_ring(useless, dev_priv, i) { - u32 mbox_reg = signaller->semaphore.mbox.signal[i]; - if (mbox_reg != GEN6_NOSYNC) { - intel_ring_emit(signaller, MI_LOAD_REGISTER_IMM(1)); - intel_ring_emit(signaller, mbox_reg); - intel_ring_emit(signaller, signaller->outstanding_lazy_seqno); - intel_ring_emit(signaller, MI_NOOP); - } else { - intel_ring_emit(signaller, MI_NOOP); - intel_ring_emit(signaller, MI_NOOP); - intel_ring_emit(signaller, MI_NOOP); - intel_ring_emit(signaller, MI_NOOP); - } + if (mbox_reg != GEN6_NOSYNC) { + intel_ring_emit(signaller, MI_LOAD_REGISTER_IMM(1)); + intel_ring_emit(signaller, mbox_reg); + intel_ring_emit(signaller, seqno); + intel_ring_emit(signaller, MI_NOOP); + } else { + intel_ring_emit(signaller, MI_NOOP); + intel_ring_emit(signaller, MI_NOOP); + intel_ring_emit(signaller, MI_NOOP); + intel_ring_emit(signaller, MI_NOOP); } + __intel_ring_advance(signaller); return 0; } @@ -727,7 +719,7 @@ gen6_add_request(struct intel_engine_cs *ring) { int ret; - ret = ring->semaphore.signal(ring, 4); + ret = intel_ring_begin(ring, 4); if (ret) return ret; @@ -779,6 +771,13 @@ gen6_ring_sync(struct intel_engine_cs *waiter, /* If seqno wrap happened, omit the wait with no-ops */ if (likely(!i915_gem_has_seqno_wrapped(waiter->dev, seqno))) { + /* Add the Mbox update command in the Signaller ring, +* this a point where the actual inter ring dependency has +* been ascertained. +*/ + ret = signaller->semaphore.signal(signaller, waiter, seqno+1); + if (ret) + return ret; intel_ring_emit(waiter, dw1 | wait_mbox); intel_ring_emit(waiter, seqno); intel_ring_emit(waiter, 0); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 910c83c..216c341 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -142,8 +142,8 @@ struct intel_engine_cs { struct intel_engine_cs *to, u32 seqno); int (*signal)(struct intel_engine_cs *signaller, - /* num_dwords needed by caller */ - unsigned int num_dwords); + struct intel_engine_cs *waiter, +
Re: [Intel-gfx] [PATCH] drm/i915 : Removed the unconditional cross engine/ring update of MBOX registers
On Tue, Jun 10, 2014 at 12:48:44PM +0530, sourab.gu...@intel.com wrote: > From: Akash Goel > > Removed the unconditional cross engine/ring update of MBOX registers. > The MBox update will done only when needed when the actual inter ring > dependency has been ascertained. Although this late sync could affect > the Media performance slightly but it shall improve the residency time > of individual power wells in C6 state. NAK. Did you even consider the deadlocks above and beyond the issues with latency? Maybe suggest that the hardware guys consider a reordering write FIFO next time like elsewhere on the chip. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: set backlight duty cycle after backlight enable for gen4
On Mon, Jun 09, 2014 at 06:24:34PM +0300, Jani Nikula wrote: > For reasons I can't claim to fully understand gen4 seems to require > backlight duty cycle setting after the backlight has been enabled, or > else black screen follows. I don't have documentation for the correct > sequence on gen4 either. Confirmed on Dell Latitude D630 and MacBook4,1. > > This fixes a regression introduced by > commit b35684b8fa94e04f55fd38bf672b737741d2f9e2 > Author: Jani Nikula > Date: Thu Nov 14 12:13:41 2013 +0200 > > drm/i915: do full backlight setup at enable time > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=75791 > Reported-and-tested-by: m...@lm7.fr > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79423 > Reported-and-tested-by: Marc Milgram > Cc: Stable # 3.14+ > Signed-off-by: Jani Nikula Yeah, docs don't mention anything. But if it works it's fine, so Acked-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_panel.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_panel.c > b/drivers/gpu/drm/i915/intel_panel.c > index 5e6c888b4928..38a98570d10c 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -798,9 +798,6 @@ static void i965_enable_backlight(struct intel_connector > *connector) > ctl = freq << 16; > I915_WRITE(BLC_PWM_CTL, ctl); > > - /* XXX: combine this into above write? */ > - intel_panel_actually_set_backlight(connector, panel->backlight.level); > - > ctl2 = BLM_PIPE(pipe); > if (panel->backlight.combination_mode) > ctl2 |= BLM_COMBINATION_MODE; > @@ -809,6 +806,8 @@ static void i965_enable_backlight(struct intel_connector > *connector) > I915_WRITE(BLC_PWM_CTL2, ctl2); > POSTING_READ(BLC_PWM_CTL2); > I915_WRITE(BLC_PWM_CTL2, ctl2 | BLM_PWM_ENABLE); > + > + intel_panel_actually_set_backlight(connector, panel->backlight.level); > } > > static void vlv_enable_backlight(struct intel_connector *connector) > -- > 1.9.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915 GPU lockup
On Mon, Jun 09, 2014 at 12:32:38AM +0530, Gideon D'souza wrote: > Hey Bruno, > > So I got my system up and running finally by just doing a sudo yum update. > > So it seems like the problem is fixed is a newer library. I looked at : > https://bugs.freedesktop.org That was a bug in mesa. But if it is fixed, don't worry about filing a bug report unless you get a new hang. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] "sna: Do not allow imported buffers to be cached" broke my X11
On Sat, Jun 07, 2014 at 06:23:44PM +0200, Tjernlund wrote: > Lately current git for xf86-video-intel broke for me. > Bisecting led me to "sna: Do not allow imported buffers to be cached" > and then reverting that on top(with one conflict) of current git > restored my X. > > The symptom is that drawing starts with lots of random pixels and then > it clears after 1 second. Hard to describe properly. We solved this, whilst it was in the moderation queue. Just another instance of 3.10.y being broken. LTS guys should really look at improving the flow of bugfixes back to those kernels... -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915 GPU lockup
Thanks very Much Chris and Bruno :) On Tue, Jun 10, 2014 at 12:57 PM, Chris Wilson wrote: > On Mon, Jun 09, 2014 at 12:32:38AM +0530, Gideon D'souza wrote: >> Hey Bruno, >> >> So I got my system up and running finally by just doing a sudo yum update. >> >> So it seems like the problem is fixed is a newer library. I looked at : >> https://bugs.freedesktop.org > > That was a bug in mesa. But if it is fixed, don't worry about filing a > bug report unless you get a new hang. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [ANNOUNCE] xf86-video-intel 2.99.912
Snapshot 2.99.912 (2014-06-10) == A final round of features. We have everything from support for variable cursor sizes, support for the DRI3 and Present extensions, improved DRI2 support, support for Xserver 1.16, userptr from kernel 3.16, and precursory support for DP multistream transport, * Avoid discarding dirty pixels when promoting a migration to cover the whole pixmap. Regression in 2.99.911 https://bugs.freedesktop.org/show_bug.cgi?id=77063 https://bugs.freedesktop.org/show_bug.cgi?id=77178 * Avoid overextending degenerate lines (and consequentially accessing pixels outside of our damaged area). https://bugs.freedesktop.org/show_bug.cgi?id=77074 * Fix subpixel glyph rendering on gen2 devices (830-865 chipsets) Regression in 2.99.911 https://bugs.freedesktop.org/show_bug.cgi?id=77201 * Share the global pixman glyph cache between ZaphodHeads https://bugs.freedesktop.org/show_bug.cgi?id=54707 * Light up all connected outputs, even if their status is unknown, on takeover from fbcon. This prevents loss of display after a resume on recent kernels, for example. https://bugs.freedesktop.org/show_bug.cgi?id=77768 * Show the video overlay (when supported by the hardware) across all outputs. https://bugs.freedesktop.org/show_bug.cgi?id=77802 * Do not discard damage when performing "BLT" spans inplace with the CPU. Regression from 2.20.10 * Avoid discarding IO buffers too early during their preparation for a new batch https://bugs.freedesktop.org/show_bug.cgi?id=79238 * Fix fallback handling for displaying large scaled framebuffers (that are too large to be scaled by the GPU in a single pass) https://bugs.freedesktop.org/show_bug.cgi?id=79320 * Listen to external modifications of backlight value and propagate the notifications to RandR clients. This should make the GUI report ACPI keypresses to change the backlight correctly. https://bugs.freedesktop.org/show_bug.cgi?id=79699 * UXA: fix pageflips with 3 heads. * UXA: do not report a BadMatch error for DRI2GetMsc - as clients are often unprepared and die when they get the unexpected error. Complete list of changes since 2.99.911 --- Adam Jackson (1): configure: Don't link the driver against libX11 Chris Wilson (287): intel: Refactor finding device path if unknown intel: Do not close server fds sna: Only enable cursor support if the hw cursor is supported sna: Eliminate a few conditionals in glyph fast path sna: Consolidate handling of uncacheable glyphs sna: Remove one conditional from rendering glyphs into a mask intel-virtual-output: Do not detach with DBG enabled intel-virtual-output: Add a little more DBG around damaging clones intel-virtual-output: Fix damage iteration over active list sna: Print probed maximum cursor size intel-virtual-output: Add DBG option to force 16 bit transfers sna: Tighten detection of GCs that translate to solid fills sna: Move cursor reload into crtc notify sna: Support variable sized cursors sna: Clear the surrounding areas of small cursors sna: Reorder the cursor cache search sna: Cursors only need to be cleared when they are shrunk sna: Fix 2-color to ARGB cursor conversion sna: RefCursor is too recent sna: drmGetCap is too recent for our supported versions of libdrm sna: Cursors are invariant sna: Virtual CRTCs are last, so break loops early sna: Our cursors are always square. sna: Only transform the temporary cursor coordinates sna: Suppress a compiler warning sna: Search cursor cache first sna: Regularly check that damage does exceed the pixmap sna: Promote to the GPU operations that cover the whole pixmap sna: Discard damage tracking for operations to the whole pixmap sna: Allow reassignment of inactive VMA if not mapped into the GTT sna: Use a cheaper check for a replacement operation sna: Fix predicate inversion for assertion sna: Update tiling flags after reusing inactive VMA sna: Prevent signal re-entrancy into cursor update routines sna: Silence compiler warning sna: Short-circuit repeated clearing to the same color sna/gen2+: Replace composite sources with solids where possible sna: Precompute OVER/ADD with solids to convert it into a BLT operation sna/gen7: Move constants MOCS into chipset specific info blocks sna/gen2+: Beware the unattached ShmPixmap sna: Remove unitialized use of 'cursor' sna/gen8: w/a for NULL depth buffer sna/gen8: Shrink 3DSTATE_SAMPLE_PATTERN packet sna: Avoid discarding damage when applying WHOLE hint to pixmap migration sna: Avoid double application of pixel widening for degenerate lines sna: Simplify checking for singular damage intel-virtual-output
Re: [Intel-gfx] [PATCH v3 1/4] drm/crtc: Add property for aspect ratio
On Jun-05-2014 2:58 PM, Thierry Reding wrote: > On Thu, Jun 05, 2014 at 02:40:08PM +0530, Vandana Kannan wrote: > [...] >> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > [...] >> /** >> + * drm_mode_create_aspect_ratio_property - create aspect ratio property >> + * @dev: DRM device >> + * >> + * Called by a driver the first time it's needed, must be attached to >> desired >> + * connectors. >> + */ >> +int drm_mode_create_aspect_ratio_property(struct drm_device *dev) >> +{ >> +if (dev->mode_config.aspect_ratio_property) >> +return 0; >> + >> +dev->mode_config.aspect_ratio_property = >> +drm_property_create_enum(dev, 0, "aspect ratio", >> +drm_aspect_ratio_enum_list, >> +ARRAY_SIZE(drm_aspect_ratio_enum_list)); >> + >> +return 0; > > Sorry for not noticing this before: what if drm_propert_create_enum() > fails? Should that return an error? This function will currently > silently ignore failure to create the property. > > Thierry > Yes.. I can 1. modify this to return the property (which would be NULL if create fails) and have a NULL check at the calling end or 2. have a NULL check for the property at the calling end keeping the existing implementation or 3. return a non-zero value in case of failure. Please let me know your inputs on this.. - Vandana ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Avoid double mutex lock applying pipe A quirk during sanitize_crtc()
On Tue, Jun 10, 2014 at 09:02:07AM +0200, Daniel Vetter wrote: > On Mon, Jun 09, 2014 at 11:30:26AM +0300, Ville Syrjälä wrote: > > On Mon, Jun 09, 2014 at 07:47:10AM +0100, Chris Wilson wrote: > > > Thomas found that his machine would deadlock reloading the i915.ko > > > module after resume. He identified that this was caused by the > > > reacquisition of the connection mutex inside intel_enable_pipe_a() > > > during the CRTC sanitization routine. This will only affect machines > > > that quirk PIPE A, i.e. the original 830m chipsets. > > > > > > This patch move the locking into a wrapper function so that > > > intel_enable_pipe_a() can bypass the locking knowing that it already > > > holds the correct locks. > > > > It can still try to grab crtc->mutex twice. Looks like Danial undid my > > fix to not take all the modeset locks around > > intel_modeset_setup_hw_state(). > > Hm, I didn't find anything in git logs and we've had places where we used > modeset_lock_all since a long time. And I don't see how Chris' patch > wouldn't address this here. Can you please explain? I added the modeset_all locking here: commit 027476642811f8559cbe00ef6cc54db230e48a20 Author: Ville Syrjälä Date: Mon Dec 2 11:08:06 2013 +0200 drm/i915: Take modeset locks around intel_modeset_setup_hw_state() and then had to reduce it to just the mode_config.mutex precisely due to the pipe A quirk here: commit 7ad228b11ec26a820291c9f5a1168d6176580dc1 Author: Ville Syrjälä Date: Tue Jan 7 16:15:36 2014 +0200 drm/i915: Don't grab crtc mutexes in intel_modeset_gem_init() -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Fixed the review issues for pm_rc6_residency IGT case
Hi Wendy, I think it's better to squash this into the original patch so I've reverted your patch from i-g-t for now. Also chatted with Ben on irc and he's ok with that. More comments after I've read your test more carefully: - Please don't use abort() or exit() anywhere in your test. Use the igt macros and functions instead so that the test result handling is done properly. We have rather good igt documentation now, I suggest http://people.freedesktop.org/~danvet/igt/intel-gpu-tools-i-g-t-core.html as a starting point. - The new subtest doesn't really test anything new, but only adds more informational output afaics. I think it's better to just merge that into the existing basic subtest for rc6 residency. You could also use igt_debug() to print optional diagnostical information. See the above documentation for how to use it. - When we convert an existing simple testcase into a test with subtests we call the basic subtest "basic". This way it's easier for QA to track regressions and test status across such reorganizations. Also for subtests there's no reason to repeat the overall testname so here you can drop the "rc6_residency" part since that's already included in the "pm_rc6_residency" name. On a quick glance your fixup here looks good. Please cc me on the next revision so that I can work together with you to polish this patch. Thanks, Daniel On Mon, Jun 09, 2014 at 04:36:47PM +0800, Wendy Wang wrote: > Why need add rc6_residency_counter subtest case: > RC6 feature support residency counter,from power consumption aspect, > the counter closer to 1,the better.If the counter is < 0.9, the residency > is not good and will impact power consumption value, if the counter is > 1, > sysfs file is inaccurate. > > Attach the test result message: > root@x-bdw05:/GFX/Test/Intel_gpu_tools/intel-gpu-tools/tests# > ./pm_rc6_residency > IGT-Version: 1.6-g9a70e29 (x86_64) (Linux: > 3.15.0-rc7_drm-intel-nightly_0a37b5_20140604+ x86_64) > Subtest rc6-residency-check: SUCCESS > This machine doesn't support rc6pp > This machine doesn't support rc6p > The residency counter : 0.987000 > This machine entry rc6 state. > Subtest rc6-residency-counter: SUCCESS > > root@x-bdw05:/GFX/Test/Intel_gpu_tools/intel-gpu-tools/tests# > ./pm_rc6_residency --run-subtest rc6-residency-counter > IGT-Version: 1.6-g9a70e29 (x86_64) (Linux: > 3.15.0-rc7_drm-intel-nightly_0a37b5_20140604+ x86_64) > This machine doesn't support rc6pp > This machine doesn't support rc6p > The residency counter : 0.987000 > This machine entry rc6 state. > Subtest rc6-residency-counter: SUCCESS > > root@x-bdw05:/GFX/Test/Intel_gpu_tools/intel-gpu-tools/tests# > ./pm_rc6_residency --run-subtest rc6-residency-check > IGT-Version: 1.6-g9a70e29 (x86_64) (Linux: > 3.15.0-rc7_drm-intel-nightly_0a37b5_20140604+ x86_64) > Subtest rc6-residency-check: SUCCESS > > root@x-bdw05:/GFX/Test/Intel_gpu_tools/intel-gpu-tools/tests# > ./pm_rc6_residency --list > rc6-residency-check > rc6-residency-counter > > Run as non-root > [haha@x-pk home]$ ./pm_rc6_residency > IGT-Version: 1.6-g18d2130 (x86_64) (Linux: > 3.13.0-rc3_drm-intel-nightly_639e4d_20131210+ x86_64) > No intel gpu found > Subtest rc6-residency-check: SKIP > Subtest rc6-residency-counter: SKIP > > Run on non-intel platform > [root@x-pk5 home]# ./pm_rc6_residency > IGT-Version: 1.6-g18d2130 (x86_64) (Linux: > 3.13.0-rc3_drm-intel-nightly_639e4d_20131210+ x86_64) > Test requirement not met in function read_rc6_residency, file > pm_rc6_residency.c:77: > Last errno: 2, No such file or directory > Test requirement: (!(file)) > Subtest rc6-residency-check: SKIP > Subtest rc6-residency-counter: SKIP > > Signed-off-by: Wendy Wang > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > index e4c23b3..214c055 100644 > --- a/tests/Makefile.sources > +++ b/tests/Makefile.sources > @@ -72,6 +72,7 @@ TESTS_progs_M = \ > pm_lpsp \ > pm_rpm \ > pm_rps \ > + pm_rc6_residency \ > prime_self_import \ > template \ > $(NULL) > @@ -138,7 +139,6 @@ TESTS_progs = \ > kms_sink_crc_basic \ > kms_fence_pin_leak \ > pm_psr \ > - pm_rc6_residency \ > prime_udl \ > $(NULL) > > diff --git a/tests/pm_rc6_residency.c b/tests/pm_rc6_residency.c > index 550e3ad..d364369 100644 > --- a/tests/pm_rc6_residency.c > +++ b/tests/pm_rc6_residency.c > @@ -38,7 +38,6 @@ > #define SLEEP_DURATION 3000 // in milliseconds > #define RC6_FUDGE 900 // in milliseconds > > - > static unsigned int readit(const char *path) > { > unsigned int ret; > @@ -60,6 +59,7 @@ static unsigned int readit(const char *path) > > static void read_rc6_residency( int value[], const char > *name_of_rc6_residency[]) > { > + unsigned int i; > const int device = drm_get_card(); > char *path ; > int ret; > @@ -72,32 +72,33 @@ static void read_rc6_residency( int value[], const char > *name_of_rc6_residency[] > ret
Re: [Intel-gfx] [PATCH] drm/i915: Avoid double mutex lock applying pipe A quirk during sanitize_crtc()
On Tue, Jun 10, 2014 at 11:53:51AM +0300, Ville Syrjälä wrote: > On Tue, Jun 10, 2014 at 09:02:07AM +0200, Daniel Vetter wrote: > > On Mon, Jun 09, 2014 at 11:30:26AM +0300, Ville Syrjälä wrote: > > > On Mon, Jun 09, 2014 at 07:47:10AM +0100, Chris Wilson wrote: > > > > Thomas found that his machine would deadlock reloading the i915.ko > > > > module after resume. He identified that this was caused by the > > > > reacquisition of the connection mutex inside intel_enable_pipe_a() > > > > during the CRTC sanitization routine. This will only affect machines > > > > that quirk PIPE A, i.e. the original 830m chipsets. > > > > > > > > This patch move the locking into a wrapper function so that > > > > intel_enable_pipe_a() can bypass the locking knowing that it already > > > > holds the correct locks. > > > > > > It can still try to grab crtc->mutex twice. Looks like Danial undid my > > > fix to not take all the modeset locks around > > > intel_modeset_setup_hw_state(). > > > > Hm, I didn't find anything in git logs and we've had places where we used > > modeset_lock_all since a long time. And I don't see how Chris' patch > > wouldn't address this here. Can you please explain? > > I added the modeset_all locking here: > commit 027476642811f8559cbe00ef6cc54db230e48a20 > Author: Ville Syrjälä > Date: Mon Dec 2 11:08:06 2013 +0200 > > drm/i915: Take modeset locks around intel_modeset_setup_hw_state() > > and then had to reduce it to just the mode_config.mutex precisely due to > the pipe A quirk here: > commit 7ad228b11ec26a820291c9f5a1168d6176580dc1 > Author: Ville Syrjälä > Date: Tue Jan 7 16:15:36 2014 +0200 > > drm/i915: Don't grab crtc mutexes in intel_modeset_gem_init() Hm, indeed missed this since we have a bunch of other places that still do the full modeset_lock_all, but probably not possible to hit the pipe A quirk there. The ww mutex code makes this a bit more annoying so I guess Chris' patch with having a lock-less pipe A quirk is the way to go. But that one needs to be updated for latest drm-next/-nightly. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: Check for a stalled page flip after each vblank
Long ago, back in the racy haydays of 915gm interrupt handling, page flips would occasionally go astray and leave the hardware stuck, and the display not updating. This annoyed people who relied on their systems being able to display continuously updating information 24/7, and so some code to detect when the driver missed the page flip completion signal was added. Until recently, it was presumed that the interrupt handling was now flawless, but once again Simon Farnsworth has found a system whose display will stall. Reinstate the pageflip stall detection, which works by checking to see if the hardware has been updated to the new framebuffer address following each vblank. If the hardware is scanning out from the new framebuffer, but we still think the flip is pending, then we kick our driver into submision. This is a continuation of the effort started with commit 4e5359cd053bfb7d8dabe4a63624a5726848ffbc Author: Simon Farnsworth Date: Wed Sep 1 17:47:52 2010 +0100 drm/i915: Avoid pageflipping freeze when we miss the flip prepare interrupt This now includes a belt-and-braces approach to make sure the driver (or the hardware) doesn't miss an interrupt and cause us to stop updating the display should the unthinkable happen and the pageflip fail - i.e. that the user is able to continue submitting flips. v2: Cleanup, refactor, and rename Reported-by: Simon Farnsworth Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75502 Signed-off-by: Chris Wilson Cc: Daniel Vetter Cc: Ville Syrjälä --- drivers/gpu/drm/i915/i915_debugfs.c | 23 +++--- drivers/gpu/drm/i915/i915_irq.c | 85 +++--- drivers/gpu/drm/i915/intel_display.c | 130 -- drivers/gpu/drm/i915/intel_drv.h |2 + 4 files changed, 147 insertions(+), 93 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index e8ea1ef..a5f0a26 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -581,6 +581,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) { struct drm_info_node *node = m->private; struct drm_device *dev = node->minor->dev; + struct drm_i915_private *dev_priv = dev->dev_private; unsigned long flags; struct intel_crtc *crtc; @@ -595,6 +596,8 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) seq_printf(m, "No flip due on pipe %c (plane %c)\n", pipe, plane); } else { + u32 addr; + if (atomic_read(&work->pending) < INTEL_FLIP_COMPLETE) { seq_printf(m, "Flip queued on pipe %c (plane %c)\n", pipe, plane); @@ -602,23 +605,23 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) seq_printf(m, "Flip pending (waiting for vsync) on pipe %c (plane %c)\n", pipe, plane); } + seq_printf(m, "Flip queued on frame %d, now %d\n", + work->sbc, atomic_read(&dev->vblank[crtc->pipe].count)); if (work->enable_stall_check) seq_puts(m, "Stall check enabled, "); else seq_puts(m, "Stall check waiting for page flip ioctl, "); seq_printf(m, "%d prepares\n", atomic_read(&work->pending)); - if (work->old_fb_obj) { - struct drm_i915_gem_object *obj = work->old_fb_obj; - if (obj) - seq_printf(m, "Old framebuffer gtt_offset 0x%08lx\n", - i915_gem_obj_ggtt_offset(obj)); - } + if (INTEL_INFO(dev)->gen >= 4) + addr = I915_HI_DISPBASE(I915_READ(DSPSURF(crtc->plane))); + else + addr = I915_READ(DSPADDR(crtc->plane)); + seq_printf(m, "Current scanout address 0x%08x\n", addr); + if (work->pending_flip_obj) { - struct drm_i915_gem_object *obj = work->pending_flip_obj; - if (obj) - seq_printf(m, "New framebuffer gtt_offset 0x%08lx\n", - i915_gem_obj_ggtt_offset(obj)); + seq_printf(m, "New framebuffer address 0x%08lx\n", (long)work->gtt_offset); + seq_printf(m, "MMIO update completed? %d\n", addr == work->gtt_offset); } } spin_unlock_irqrestore(&de
[Intel-gfx] [PATCH 2/3] drm/i915: Decouple the stuck pageflip on modeset
If we successfully confuse the hardware, and cause it to drop a queued pageflip, we wait for 60s and issue a warning before continuing on with the modeset. However, this leaves the pending pageflip still stuck indefinitely. Pretend to userspace that it does complete, and let us start afresh following the modeset. v2: Rebase after refactor Signed-off-by: Chris Wilson Cc: Daniel Vetter Cc: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b87b4ce..9ecc6bf 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3363,9 +3363,19 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc) WARN_ON(waitqueue_active(&dev_priv->pending_flip_queue)); - WARN_ON(wait_event_timeout(dev_priv->pending_flip_queue, - !intel_crtc_has_pending_flip(crtc), - 60*HZ) == 0); + if (WARN_ON(wait_event_timeout(dev_priv->pending_flip_queue, + !intel_crtc_has_pending_flip(crtc), + 60*HZ) == 0)) { + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + unsigned long flags; + + spin_lock_irqsave(&dev->event_lock, flags); + if (intel_crtc->unpin_work) { + WARN_ONCE(1, "Removing stuck page flip\n"); + page_flip_completed(intel_crtc); + } + spin_unlock_irqrestore(&dev->event_lock, flags); + } mutex_lock(&dev->struct_mutex); intel_finish_fb(crtc->primary->fb); -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Boost GPU frequency if we detect outstanding pageflips
If we hit a vblank and see that have a pageflip queue but not yet processed, ensure that the GPU is running at maximum in order to clear the backlog. Pageflips are only queued for the following vblank, if we miss it, there will be a visible stutter. Boosting the GPU frequency doesn't prevent us from missing the target vblank, but it should help the subsequent frames hitting theirs. v2: Reorder vblank vs flip-complete so that we only check for a missed flip after processing the completion events, and avoid spurious boosts. v3: Rename missed_vblank Signed-off-by: Chris Wilson Cc: Daniel Vetter Cc: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.h |1 + drivers/gpu/drm/i915/intel_display.c |6 ++ drivers/gpu/drm/i915/intel_drv.h |1 + drivers/gpu/drm/i915/intel_pm.c | 15 +++ 4 files changed, 23 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 10dd80a..33ed0c6 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -910,6 +910,7 @@ struct intel_gen6_power_mgmt { bool enabled; struct delayed_work delayed_resume_work; + struct work_struct boost_work; /* * Protects RPS/RC6 register access and PCU communication. diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9ecc6bf..aeb58fa 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9339,6 +9339,7 @@ void intel_check_page_flip(struct drm_device *dev, int pipe) struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); unsigned long flags; + bool missed_vblank; if (crtc == NULL) return; @@ -9349,7 +9350,12 @@ void intel_check_page_flip(struct drm_device *dev, int pipe) intel_crtc->unpin_work->sbc, crtc_sbc(intel_crtc)); page_flip_completed(intel_crtc); } + missed_vblank = (intel_crtc->unpin_work != NULL && +crtc_sbc(intel_crtc) - intel_crtc->unpin_work->sbc > 1); spin_unlock_irqrestore(&dev->event_lock, flags); + + if (missed_vblank) + intel_queue_rps_boost(dev); } static int intel_crtc_page_flip(struct drm_crtc *crtc, diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index ac902ad..75fba0d 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -966,6 +966,7 @@ void ironlake_teardown_rc6(struct drm_device *dev); void gen6_update_ring_freq(struct drm_device *dev); void gen6_rps_idle(struct drm_i915_private *dev_priv); void gen6_rps_boost(struct drm_i915_private *dev_priv); +void intel_queue_rps_boost(struct drm_device *dev); void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv); void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv); void intel_runtime_pm_get(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index b06f896..ab760e5 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -6722,6 +6722,19 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val) return DIV_ROUND_CLOSEST(4 * mul * val, dev_priv->mem_freq) + 0xbd - 6; } +static void __intel_rps_boost_work(struct work_struct *work) +{ + gen6_rps_boost(container_of(work, struct drm_i915_private, rps.boost_work)); +} + +void intel_queue_rps_boost(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + + if (INTEL_INFO(dev)->gen >= 6) + queue_work(dev_priv->wq, &dev_priv->rps.boost_work); +} + void intel_pm_setup(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -6730,6 +6743,8 @@ void intel_pm_setup(struct drm_device *dev) INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work, intel_gen6_powersave_work); + INIT_WORK(&dev_priv->rps.boost_work, + __intel_rps_boost_work); dev_priv->pm.suspended = false; dev_priv->pm.irqs_disabled = false; -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Simplify processing of the golden render context state
Rewrite i915_gem_render_state.c for the purposes of clarity and compactness, in the process we can eliminate some dodgy math that did not handle 64bit addresses correctly. Signed-off-by: Chris Wilson Cc: Ville Syrjälä Cc: Damien Lespiau Cc: Mika Kuoppala --- drivers/gpu/drm/i915/i915_gem_render_state.c | 161 +++--- drivers/gpu/drm/i915/intel_renderstate.h | 2 - drivers/gpu/drm/i915/intel_renderstate_gen6.c | 1 + drivers/gpu/drm/i915/intel_renderstate_gen7.c | 1 + drivers/gpu/drm/i915/intel_renderstate_gen8.c | 1 + 5 files changed, 69 insertions(+), 97 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c index 3521f998a178..e60be3f552a6 100644 --- a/drivers/gpu/drm/i915/i915_gem_render_state.c +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c @@ -28,64 +28,13 @@ #include "i915_drv.h" #include "intel_renderstate.h" -struct i915_render_state { +struct render_state { + const struct intel_renderstate_rodata *rodata; struct drm_i915_gem_object *obj; - unsigned long ggtt_offset; - void *batch; - u32 size; - u32 len; + u64 ggtt_offset; + int gen; }; -static struct i915_render_state *render_state_alloc(struct drm_device *dev) -{ - struct i915_render_state *so; - struct page *page; - int ret; - - so = kzalloc(sizeof(*so), GFP_KERNEL); - if (!so) - return ERR_PTR(-ENOMEM); - - so->obj = i915_gem_alloc_object(dev, 4096); - if (so->obj == NULL) { - ret = -ENOMEM; - goto free; - } - so->size = 4096; - - ret = i915_gem_obj_ggtt_pin(so->obj, 4096, 0); - if (ret) - goto free_gem; - - BUG_ON(so->obj->pages->nents != 1); - page = sg_page(so->obj->pages->sgl); - - so->batch = kmap(page); - if (!so->batch) { - ret = -ENOMEM; - goto unpin; - } - - so->ggtt_offset = i915_gem_obj_ggtt_offset(so->obj); - - return so; -unpin: - i915_gem_object_ggtt_unpin(so->obj); -free_gem: - drm_gem_object_unreference(&so->obj->base); -free: - kfree(so); - return ERR_PTR(ret); -} - -static void render_state_free(struct i915_render_state *so) -{ - kunmap(so->batch); - i915_gem_object_ggtt_unpin(so->obj); - drm_gem_object_unreference(&so->obj->base); - kfree(so); -} - static const struct intel_renderstate_rodata * render_state_get_rodata(struct drm_device *dev, const int gen) { @@ -101,98 +50,120 @@ render_state_get_rodata(struct drm_device *dev, const int gen) return NULL; } -static int render_state_setup(const int gen, - const struct intel_renderstate_rodata *rodata, - struct i915_render_state *so) +static int render_state_init(struct render_state *so, struct drm_device *dev) { - const u64 goffset = i915_gem_obj_ggtt_offset(so->obj); - u32 reloc_index = 0; - u32 * const d = so->batch; - unsigned int i = 0; int ret; - if (!rodata || rodata->batch_items * 4 > so->size) + so->gen = INTEL_INFO(dev)->gen; + so->rodata = render_state_get_rodata(dev, so->gen); + if (so->rodata == NULL) + return 0; + + if (so->rodata->batch_items * 4 > 4096) return -EINVAL; + so->obj = i915_gem_alloc_object(dev, 4096); + if (so->obj == NULL) + return -ENOMEM; + + ret = i915_gem_obj_ggtt_pin(so->obj, 4096, 0); + if (ret) + goto free_gem; + + so->ggtt_offset = i915_gem_obj_ggtt_offset(so->obj); + return 0; + +free_gem: + drm_gem_object_unreference(&so->obj->base); + return ret; +} + +static int render_state_setup(struct render_state *so) +{ + const struct intel_renderstate_rodata *rodata = so->rodata; + unsigned int i = 0, reloc_index = 0; + struct page *page; + u32 *d; + int ret; + ret = i915_gem_object_set_to_cpu_domain(so->obj, true); if (ret) return ret; + page = sg_page(so->obj->pages->sgl); + d = kmap(page); + while (i < rodata->batch_items) { u32 s = rodata->batch[i]; - if (reloc_index < rodata->reloc_items && - i * 4 == rodata->reloc[reloc_index]) { - - s += goffset & 0x; - - /* We keep batch offsets max 32bit */ - if (gen >= 8) { + if (i * 4 == rodata->reloc[reloc_index]) { + u64 r = s + so->ggtt_offset; + s = lower_32_bits(r); + if (so->gen >= 8) { if (i + 1 >= rodata->batch_items || rodata->batch[i + 1] != 0) return -EINVAL; -
Re: [Intel-gfx] [PATCH] drm/i915: Only mark the ctx as initialised after a SET_CONTEXT operation
On Fri, May 30, 2014 at 02:16:30PM +0100, Chris Wilson wrote: > Fallout from > > commit 46470fc932ac8a0e8317a220b3f4ea4ed903338e > Author: Mika Kuoppala > Date: Wed May 21 19:01:06 2014 +0300 > > drm/i915: Add null state batch to active list > > undid the earlier fix of only marking the ctx as initialised after it is > saved by the hardware during a SET_CONTEXT operation. > > Signed-off-by: Chris Wilson > Cc: Ville Syrjälä > Cc: Damien Lespiau > Cc: Mika Kuoppala > Cc: Ben Widawsky Ping. Ben's comments do not impact upon the urgent need for this fix. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i95: Initialize active ring->pid to -1
Otherwise we print out spurious processes on unused rings in the error state. Signed-off-by: Chris Wilson Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/i915_gpu_error.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 87ec60e181a7..66cf41765bf9 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -888,6 +888,8 @@ static void i915_gem_record_rings(struct drm_device *dev, for (i = 0; i < I915_NUM_RINGS; i++) { struct intel_engine_cs *ring = &dev_priv->ring[i]; + error->ring[i].pid = -1; + if (ring->dev == NULL) continue; @@ -895,7 +897,6 @@ static void i915_gem_record_rings(struct drm_device *dev, i915_record_ring_state(dev, ring, &error->ring[i]); - error->ring[i].pid = -1; request = i915_gem_find_active_request(ring); if (request) { /* We need to copy these to an anonymous buffer -- 2.0.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Use remap_pfn_range() to prefault all PTE in a single pass
On an Ivybridge i7-3720qm with 1600MHz DDR3, with 32 fences, Upload rate for 2 linear surfaces: 8134MiB/s -> 8154MiB/s Upload rate for 2 tiled surfaces: 8625MiB/s -> 8632MiB/s Upload rate for 4 linear surfaces: 8127MiB/s -> 8134MiB/s Upload rate for 4 tiled surfaces: 8602MiB/s -> 8629MiB/s Upload rate for 8 linear surfaces: 8124MiB/s -> 8137MiB/s Upload rate for 8 tiled surfaces: 8603MiB/s -> 8624MiB/s Upload rate for 16 linear surfaces: 8123MiB/s -> 8128MiB/s Upload rate for 16 tiled surfaces: 8606MiB/s -> 8618MiB/s Upload rate for 32 linear surfaces: 8121MiB/s -> 8128MiB/s Upload rate for 32 tiled surfaces: 8605MiB/s -> 8614MiB/s Upload rate for 64 linear surfaces: 8121MiB/s -> 8127MiB/s Upload rate for 64 tiled surfaces: 3017MiB/s -> 5127MiB/s Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 28 +--- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index e1f68f06c2ef..7128d389a6ff 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1709,21 +1709,19 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf) pfn >>= PAGE_SHIFT; if (!obj->fault_mappable) { - int i; - - for (i = 0; i < obj->base.size >> PAGE_SHIFT; i++) { - ret = vm_insert_pfn(vma, - (unsigned long)vma->vm_start + i * PAGE_SIZE, - pfn + i); - if (ret) - break; - } - - obj->fault_mappable = true; - } else - ret = vm_insert_pfn(vma, - (unsigned long)vmf->virtual_address, - pfn + page_offset); + ret = remap_pfn_range(vma, + vma->vm_start, + pfn, + obj->base.size, + vma->vm_page_prot); + obj->fault_mappable = ret == 0; + } else { + ret = remap_pfn_range(vma, + (unsigned long)vmf->virtual_address, + pfn + page_offset, + PAGE_SIZE, + vma->vm_page_prot); + } unpin: i915_gem_object_ggtt_unpin(obj); unlock: -- 2.0.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Prefault the entire object on first page fault
Inserting additional PTEs has no side-effect for us as the pfn are fixed for the entire time the object is resident in the global GTT. The downside is that we pay the entire cost of faulting the object upon the first hit, for which we in return receive the benefit of removing the per-page faulting overhead. On an Ivybridge i7-3720qm with 1600MHz DDR3, with 32 fences, Upload rate for 2 linear surfaces: 8127MiB/s -> 8134MiB/s Upload rate for 2 tiled surfaces: 8607MiB/s -> 8625MiB/s Upload rate for 4 linear surfaces: 8127MiB/s -> 8127MiB/s Upload rate for 4 tiled surfaces: 8611MiB/s -> 8602MiB/s Upload rate for 8 linear surfaces: 8114MiB/s -> 8124MiB/s Upload rate for 8 tiled surfaces: 8601MiB/s -> 8603MiB/s Upload rate for 16 linear surfaces: 8110MiB/s -> 8123MiB/s Upload rate for 16 tiled surfaces: 8595MiB/s -> 8606MiB/s Upload rate for 32 linear surfaces: 8104MiB/s -> 8121MiB/s Upload rate for 32 tiled surfaces: 8589MiB/s -> 8605MiB/s Upload rate for 64 linear surfaces: 8107MiB/s -> 8121MiB/s Upload rate for 64 tiled surfaces: 2013MiB/s -> 3017MiB/s Signed-off-by: Chris Wilson Cc: "Goel, Akash" --- drivers/gpu/drm/i915/i915_gem.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 3aaf7e01235e..e1f68f06c2ef 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1704,14 +1704,26 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf) if (ret) goto unpin; - obj->fault_mappable = true; - + /* Finally, remap it using the new GTT offset */ pfn = dev_priv->gtt.mappable_base + i915_gem_obj_ggtt_offset(obj); pfn >>= PAGE_SHIFT; - pfn += page_offset; - /* Finally, remap it using the new GTT offset */ - ret = vm_insert_pfn(vma, (unsigned long)vmf->virtual_address, pfn); + if (!obj->fault_mappable) { + int i; + + for (i = 0; i < obj->base.size >> PAGE_SHIFT; i++) { + ret = vm_insert_pfn(vma, + (unsigned long)vma->vm_start + i * PAGE_SIZE, + pfn + i); + if (ret) + break; + } + + obj->fault_mappable = true; + } else + ret = vm_insert_pfn(vma, + (unsigned long)vmf->virtual_address, + pfn + page_offset); unpin: i915_gem_object_ggtt_unpin(obj); unlock: -- 2.0.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/4] drm/crtc: Add property for aspect ratio
On Tue, Jun 10, 2014 at 02:00:37PM +0530, Vandana Kannan wrote: > On Jun-05-2014 2:58 PM, Thierry Reding wrote: > > On Thu, Jun 05, 2014 at 02:40:08PM +0530, Vandana Kannan wrote: > > [...] > >> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > > [...] > >> /** > >> + * drm_mode_create_aspect_ratio_property - create aspect ratio property > >> + * @dev: DRM device > >> + * > >> + * Called by a driver the first time it's needed, must be attached to > >> desired > >> + * connectors. > >> + */ > >> +int drm_mode_create_aspect_ratio_property(struct drm_device *dev) > >> +{ > >> + if (dev->mode_config.aspect_ratio_property) > >> + return 0; > >> + > >> + dev->mode_config.aspect_ratio_property = > >> + drm_property_create_enum(dev, 0, "aspect ratio", > >> + drm_aspect_ratio_enum_list, > >> + ARRAY_SIZE(drm_aspect_ratio_enum_list)); > >> + > >> + return 0; > > > > Sorry for not noticing this before: what if drm_propert_create_enum() > > fails? Should that return an error? This function will currently > > silently ignore failure to create the property. > > > > Thierry > > > Yes.. > I can > 1. modify this to return the property (which would be NULL if create > fails) and have a NULL check at the calling end or > 2. have a NULL check for the property at the calling end keeping the > existing implementation or > 3. return a non-zero value in case of failure. I prefer 3. -ENOMEM sounds like a good candidate here. Thierry pgpMix1FdxLAG.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Check for a stalled page flip after each vblank
On Tue, Jun 10, 2014 at 11:04:00AM +0100, Chris Wilson wrote: > Long ago, back in the racy haydays of 915gm interrupt handling, page > flips would occasionally go astray and leave the hardware stuck, and the > display not updating. This annoyed people who relied on their systems > being able to display continuously updating information 24/7, and so > some code to detect when the driver missed the page flip completion > signal was added. Until recently, it was presumed that the interrupt > handling was now flawless, but once again Simon Farnsworth has found a > system whose display will stall. Reinstate the pageflip stall detection, > which works by checking to see if the hardware has been updated to the > new framebuffer address following each vblank. If the hardware is > scanning out from the new framebuffer, but we still think the flip is > pending, then we kick our driver into submision. > > This is a continuation of the effort started with > commit 4e5359cd053bfb7d8dabe4a63624a5726848ffbc > Author: Simon Farnsworth > Date: Wed Sep 1 17:47:52 2010 +0100 > > drm/i915: Avoid pageflipping freeze when we miss the flip prepare > interrupt > > This now includes a belt-and-braces approach to make sure the driver > (or the hardware) doesn't miss an interrupt and cause us to stop > updating the display should the unthinkable happen and the pageflip fail - > i.e. > that the user is able to continue submitting flips. > > v2: Cleanup, refactor, and rename > > Reported-by: Simon Farnsworth > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75502 > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > Cc: Ville Syrjälä > --- > drivers/gpu/drm/i915/i915_debugfs.c | 23 +++--- > drivers/gpu/drm/i915/i915_irq.c | 85 +++--- > drivers/gpu/drm/i915/intel_display.c | 130 > -- > drivers/gpu/drm/i915/intel_drv.h |2 + > 4 files changed, 147 insertions(+), 93 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index e8ea1ef..a5f0a26 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -581,6 +581,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, > void *data) > { > struct drm_info_node *node = m->private; > struct drm_device *dev = node->minor->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > unsigned long flags; > struct intel_crtc *crtc; > > @@ -595,6 +596,8 @@ static int i915_gem_pageflip_info(struct seq_file *m, > void *data) > seq_printf(m, "No flip due on pipe %c (plane %c)\n", > pipe, plane); > } else { > + u32 addr; > + > if (atomic_read(&work->pending) < INTEL_FLIP_COMPLETE) { > seq_printf(m, "Flip queued on pipe %c (plane > %c)\n", > pipe, plane); > @@ -602,23 +605,23 @@ static int i915_gem_pageflip_info(struct seq_file *m, > void *data) > seq_printf(m, "Flip pending (waiting for vsync) > on pipe %c (plane %c)\n", > pipe, plane); > } > + seq_printf(m, "Flip queued on frame %d, now %d\n", > +work->sbc, > atomic_read(&dev->vblank[crtc->pipe].count)); > if (work->enable_stall_check) > seq_puts(m, "Stall check enabled, "); > else > seq_puts(m, "Stall check waiting for page flip > ioctl, "); > seq_printf(m, "%d prepares\n", > atomic_read(&work->pending)); > > - if (work->old_fb_obj) { > - struct drm_i915_gem_object *obj = > work->old_fb_obj; > - if (obj) > - seq_printf(m, "Old framebuffer > gtt_offset 0x%08lx\n", > - > i915_gem_obj_ggtt_offset(obj)); > - } > + if (INTEL_INFO(dev)->gen >= 4) > + addr = > I915_HI_DISPBASE(I915_READ(DSPSURF(crtc->plane))); > + else > + addr = I915_READ(DSPADDR(crtc->plane)); > + seq_printf(m, "Current scanout address 0x%08x\n", addr); > + > if (work->pending_flip_obj) { > - struct drm_i915_gem_object *obj = > work->pending_flip_obj; > - if (obj) > - seq_printf(m, "New framebuffer > gtt_offset 0x%08lx\n", > - > i915_gem_obj_ggtt_offset(obj)); > + seq_printf(m, "New framebuffer address > 0x%08lx\n", (long)work
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Decouple the stuck pageflip on modeset
On Tue, Jun 10, 2014 at 11:04:01AM +0100, Chris Wilson wrote: > If we successfully confuse the hardware, and cause it to drop a queued > pageflip, we wait for 60s and issue a warning before continuing on with > the modeset. However, this leaves the pending pageflip still stuck > indefinitely. Pretend to userspace that it does complete, and let us > start afresh following the modeset. > > v2: Rebase after refactor > > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > Cc: Ville Syrjälä Reviewed-by: Ville Syrjälä still stands. > --- > drivers/gpu/drm/i915/intel_display.c | 16 +--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index b87b4ce..9ecc6bf 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3363,9 +3363,19 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc > *crtc) > > WARN_ON(waitqueue_active(&dev_priv->pending_flip_queue)); > > - WARN_ON(wait_event_timeout(dev_priv->pending_flip_queue, > -!intel_crtc_has_pending_flip(crtc), > -60*HZ) == 0); > + if (WARN_ON(wait_event_timeout(dev_priv->pending_flip_queue, > +!intel_crtc_has_pending_flip(crtc), > +60*HZ) == 0)) { > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + unsigned long flags; > + > + spin_lock_irqsave(&dev->event_lock, flags); > + if (intel_crtc->unpin_work) { > + WARN_ONCE(1, "Removing stuck page flip\n"); > + page_flip_completed(intel_crtc); > + } > + spin_unlock_irqrestore(&dev->event_lock, flags); > + } > > mutex_lock(&dev->struct_mutex); > intel_finish_fb(crtc->primary->fb); > -- > 1.7.9.5 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Check for a stalled page flip after each vblank
On Tue, Jun 10, 2014 at 02:25:50PM +0300, Ville Syrjälä wrote: > On Tue, Jun 10, 2014 at 11:04:00AM +0100, Chris Wilson wrote: > > +static inline int crtc_sbc(struct intel_crtc *crtc) > > +{ > > + return atomic_read(&crtc->base.dev->vblank[crtc->pipe].count); > > +} > > Still says 'sbc' which doesn't make sense to me. I just don't like the term msc. :-p crtc_vblank_counter()? > > + > > static inline void intel_mark_page_flip_active(struct intel_crtc > > *intel_crtc) > > { > > /* Ensure that the work item is consistent when activating it ... */ > > smp_wmb(); > > atomic_set(&intel_crtc->unpin_work->pending, INTEL_FLIP_PENDING); > > + intel_crtc->unpin_work->sbc = crtc_sbc(intel_crtc); > > /* and that it is marked active as soon as the irq could fire. */ > > smp_wmb(); > > } > > @@ -9265,6 +9279,69 @@ static int intel_default_queue_flip(struct > > drm_device *dev, > > return -ENODEV; > > } > > > > +static bool i915_gem_object_is_idle(struct drm_i915_gem_object *obj, > > + bool readonly) > > +{ > > + struct intel_engine_cs *ring = obj->ring; > > + u32 seqno = readonly ? obj->last_write_seqno : obj->last_read_seqno; > > + > > + if (ring == NULL || seqno == 0) > > + return true; > > + > > + return i915_seqno_passed(ring->get_seqno(ring, true), seqno); > > +} > > + > > +static bool __intel_pageflip_stall_check(struct drm_device *dev, > > +struct drm_crtc *crtc) > > +{ > > + struct drm_i915_private *dev_priv = dev->dev_private; > > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > + struct intel_unpin_work *work = intel_crtc->unpin_work; > > + u32 addr; > > + > > + if (atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE) > > + return true; > > + > > + if (!work->enable_stall_check) > > + return false; > > + > > + if ((crtc_sbc(intel_crtc) - intel_crtc->unpin_work->sbc) < 3) > > + return false; > > + > > + if (!i915_gem_object_is_idle(work->pending_flip_obj, true)) > > + return false; > > I take it this is done to mitigate my premature flip completion > concerns? Should work for the common case I suppose. Although if > someone does something like this "write,read(s),flip" it could > still complete the flip too soon. Waiting for last_read_seqno would > avoid that. It actually predated your concerns. I considered the person who continues to write to the pending fb and decided that I didn't care too much for them. What I actually wanted to do was to capture the vblank counter for when the object was idle and then start watching for a missed flip. Indeed, tracking when we believe the flip was queued would be better again. > And double checking the hardware flip counter should avoid this > problem entirely. We already have it sampled in unpin_work->flip_count > for the mmio vs. cs flip race so it should be easy to check it here as > well. I suppose having both the flip counter and seqno checks should > provide the best protection for all gens. That was half the reason for waiting for that series to land first. Just I never adapted to the framecounter code. > As far as the seqno check goes, I was wondering if we should sample > the seqno when submitting the flip? I'm slightly worried how this will > work if userspace submitted a flip and already managed to pile more > rendering on top. This code would then wait for the seqno for those > later rendering operations. Right that is how mmio flips avoid this issue. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Boost GPU frequency if we detect outstanding pageflips
On Tue, Jun 10, 2014 at 11:04:02AM +0100, Chris Wilson wrote: > If we hit a vblank and see that have a pageflip queue but not yet > processed, ensure that the GPU is running at maximum in order to clear > the backlog. Pageflips are only queued for the following vblank, if we > miss it, there will be a visible stutter. Boosting the GPU frequency > doesn't prevent us from missing the target vblank, but it should help > the subsequent frames hitting theirs. > > v2: Reorder vblank vs flip-complete so that we only check for a missed > flip after processing the completion events, and avoid spurious boosts. > > v3: Rename missed_vblank > > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > Cc: Ville Syrjälä > --- > drivers/gpu/drm/i915/i915_drv.h |1 + > drivers/gpu/drm/i915/intel_display.c |6 ++ > drivers/gpu/drm/i915/intel_drv.h |1 + > drivers/gpu/drm/i915/intel_pm.c | 15 +++ > 4 files changed, 23 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 10dd80a..33ed0c6 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -910,6 +910,7 @@ struct intel_gen6_power_mgmt { > > bool enabled; > struct delayed_work delayed_resume_work; > + struct work_struct boost_work; > > /* >* Protects RPS/RC6 register access and PCU communication. > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 9ecc6bf..aeb58fa 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9339,6 +9339,7 @@ void intel_check_page_flip(struct drm_device *dev, int > pipe) > struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > unsigned long flags; > + bool missed_vblank; > > if (crtc == NULL) > return; > @@ -9349,7 +9350,12 @@ void intel_check_page_flip(struct drm_device *dev, int > pipe) >intel_crtc->unpin_work->sbc, crtc_sbc(intel_crtc)); > page_flip_completed(intel_crtc); > } > + missed_vblank = (intel_crtc->unpin_work != NULL && > + crtc_sbc(intel_crtc) - intel_crtc->unpin_work->sbc > > 1); So this will boost when we notice that we've crossed into the second vblank since the flip was queued. I was wondering if we should try to boost already after the first vblank. But doing that is a bit more problematic since we process the "flip done" interrupt after the vblank interrupt. So simply changing the check to >0 would end up boosting every time even if the flip already happened and we're just about to complete it when we get to processing the "flip done" interrupt. > spin_unlock_irqrestore(&dev->event_lock, flags); > + > + if (missed_vblank) > + intel_queue_rps_boost(dev); > } > > static int intel_crtc_page_flip(struct drm_crtc *crtc, > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index ac902ad..75fba0d 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -966,6 +966,7 @@ void ironlake_teardown_rc6(struct drm_device *dev); > void gen6_update_ring_freq(struct drm_device *dev); > void gen6_rps_idle(struct drm_i915_private *dev_priv); > void gen6_rps_boost(struct drm_i915_private *dev_priv); > +void intel_queue_rps_boost(struct drm_device *dev); > void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv); > void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv); > void intel_runtime_pm_get(struct drm_i915_private *dev_priv); > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index b06f896..ab760e5 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -6722,6 +6722,19 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, > int val) > return DIV_ROUND_CLOSEST(4 * mul * val, dev_priv->mem_freq) + 0xbd - 6; > } > > +static void __intel_rps_boost_work(struct work_struct *work) > +{ > + gen6_rps_boost(container_of(work, struct drm_i915_private, > rps.boost_work)); > +} > + > +void intel_queue_rps_boost(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = to_i915(dev); > + > + if (INTEL_INFO(dev)->gen >= 6) > + queue_work(dev_priv->wq, &dev_priv->rps.boost_work); > +} > + > void intel_pm_setup(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -6730,6 +6743,8 @@ void intel_pm_setup(struct drm_device *dev) > > INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work, > intel_gen6_powersave_work); > + INIT_WORK(&dev_priv->rps.boost_work, > + __intel_rps_boost_work); > > dev_priv->pm.suspended = false; > dev_priv->pm.irqs_disabled = false; > -- > 1.7.9.5
[Intel-gfx] [PATCH maintainer-tools] frob-patch-rank: A little script to batch renaming patch files
The "usage" text should explain it all. I found, in my quilt series handling endeavours, that I wanted to be able to shift the prefix numbers of a patch series. Signed-off-by: Damien Lespiau --- frob-patch-rank | 47 +++ 1 file changed, 47 insertions(+) create mode 100755 frob-patch-rank diff --git a/frob-patch-rank b/frob-patch-rank new file mode 100755 index 000..4be42e5 --- /dev/null +++ b/frob-patch-rank @@ -0,0 +1,47 @@ +#!/bin/sh +set -e + +script=$(basename $0) +[ $# -ge 3 ] || { + echo "Usage: $script start end expr" + echo + echo " Frob patches." + echo + echo " This tiny script renames \"git format-patch\" patches by executing 'expr'" + echo " on the number that prefix the patch file, but only if the patch file name " + echo " starts with a number in ['start','end']." + echo + echo "Examples:" + echo " $ ls *patch" + echo " 0008-Super-patch.patch" + echo " 0009-Mega-patch.patch" + echo " $ $script 8 9 -7" + echo " $ ls *patch" + echo " 0001-Super-patch.patch" + echo " 0002-Mega-patch.patch" + echo + echo "Examples:" + echo " $ ls *patch" + echo " 0117-Super-patch.patch" + echo " 0118-Mega-patch.patch" + echo " $ $script 8 9 +900 -17" + echo " $ ls *patch" + echo " 1000-Super-patch.patch" + echo " 1001-Mega-patch.patch" + exit 1 +} + +start=$1 +end=$2 +shift 2 +op=$* + +for i in $(seq $start $end); do + prefix=$(printf "%04d" $i) + for f in $(ls $prefix-*.patch); do + base=${f#$prefix-} + ((n=$i $op)) + new_prefix=$(printf "%04d" $n) + mv $prefix-$base $new_prefix-$base + done +done -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Check for a stalled page flip after each vblank
On Tue, Jun 10, 2014 at 12:33:48PM +0100, Chris Wilson wrote: > On Tue, Jun 10, 2014 at 02:25:50PM +0300, Ville Syrjälä wrote: > > On Tue, Jun 10, 2014 at 11:04:00AM +0100, Chris Wilson wrote: > > > +static inline int crtc_sbc(struct intel_crtc *crtc) > > > +{ > > > + return atomic_read(&crtc->base.dev->vblank[crtc->pipe].count); > > > +} > > > > Still says 'sbc' which doesn't make sense to me. > > I just don't like the term msc. :-p > crtc_vblank_counter()? Maybe stick it into drmP.h and call it drm_crtc_vblank_counter() or something? Hopefully people won't confuse it with the hardware counter. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 08/16] drm/i915: Split watermark programming into pre and post steps
On Mon, 09 Jun 2014, Ville Syrjälä wrote: > On Wed, Jun 04, 2014 at 06:22:13PM +0200, Daniel Vetter wrote: >> On Tue, Jun 03, 2014 at 05:51:01PM -0300, Paulo Zanoni wrote: >> > 2014-05-22 11:48 GMT-03:00 : >> > > From: Ville Syrjälä >> > > >> > > We need to perform watermark programming before and after changing the >> > > plane configuration. Add two new vfuncs to do that. The pre phase is >> > > supposed to switch over to the intermediate watermarks which are >> > > computed so that they can deal with both the old and new plane >> > > configurations. The post phase will arm the vblank based update >> > > systems to switch over to the optimal target watermarks after the >> > > plane configuration has for sure changed. >> > > >> > > v2: Pass around intel_crtc and s/intel_crtc/crtc/ >> > > >> > > Signed-off-by: Ville Syrjälä >> > > --- >> > > drivers/gpu/drm/i915/i915_drv.h | 5 +++ >> > > drivers/gpu/drm/i915/intel_drv.h | 11 + >> > > drivers/gpu/drm/i915/intel_pm.c | 88 >> > > >> > > 3 files changed, 104 insertions(+) >> > > >> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h >> > > b/drivers/gpu/drm/i915/i915_drv.h >> > > index c90d5ac..d4f8ae8 100644 >> > > --- a/drivers/gpu/drm/i915/i915_drv.h >> > > +++ b/drivers/gpu/drm/i915/i915_drv.h >> > > @@ -407,6 +407,7 @@ struct intel_plane_config; >> > > struct intel_crtc; >> > > struct intel_limit; >> > > struct dpll; >> > > +struct intel_crtc_wm_config; >> > > >> > > struct drm_i915_display_funcs { >> > > bool (*fbc_enabled)(struct drm_device *dev); >> > > @@ -437,6 +438,10 @@ struct drm_i915_display_funcs { >> > > struct drm_crtc *crtc, >> > > uint32_t sprite_width, int pixel_size, >> > > bool enable, bool scaled); >> > > + void (*program_wm_pre)(struct intel_crtc *crtc, >> > > + const struct intel_crtc_wm_config >> > > *config); >> > > + void (*program_wm_post)(struct intel_crtc *crtc, >> > > + const struct intel_crtc_wm_config >> > > *config); >> > > void (*modeset_global_resources)(struct drm_device *dev); >> > > /* Returns the active state of the crtc, and if the crtc is >> > > active, >> > > * fills out the pipe-config with the hw state. */ >> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h >> > > b/drivers/gpu/drm/i915/intel_drv.h >> > > index 72f01b1..4b59be3 100644 >> > > --- a/drivers/gpu/drm/i915/intel_drv.h >> > > +++ b/drivers/gpu/drm/i915/intel_drv.h >> > > @@ -358,6 +358,13 @@ struct intel_pipe_wm { >> > > bool sprites_scaled; >> > > }; >> > > >> > > +struct intel_crtc_wm_config { >> > > + /* target watermarks for the pipe */ >> > > + struct intel_pipe_wm target; >> > > + /* intermediate watermarks for pending/active->target transition >> > > */ >> > > + struct intel_pipe_wm intm; >> > >> > It seems you always prefer shorter names such as "intm", and I usually >> > prefer the longer ones like "intermediate". Looks like this is a >> > common topic for my bikesheddings on your patches. When I read "intm" >> > my brain parses it as "Int M" and then aborts execution =P >> >> I agree with Paulo here. Some other name suggestion (since intermediate is >> so long): transition/transit, merged, pending, ... > > The two good names I could think of "intermediate" and "transitional" > are both really long. I agree that "intm" is a horrible shorthand, > but I couldn't come up with anything half sane and still reasonably > short. > > "merged" and "pending" already appear in the code with different > meaning, so I'd rather not confuse the matter by reusing those. > "transition" seems OK to me, though not that short either. > "transit" brings up wrong kinds of images in my brain so i don't > really like it. interim, transient, temporary, or simly just temp? BR, Jani. > >> -Daniel >> >> > >> > With or without that changed: Reviewed-by: Paulo Zanoni >> > >> > >> > > +}; >> > > + >> > > struct intel_crtc { >> > > struct drm_crtc base; >> > > enum pipe pipe; >> > > @@ -1001,6 +1008,10 @@ void intel_init_runtime_pm(struct >> > > drm_i915_private *dev_priv); >> > > void intel_fini_runtime_pm(struct drm_i915_private *dev_priv); >> > > void ilk_wm_get_hw_state(struct drm_device *dev); >> > > void ilk_update_pipe_wm(struct drm_device *dev, enum pipe pipe); >> > > +void intel_program_watermarks_pre(struct intel_crtc *crtc, >> > > + const struct intel_crtc_wm_config >> > > *config); >> > > +void intel_program_watermarks_post(struct intel_crtc *crtc, >> > > + const struct intel_crtc_wm_config >> > > *config); >> > > >> > > >> > > /* intel_sdvo.c */ >> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c >> > > b/drivers/gpu/drm/i915/intel_pm.c >> > > index 6fc6416..ccf920a 10064
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Boost GPU frequency if we detect outstanding pageflips
On Tue, Jun 10, 2014 at 02:41:20PM +0300, Ville Syrjälä wrote: > On Tue, Jun 10, 2014 at 11:04:02AM +0100, Chris Wilson wrote: > > If we hit a vblank and see that have a pageflip queue but not yet > > processed, ensure that the GPU is running at maximum in order to clear > > the backlog. Pageflips are only queued for the following vblank, if we > > miss it, there will be a visible stutter. Boosting the GPU frequency > > doesn't prevent us from missing the target vblank, but it should help > > the subsequent frames hitting theirs. > > > > v2: Reorder vblank vs flip-complete so that we only check for a missed > > flip after processing the completion events, and avoid spurious boosts. > > > > v3: Rename missed_vblank > > > > Signed-off-by: Chris Wilson > > Cc: Daniel Vetter > > Cc: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/i915_drv.h |1 + > > drivers/gpu/drm/i915/intel_display.c |6 ++ > > drivers/gpu/drm/i915/intel_drv.h |1 + > > drivers/gpu/drm/i915/intel_pm.c | 15 +++ > > 4 files changed, 23 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 10dd80a..33ed0c6 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -910,6 +910,7 @@ struct intel_gen6_power_mgmt { > > > > bool enabled; > > struct delayed_work delayed_resume_work; > > + struct work_struct boost_work; > > > > /* > > * Protects RPS/RC6 register access and PCU communication. > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 9ecc6bf..aeb58fa 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -9339,6 +9339,7 @@ void intel_check_page_flip(struct drm_device *dev, > > int pipe) > > struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > unsigned long flags; > > + bool missed_vblank; > > > > if (crtc == NULL) > > return; > > @@ -9349,7 +9350,12 @@ void intel_check_page_flip(struct drm_device *dev, > > int pipe) > > intel_crtc->unpin_work->sbc, crtc_sbc(intel_crtc)); > > page_flip_completed(intel_crtc); > > } > > + missed_vblank = (intel_crtc->unpin_work != NULL && > > +crtc_sbc(intel_crtc) - intel_crtc->unpin_work->sbc > > > 1); > > So this will boost when we notice that we've crossed into the second > vblank since the flip was queued. I was wondering if we should try to > boost already after the first vblank. But doing that is a bit more > problematic since we process the "flip done" interrupt after the > vblank interrupt. So simply changing the check to >0 would end up > boosting every time even if the flip already happened and we're just > about to complete it when we get to processing the "flip done" > interrupt. Exactly. It is a little too eager if we start boosting after one missed vblank as we process the vblank before the flip-complete. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Bisecting the heisenbugs (was Re: 3.15-rc: regression in suspend)
On Mon 2014-06-09 13:03:31, Jiri Kosina wrote: > On Mon, 9 Jun 2014, Pavel Machek wrote: > > > > > Strange. It seems 3.15 with the patch reverted only boots in 30% or so > > > > cases... And I've seen resume failure, too, so maybe I was just lucky > > > > that it worked for a while. > > > > > > git bisect really likes 25f397a429dfa43f22c278d0119a60 - you're about > > > the 5th report or so that claims this is the culprit but it's > > > something else. The above code is definitely not used in i915 so bogus > > > bisect result. > > > > Note I did not do the bisect, I only attempted revert and test. > > > > And did three boots of successful s2ram.. only to find out that it > > does not really fix s2ram, I was just lucky :-(. > > > > Unfortunately, this means my s2ram problem will be tricky/impossible > > to bisect :-(. > > Welcome to the situation I have been in for past several months. I attempted to do some analysis. It should be possible to bisect when tests are not reliable, but it will take time and it will be almost neccessary to have the bisection automated. How long does the testing take for you to get to 50% test reliability? It seems to be one minute here. Trivial strategy is to repeat each test to get to 99% test reliability. That should make the test about 2x longer. There are other strategies possible -- like selecting bisect points closer to the "bad" end, and tricky "lets compute probabilities for each point", that work well for some parameter settings. There is probably even better strategy possible... if you have an idea, you can try it below. Monte carlo simulation is attached. Bisector on reliable bug - 1024 versions bug with probability of 0 false success, monte carlo of 3 tries Assume compilation takes 6 minutes and test takes 1 minutes Average cost 71.0522 minutes Average tests 9.9979333 Bisector - 1024 versions bug with probability of 0.5 false success, monte carlo of 3 tries Assume compilation takes 6 minutes and test takes 1 minutes Average cost 143.39393 minutes Average tests 44.537467 Trisector - 1024 versions bug with probability of 0.5 false success, monte carlo of 3 tries Assume compilation takes 6 minutes and test takes 1 minutes Average cost 160.554 minutes Average tests 39.955267 Strange - 1024 versions bug with probability of 0.5 false success, monte carlo of 3000 tries Assume compilation takes 6 minutes and test takes 1 minutes Average cost 246.658 minutes Average tests 38.412 pavel@amd:~/WWW$ Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html #!/usr/bin/python import random import numpy class Devil: def init(m): m.versions = 1024 # Costs in minutes m.cost_compile = 6 m.cost_test = 1 # Penalty for wrongly identifying a commit. m.cost_failure = 1000 # 0. == nicely behaved bug which always triggers m.p_false_success = .5 m.verbose = 2 def init_run(m): m.broken = random.randint(0, m.versions-1) m.tests = 0 m.last_ver = -1 m.cost = 0 def test_failed(m, ver): m.tests += 1 if ver != m.last_ver: m.cost += m.cost_compile m.cost += m.cost_test m.last_ver = ver if m.verbose > 1: print " testing version ", ver, "(tests %d, cost %d)" % (m.tests, m.cost), if ver >= m.broken: if m.verbose > 1: print "(bad)", if random.random() > m.p_false_success: if m.verbose > 1: print "FAIL" return 1 if m.verbose > 1: print "pass" return 0 def evaluate(m): ver = m.run() if ver == m.broken: if m.verbose: print "success" else: if m.verbose: print "FAILURE" m.cost += m.cost_failure class Bisector(Devil): def init_run(m): Devil.init_run(m) m.good = 0 m.bad = m.versions-1 def run(m): while m.good+1 < m.bad: ver = (m.good + m.bad) / 2 p_bad = 1 failed = 0 while p_bad > .01: if m.test_failed(ver): m.bad = ver failed = 1 break p_bad *= m.p_false_success if not failed: m.good = ver return m.bad class Trisector(Devil): def init_run(m): Devil.init_run(m) m.good = 0 m.bad = m.versions-1 def run(m): while m.good+1 < m.bad: ver = (m.good*6 + m.bad*14) / 20 p_bad = 1 failed = 0 while p_bad > .01: if m.test_failed(ver): m.bad = v
Re: [Intel-gfx] [PATCH maintainer-tools] frob-patch-rank: A little script to batch renaming patch files
On Tue, 10 Jun 2014, Damien Lespiau wrote: > The "usage" text should explain it all. I found, in my quilt series > handling endeavours, that I wanted to be able to shift the prefix > numbers of a patch series. > > Signed-off-by: Damien Lespiau > --- > frob-patch-rank | 47 +++ > 1 file changed, 47 insertions(+) > create mode 100755 frob-patch-rank > > diff --git a/frob-patch-rank b/frob-patch-rank > new file mode 100755 > index 000..4be42e5 > --- /dev/null > +++ b/frob-patch-rank > @@ -0,0 +1,47 @@ > +#!/bin/sh > +set -e > + > +script=$(basename $0) > +[ $# -ge 3 ] || { > + echo "Usage: $script start end expr" > + echo > + echo " Frob patches." > + echo > + echo " This tiny script renames \"git format-patch\" patches by > executing 'expr'" > + echo " on the number that prefix the patch file, but only if the patch > file name " > + echo " starts with a number in ['start','end']." > + echo > + echo "Examples:" > + echo " $ ls *patch" > + echo " 0008-Super-patch.patch" > + echo " 0009-Mega-patch.patch" > + echo " $ $script 8 9 -7" > + echo " $ ls *patch" > + echo " 0001-Super-patch.patch" > + echo " 0002-Mega-patch.patch" > + echo > + echo "Examples:" > + echo " $ ls *patch" > + echo " 0117-Super-patch.patch" > + echo " 0118-Mega-patch.patch" > + echo " $ $script 8 9 +900 -17" I was scratching my head for a while with this example. It's wrong isn't it? The help could benefit from a "here" document. > + echo " $ ls *patch" > + echo " 1000-Super-patch.patch" > + echo " 1001-Mega-patch.patch" > + exit 1 > +} > + > +start=$1 > +end=$2 > +shift 2 > +op=$* > + > +for i in $(seq $start $end); do > + prefix=$(printf "%04d" $i) > + for f in $(ls $prefix-*.patch); do > + base=${f#$prefix-} > + ((n=$i $op)) > + new_prefix=$(printf "%04d" $n) > + mv $prefix-$base $new_prefix-$base mv -i just in case? BR, Jani. > + done > +done > -- > 1.8.3.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: do runtime_get/put during display well power gate/ungate
Hi Sagar, On Tue, 2014-06-10 at 00:27 +0530, sagar.a.kam...@intel.com wrote: > From: Sagar Kamble > > Display power island is on during boot, we have one count for it > once this power gates, we do a put making sure runtime_suspend is > called > > Cc: Daniel Vetter (supporter:INTEL DRM DRIVERS...) > Cc: Jani Nikula (supporter:INTEL DRM DRIVERS...) > Signed-off-by: Sagar Kamble > --- > drivers/gpu/drm/i915/intel_pm.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index f83d1ff..b333aae 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -6017,6 +6017,12 @@ void __vlv_set_power_well(struct drm_i915_private > *dev_priv, > state, > vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_CTRL)); > > + if (PUNIT_POWER_WELL_DISP2D == power_well_id) { > + if (enable) > + intel_runtime_pm_get(dev_priv); > + else > + intel_runtime_pm_put(dev_priv); > + } The RPM refcount should already be get/put properly in intel_display_power_get/put(), so the above doesn't seem correct to me. With current -nightly after blanking the screen the RPM refcount does drop to 0 for me, so I'm not sure what you're missing. One possibility is: # echo auto > /sys/bus/pci/devices/:00:02.0/power/control --Imre signature.asc Description: This is a digitally signed message part ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Boost GPU frequency if we detect outstanding pageflips
On Tue, Jun 10, 2014 at 11:04:02AM +0100, Chris Wilson wrote: > If we hit a vblank and see that have a pageflip queue but not yet > processed, ensure that the GPU is running at maximum in order to clear > the backlog. Pageflips are only queued for the following vblank, if we > miss it, there will be a visible stutter. Boosting the GPU frequency > doesn't prevent us from missing the target vblank, but it should help > the subsequent frames hitting theirs. > > v2: Reorder vblank vs flip-complete so that we only check for a missed > flip after processing the completion events, and avoid spurious boosts. > > v3: Rename missed_vblank > > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > Cc: Ville Syrjälä > --- > drivers/gpu/drm/i915/i915_drv.h |1 + > drivers/gpu/drm/i915/intel_display.c |6 ++ > drivers/gpu/drm/i915/intel_drv.h |1 + > drivers/gpu/drm/i915/intel_pm.c | 15 +++ > 4 files changed, 23 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 10dd80a..33ed0c6 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -910,6 +910,7 @@ struct intel_gen6_power_mgmt { > > bool enabled; > struct delayed_work delayed_resume_work; > + struct work_struct boost_work; > > /* >* Protects RPS/RC6 register access and PCU communication. > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 9ecc6bf..aeb58fa 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9339,6 +9339,7 @@ void intel_check_page_flip(struct drm_device *dev, int > pipe) > struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > unsigned long flags; > + bool missed_vblank; > > if (crtc == NULL) > return; > @@ -9349,7 +9350,12 @@ void intel_check_page_flip(struct drm_device *dev, int > pipe) >intel_crtc->unpin_work->sbc, crtc_sbc(intel_crtc)); > page_flip_completed(intel_crtc); > } > + missed_vblank = (intel_crtc->unpin_work != NULL && > + crtc_sbc(intel_crtc) - intel_crtc->unpin_work->sbc > > 1); > spin_unlock_irqrestore(&dev->event_lock, flags); > + > + if (missed_vblank) > + intel_queue_rps_boost(dev); > } > > static int intel_crtc_page_flip(struct drm_crtc *crtc, > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index ac902ad..75fba0d 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -966,6 +966,7 @@ void ironlake_teardown_rc6(struct drm_device *dev); > void gen6_update_ring_freq(struct drm_device *dev); > void gen6_rps_idle(struct drm_i915_private *dev_priv); > void gen6_rps_boost(struct drm_i915_private *dev_priv); > +void intel_queue_rps_boost(struct drm_device *dev); > void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv); > void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv); > void intel_runtime_pm_get(struct drm_i915_private *dev_priv); > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index b06f896..ab760e5 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -6722,6 +6722,19 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, > int val) > return DIV_ROUND_CLOSEST(4 * mul * val, dev_priv->mem_freq) + 0xbd - 6; > } > > +static void __intel_rps_boost_work(struct work_struct *work) > +{ > + gen6_rps_boost(container_of(work, struct drm_i915_private, > rps.boost_work)); gen6_rps_boost() checks for rps.enabled so it doesn't matter when this gets scheduled wrt. rps enable/disable. Check. And we have a flush_workqueue() in unload path after the modeset cleanup, so the work should be done by the time the workqueue gets torn down. Check. Reviewed-by: Ville Syrjälä > +} > + > +void intel_queue_rps_boost(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = to_i915(dev); > + > + if (INTEL_INFO(dev)->gen >= 6) > + queue_work(dev_priv->wq, &dev_priv->rps.boost_work); > +} > + > void intel_pm_setup(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -6730,6 +6743,8 @@ void intel_pm_setup(struct drm_device *dev) > > INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work, > intel_gen6_powersave_work); > + INIT_WORK(&dev_priv->rps.boost_work, > + __intel_rps_boost_work); > > dev_priv->pm.suspended = false; > dev_priv->pm.irqs_disabled = false; > -- > 1.7.9.5 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/in
[Intel-gfx] [PATCH igt] tests/kms_fbc_crc: Update blit code for BDW
From: Ville Syrjälä Switch to XY_COLOR_BLT from COLOR_BLT and use the appropriate macros to make the code work on BDW. Also make the blit 8bpp instead if 16bpp. 8bpp is what it was supposed to use all along. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76307 Signed-off-by: Ville Syrjälä --- tests/kms_fbc_crc.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c index 810f6f3..a99bf36 100644 --- a/tests/kms_fbc_crc.c +++ b/tests/kms_fbc_crc.c @@ -93,11 +93,12 @@ static void fill_blt(data_t *data, uint32_t handle, unsigned char color) batch = intel_batchbuffer_alloc(data->bufmgr, data->devid); igt_assert(batch); - BEGIN_BATCH(5); - OUT_BATCH(COLOR_BLT_CMD); - OUT_BATCH((1 << 24) | (0xf0 << 16) | 0); + COLOR_BLIT_COPY_BATCH_START(batch->devid, 0); + OUT_BATCH((0 << 24) | (0xf0 << 16) | 0); + OUT_BATCH(0); OUT_BATCH(1 << 16 | 4); OUT_RELOC(dst, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0); + BLIT_RELOC_UDW(batch->devid); OUT_BATCH(color); ADVANCE_BATCH(); -- 1.8.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915/vlv: Set D3_hot for vlv during runtime_suspend
On Tue, 2014-06-10 at 00:27 +0530, sagar.a.kam...@intel.com wrote: > From: Sagar Kamble > > To do a platform wide S0i3 transition, Gfx is required to go > to D3_hot state. pci_save_state and pci_restore_state needed to avoid ring > hangs across D3_hot transitions. > > Cc: Daniel Vetter (supporter:INTEL DRM DRIVERS...) > Cc: Jani Nikula (supporter:INTEL DRM DRIVERS...) > Signed-off-by: Sagar Kamble > --- > drivers/gpu/drm/i915/i915_drv.c | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 5a08c86..70bb456 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1412,6 +1412,11 @@ static int intel_runtime_suspend(struct device *device) >* via the suspend path. >*/ > intel_opregion_notify_adapter(dev, PCI_D1); > + if (IS_VALLEYVIEW(dev)) { > + pci_save_state(pdev); > + pci_disable_device(pdev); > + pci_set_power_state(pdev, PCI_D3hot); > + } > > DRM_DEBUG_KMS("Device suspended\n"); > return 0; > @@ -1428,6 +1433,12 @@ static int intel_runtime_resume(struct device *device) > > DRM_DEBUG_KMS("Resuming device\n"); > > + if (IS_VALLEYVIEW(dev)) { > + pci_set_power_state(pdev, PCI_D0); > + pci_restore_state(pdev); > + pci_enable_device(pdev); > + } Setting the proper Dx state and saving/restoring the PCI config space is already done for us by the PCI runtime PM framework, see pci_pm_runtime_suspend/resume(). They don't disable/enable the PCI device, but I'm not sure if that's really needed. Based on the docs I found so far the requirement for S0ix is that we put the device into D3 state and that's already the case w/o disabling the device. So could you explain/confirm if we need that particular step? Also if it's really needed it should be done for all platforms. --Imre > + > intel_opregion_notify_adapter(dev, PCI_D0); > dev_priv->pm.suspended = false; > signature.asc Description: This is a digitally signed message part ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Boost GPU frequency if we detect outstanding pageflips
If we hit a vblank and see that have a pageflip queue but not yet processed, ensure that the GPU is running at maximum in order to clear the backlog. Pageflips are only queued for the following vblank, if we miss it, there will be a visible stutter. Boosting the GPU frequency doesn't prevent us from missing the target vblank, but it should help the subsequent frames hitting theirs. v2: Reorder vblank vs flip-complete so that we only check for a missed flip after processing the completion events, and avoid spurious boosts. v3: Rename missed_vblank v4: Rebase Signed-off-by: Chris Wilson Cc: Daniel Vetter Cc: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 6 ++ drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 15 +++ 4 files changed, 23 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 10dd80a12658..33ed0c6b8a9c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -910,6 +910,7 @@ struct intel_gen6_power_mgmt { bool enabled; struct delayed_work delayed_resume_work; + struct work_struct boost_work; /* * Protects RPS/RC6 register access and PCU communication. diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5261c145e633..89a4d01fefb4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9326,6 +9326,7 @@ void intel_check_page_flip(struct drm_device *dev, int pipe) struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); unsigned long flags; + bool missed_vblank; if (crtc == NULL) return; @@ -9336,7 +9337,12 @@ void intel_check_page_flip(struct drm_device *dev, int pipe) intel_crtc->unpin_work->flip_queued_vblank, drm_vblank_count(dev, pipe)); page_flip_completed(intel_crtc); } + missed_vblank = (intel_crtc->unpin_work != NULL && +drm_vblank_count(dev, pipe) - intel_crtc->unpin_work->flip_queued_vblank > 1); spin_unlock_irqrestore(&dev->event_lock, flags); + + if (missed_vblank) + intel_queue_rps_boost(dev); } static int intel_crtc_page_flip(struct drm_crtc *crtc, diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 10665716eb10..2e3c65812e72 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -969,6 +969,7 @@ void ironlake_teardown_rc6(struct drm_device *dev); void gen6_update_ring_freq(struct drm_device *dev); void gen6_rps_idle(struct drm_i915_private *dev_priv); void gen6_rps_boost(struct drm_i915_private *dev_priv); +void intel_queue_rps_boost(struct drm_device *dev); void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv); void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv); void intel_runtime_pm_get(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index b06f896740b5..ab760e5abc9a 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -6722,6 +6722,19 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val) return DIV_ROUND_CLOSEST(4 * mul * val, dev_priv->mem_freq) + 0xbd - 6; } +static void __intel_rps_boost_work(struct work_struct *work) +{ + gen6_rps_boost(container_of(work, struct drm_i915_private, rps.boost_work)); +} + +void intel_queue_rps_boost(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + + if (INTEL_INFO(dev)->gen >= 6) + queue_work(dev_priv->wq, &dev_priv->rps.boost_work); +} + void intel_pm_setup(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -6730,6 +6743,8 @@ void intel_pm_setup(struct drm_device *dev) INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work, intel_gen6_powersave_work); + INIT_WORK(&dev_priv->rps.boost_work, + __intel_rps_boost_work); dev_priv->pm.suspended = false; dev_priv->pm.irqs_disabled = false; -- 2.0.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: Check for a stalled page flip after each vblank
Long ago, back in the racy haydays of 915gm interrupt handling, page flips would occasionally go astray and leave the hardware stuck, and the display not updating. This annoyed people who relied on their systems being able to display continuously updating information 24/7, and so some code to detect when the driver missed the page flip completion signal was added. Until recently, it was presumed that the interrupt handling was now flawless, but once again Simon Farnsworth has found a system whose display will stall. Reinstate the pageflip stall detection, which works by checking to see if the hardware has been updated to the new framebuffer address following each vblank. If the hardware is scanning out from the new framebuffer, but we still think the flip is pending, then we kick our driver into submision. This is a continuation of the effort started with commit 4e5359cd053bfb7d8dabe4a63624a5726848ffbc Author: Simon Farnsworth Date: Wed Sep 1 17:47:52 2010 +0100 drm/i915: Avoid pageflipping freeze when we miss the flip prepare interrupt This now includes a belt-and-braces approach to make sure the driver (or the hardware) doesn't miss an interrupt and cause us to stop updating the display should the unthinkable happen and the pageflip fail - i.e. that the user is able to continue submitting flips. v2: Cleanup, refactor, and rename v3: Only start counting vblanks after the flip command has been seen by the hardware. Reported-by: Simon Farnsworth Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75502 Signed-off-by: Chris Wilson Cc: Daniel Vetter Cc: Ville Syrjälä --- drivers/gpu/drm/i915/i915_debugfs.c | 29 ++--- drivers/gpu/drm/i915/i915_irq.c | 85 drivers/gpu/drm/i915/intel_display.c | 121 --- drivers/gpu/drm/i915/intel_drv.h | 5 ++ 4 files changed, 147 insertions(+), 93 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index e8ea1efd3810..eae1a184 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -581,6 +581,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) { struct drm_info_node *node = m->private; struct drm_device *dev = node->minor->dev; + struct drm_i915_private *dev_priv = dev->dev_private; unsigned long flags; struct intel_crtc *crtc; @@ -595,6 +596,8 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) seq_printf(m, "No flip due on pipe %c (plane %c)\n", pipe, plane); } else { + u32 addr; + if (atomic_read(&work->pending) < INTEL_FLIP_COMPLETE) { seq_printf(m, "Flip queued on pipe %c (plane %c)\n", pipe, plane); @@ -602,23 +605,29 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) seq_printf(m, "Flip pending (waiting for vsync) on pipe %c (plane %c)\n", pipe, plane); } + seq_printf(m, "Flip queued on %s at seqno %u, now %u\n", + work->ring->name, + work->flip_queued_seqno, + work->ring->get_seqno(work->ring, true)); + seq_printf(m, "Flip queued on frame %d, (was ready on frame %d), now %d\n", + work->flip_queued_vblank, + work->flip_ready_vblank, + drm_vblank_count(dev, crtc->pipe)); if (work->enable_stall_check) seq_puts(m, "Stall check enabled, "); else seq_puts(m, "Stall check waiting for page flip ioctl, "); seq_printf(m, "%d prepares\n", atomic_read(&work->pending)); - if (work->old_fb_obj) { - struct drm_i915_gem_object *obj = work->old_fb_obj; - if (obj) - seq_printf(m, "Old framebuffer gtt_offset 0x%08lx\n", - i915_gem_obj_ggtt_offset(obj)); - } + if (INTEL_INFO(dev)->gen >= 4) + addr = I915_HI_DISPBASE(I915_READ(DSPSURF(crtc->plane))); + else + addr = I915_READ(DSPADDR(crtc->plane)); + seq_printf(m, "Current scanout address 0x%08x\n", addr); + if (work->pending_flip_obj) { - struct drm_i915_gem_object *obj = work->pending_flip_obj; -
[Intel-gfx] [PATCH v2 maintainer-tools] frob-patch-rank: A little script to batch renaming patch files
The "usage" text should explain it all. I found, in my quilt series handling endeavours, that I wanted to be able to shift the prefix numbers of a patch series. v2: Use heredoc for usage string, fix second example, use mv -i (Jani) Signed-off-by: Damien Lespiau --- frob-patch-rank | 51 +++ 1 file changed, 51 insertions(+) create mode 100755 frob-patch-rank diff --git a/frob-patch-rank b/frob-patch-rank new file mode 100755 index 000..797774e --- /dev/null +++ b/frob-patch-rank @@ -0,0 +1,51 @@ +#!/bin/sh +set -e + +script=$(basename $0) + +read -r -d '' usage << EOU || true +Usage: $script start end expr + + Frob patches." + + This tiny script renames \"git format-patch\" patches by executing 'expr' + on the number that prefix the patch file, but only if the patch file name + starts with a number in ['start','end']. + +Examples: + $ ls *patch + 0008-Super-patch.patch + 0009-Mega-patch.patch + $ $script 8 9 -7 + $ ls *patch + 0001-Super-patch.patch + 0002-Mega-patch.patch + + $ ls *patch + 0117-Super-patch.patch + 0118-Mega-patch.patch + $ $script 117 118 +900 -17 + $ ls *patch + 1000-Super-patch.patch + 1001-Mega-patch.patch +EOU + +[ $# -ge 3 ] || { + echo "$usage" + exit 1 +} + +start=$1 +end=$2 +shift 2 +op=$* + +for i in $(seq $start $end); do + prefix=$(printf "%04d" $i) + for f in $(ls $prefix-*.patch); do + base=${f#$prefix-} + ((n=$i $op)) + new_prefix=$(printf "%04d" $n) + mv -i $prefix-$base $new_prefix-$base + done +done -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Decouple the stuck pageflip on modeset
If we successfully confuse the hardware, and cause it to drop a queued pageflip, we wait for 60s and issue a warning before continuing on with the modeset. However, this leaves the pending pageflip still stuck indefinitely. Pretend to userspace that it does complete, and let us start afresh following the modeset. v2: Rebase after refactor Signed-off-by: Chris Wilson Cc: Daniel Vetter Cc: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ec166e41a27e..5261c145e633 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3363,9 +3363,19 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc) WARN_ON(waitqueue_active(&dev_priv->pending_flip_queue)); - WARN_ON(wait_event_timeout(dev_priv->pending_flip_queue, - !intel_crtc_has_pending_flip(crtc), - 60*HZ) == 0); + if (WARN_ON(wait_event_timeout(dev_priv->pending_flip_queue, + !intel_crtc_has_pending_flip(crtc), + 60*HZ) == 0)) { + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + unsigned long flags; + + spin_lock_irqsave(&dev->event_lock, flags); + if (intel_crtc->unpin_work) { + WARN_ONCE(1, "Removing stuck page flip\n"); + page_flip_completed(intel_crtc); + } + spin_unlock_irqrestore(&dev->event_lock, flags); + } mutex_lock(&dev->struct_mutex); intel_finish_fb(crtc->primary->fb); -- 2.0.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Only mark the ctx as initialised after a SET_CONTEXT operation
On Tue, Jun 10, 2014 at 11:34:34AM +0100, Chris Wilson wrote: > On Fri, May 30, 2014 at 02:16:30PM +0100, Chris Wilson wrote: > > Fallout from > > > > commit 46470fc932ac8a0e8317a220b3f4ea4ed903338e > > Author: Mika Kuoppala > > Date: Wed May 21 19:01:06 2014 +0300 > > > > drm/i915: Add null state batch to active list > > > > undid the earlier fix of only marking the ctx as initialised after it is > > saved by the hardware during a SET_CONTEXT operation. > > > > Signed-off-by: Chris Wilson > > Cc: Ville Syrjälä > > Cc: Damien Lespiau > > Cc: Mika Kuoppala > > Cc: Ben Widawsky > > Ping. Ben's comments do not impact upon the urgent need for this fix. Ok, this code is extremely convoluted. ctx->last_ring is terrible, and we should make a clear disdinction of which ctx->foo fields are for the logical context and which for the per-engine stuff. Will be much more important for execlists so that we properly fan out the state. I'm ranting here a bit since it took me a while to realize that ->is_initialized is _only_ for the RCS stuff. I guess we should have ctx->obj[ring] and ctx->is_initialized[ring] or even better, a substruct i915_hw_context with an array. With that out of the way, this is Reviewed-by: Daniel Vetter Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Check for a stalled page flip after each vblank
On Tue, Jun 10, 2014 at 02:47:29PM +0300, Ville Syrjälä wrote: > On Tue, Jun 10, 2014 at 12:33:48PM +0100, Chris Wilson wrote: > > On Tue, Jun 10, 2014 at 02:25:50PM +0300, Ville Syrjälä wrote: > > > On Tue, Jun 10, 2014 at 11:04:00AM +0100, Chris Wilson wrote: > > > > +static inline int crtc_sbc(struct intel_crtc *crtc) > > > > +{ > > > > + return atomic_read(&crtc->base.dev->vblank[crtc->pipe].count); > > > > +} > > > > > > Still says 'sbc' which doesn't make sense to me. > > > > I just don't like the term msc. :-p > > crtc_vblank_counter()? > > Maybe stick it into drmP.h and call it drm_crtc_vblank_counter() or > something? Hopefully people won't confuse it with the hardware counter. It's called drm_vblank_count, but a new wrapper which takes a drm_crtc * as the argument would indeed be nice. So drm_crtc_vblank_counter(struct drm_crtc *crtc). And please don't forget to add the kerneldoc and to pull it into the drm docbook. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i95: Initialize active ring->pid to -1
On Tue, Jun 10, 2014 at 12:09:29PM +0100, Chris Wilson wrote: > Otherwise we print out spurious processes on unused rings in the error > state. > > Signed-off-by: Chris Wilson > Cc: sta...@vger.kernel.org Personally would have done that in the dumper code, not the recording code - this here looks a bit inconsistent. Either way this is Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_gpu_error.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c > b/drivers/gpu/drm/i915/i915_gpu_error.c > index 87ec60e181a7..66cf41765bf9 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -888,6 +888,8 @@ static void i915_gem_record_rings(struct drm_device *dev, > for (i = 0; i < I915_NUM_RINGS; i++) { > struct intel_engine_cs *ring = &dev_priv->ring[i]; > > + error->ring[i].pid = -1; > + > if (ring->dev == NULL) > continue; > > @@ -895,7 +897,6 @@ static void i915_gem_record_rings(struct drm_device *dev, > > i915_record_ring_state(dev, ring, &error->ring[i]); > > - error->ring[i].pid = -1; > request = i915_gem_find_active_request(ring); > if (request) { > /* We need to copy these to an anonymous buffer > -- > 2.0.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: leave rc6 enabled at suspend time v4
On Wed, 2014-06-04 at 13:45 -0700, Jesse Barnes wrote: > This allows the system to enter the lowest power mode during system freeze. > > v2: delete force wake timer at suspend (Imre) > v3: add GT work suspend function (Imre) > v4: use uncore forcewake reset (Daniel) > > Signed-off-by: Kristen Carlson Accardi > Signed-off-by: Jesse Barnes > --- > drivers/gpu/drm/i915/i915_drv.c | 4 ++-- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_drv.h| 1 + > drivers/gpu/drm/i915/intel_pm.c | 20 > drivers/gpu/drm/i915/intel_uncore.c | 2 +- > 5 files changed, 25 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 66c6ffb..7148eac 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -521,7 +521,7 @@ static int i915_drm_freeze(struct drm_device *dev) > drm_irq_uninstall(dev); > dev_priv->enable_hotplug_processing = false; > > - intel_disable_gt_powersave(dev); > + intel_suspend_gt_powersave(dev); I realized now that we actually do need to enable RC6 explicitly. If we get here right after runtime resume, the deferred RC6 enabling might be still pending and so RC6 might not be enabled. --Imre > > /* >* Disable CRTCs directly since we want to preserve sw state > @@ -542,8 +542,8 @@ static int i915_drm_freeze(struct drm_device *dev) > > i915_save_state(dev); > > + intel_uncore_forcewake_reset(dev, false); > intel_opregion_fini(dev); > - intel_uncore_fini(dev); > > console_lock(); > intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index bea9ab40..89d6b47 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2084,6 +2084,7 @@ extern void intel_uncore_early_sanitize(struct > drm_device *dev); > extern void intel_uncore_init(struct drm_device *dev); > extern void intel_uncore_check_errors(struct drm_device *dev); > extern void intel_uncore_fini(struct drm_device *dev); > +extern void intel_uncore_forcewake_reset(struct drm_device *dev, bool > restore); > > void > i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe, > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index c597b0d..74fbe4d 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -957,6 +957,7 @@ void intel_init_gt_powersave(struct drm_device *dev); > void intel_cleanup_gt_powersave(struct drm_device *dev); > void intel_enable_gt_powersave(struct drm_device *dev); > void intel_disable_gt_powersave(struct drm_device *dev); > +void intel_suspend_gt_powersave(struct drm_device *dev); > void intel_reset_gt_powersave(struct drm_device *dev); > void ironlake_teardown_rc6(struct drm_device *dev); > void gen6_update_ring_freq(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 1840d15..139eebe 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4864,6 +4864,26 @@ void intel_cleanup_gt_powersave(struct drm_device *dev) > valleyview_cleanup_gt_powersave(dev); > } > > +/** > + * intel_suspend_gt_powersave - suspend PM work and helper threads > + * @dev: drm device > + * > + * We don't want to disable RC6 or other features here, we just want > + * to make sure any work we've queued has finished and won't bother > + * us while we're suspended. > + */ > +void intel_suspend_gt_powersave(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + /* Interrupts should be disabled already to avoid re-arming. */ > + WARN_ON(dev->irq_enabled); > + > + flush_delayed_work(&dev_priv->rps.delayed_resume_work); > + > + cancel_work_sync(&dev_priv->rps.work); > +} > + > void intel_disable_gt_powersave(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > diff --git a/drivers/gpu/drm/i915/intel_uncore.c > b/drivers/gpu/drm/i915/intel_uncore.c > index 871c284..741a4e3 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -316,7 +316,7 @@ static void gen6_force_wake_timer(unsigned long arg) > intel_runtime_pm_put(dev_priv); > } > > -static void intel_uncore_forcewake_reset(struct drm_device *dev, bool > restore) > +void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore) > { > struct drm_i915_private *dev_priv = dev->dev_private; > unsigned long irqflags; signature.asc Description: This is a digitally signed message part ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo
Re: [Intel-gfx] [PATCH] drm/i95: Initialize active ring->pid to -1
On Tue, Jun 10, 2014 at 03:30:25PM +0200, Daniel Vetter wrote: > On Tue, Jun 10, 2014 at 12:09:29PM +0100, Chris Wilson wrote: > > Otherwise we print out spurious processes on unused rings in the error > > state. > > > > Signed-off-by: Chris Wilson > > Cc: sta...@vger.kernel.org > > Personally would have done that in the dumper code, not the recording code > - this here looks a bit inconsistent. Either way this is It gets used in multiple locations, in particular inside the capture code itself, so it looked simpler to initialise it always. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Check for a stalled page flip after each vblank
On Tue, Jun 10, 2014 at 01:46:54PM +0100, Chris Wilson wrote: > Long ago, back in the racy haydays of 915gm interrupt handling, page > flips would occasionally go astray and leave the hardware stuck, and the > display not updating. This annoyed people who relied on their systems > being able to display continuously updating information 24/7, and so > some code to detect when the driver missed the page flip completion > signal was added. Until recently, it was presumed that the interrupt > handling was now flawless, but once again Simon Farnsworth has found a > system whose display will stall. Reinstate the pageflip stall detection, > which works by checking to see if the hardware has been updated to the > new framebuffer address following each vblank. If the hardware is > scanning out from the new framebuffer, but we still think the flip is > pending, then we kick our driver into submision. > > This is a continuation of the effort started with > commit 4e5359cd053bfb7d8dabe4a63624a5726848ffbc > Author: Simon Farnsworth > Date: Wed Sep 1 17:47:52 2010 +0100 > > drm/i915: Avoid pageflipping freeze when we miss the flip prepare > interrupt > > This now includes a belt-and-braces approach to make sure the driver > (or the hardware) doesn't miss an interrupt and cause us to stop > updating the display should the unthinkable happen and the pageflip fail - > i.e. > that the user is able to continue submitting flips. > > v2: Cleanup, refactor, and rename > v3: Only start counting vblanks after the flip command has been seen by > the hardware. > > Reported-by: Simon Farnsworth > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75502 > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > Cc: Ville Syrjälä Reviewed-by: Ville Syrjälä Someone may want to add the flip counter check later. At least there's a comment about danger of false positives. > --- > drivers/gpu/drm/i915/i915_debugfs.c | 29 ++--- > drivers/gpu/drm/i915/i915_irq.c | 85 > drivers/gpu/drm/i915/intel_display.c | 121 > --- > drivers/gpu/drm/i915/intel_drv.h | 5 ++ > 4 files changed, 147 insertions(+), 93 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index e8ea1efd3810..eae1a184 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -581,6 +581,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, > void *data) > { > struct drm_info_node *node = m->private; > struct drm_device *dev = node->minor->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > unsigned long flags; > struct intel_crtc *crtc; > > @@ -595,6 +596,8 @@ static int i915_gem_pageflip_info(struct seq_file *m, > void *data) > seq_printf(m, "No flip due on pipe %c (plane %c)\n", > pipe, plane); > } else { > + u32 addr; > + > if (atomic_read(&work->pending) < INTEL_FLIP_COMPLETE) { > seq_printf(m, "Flip queued on pipe %c (plane > %c)\n", > pipe, plane); > @@ -602,23 +605,29 @@ static int i915_gem_pageflip_info(struct seq_file *m, > void *data) > seq_printf(m, "Flip pending (waiting for vsync) > on pipe %c (plane %c)\n", > pipe, plane); > } > + seq_printf(m, "Flip queued on %s at seqno %u, now %u\n", > +work->ring->name, > +work->flip_queued_seqno, > +work->ring->get_seqno(work->ring, true)); > + seq_printf(m, "Flip queued on frame %d, (was ready on > frame %d), now %d\n", > +work->flip_queued_vblank, > +work->flip_ready_vblank, > +drm_vblank_count(dev, crtc->pipe)); > if (work->enable_stall_check) > seq_puts(m, "Stall check enabled, "); > else > seq_puts(m, "Stall check waiting for page flip > ioctl, "); > seq_printf(m, "%d prepares\n", > atomic_read(&work->pending)); > > - if (work->old_fb_obj) { > - struct drm_i915_gem_object *obj = > work->old_fb_obj; > - if (obj) > - seq_printf(m, "Old framebuffer > gtt_offset 0x%08lx\n", > - > i915_gem_obj_ggtt_offset(obj)); > - } > + if (INTEL_INFO(dev)->gen >= 4) > + addr = > I915_HI_DISPBASE(I915_READ(DSPSURF(crtc->plane))); > +
Re: [Intel-gfx] [PATCH] drm/i915: leave rc6 enabled at suspend time v4
On Tue, Jun 10, 2014 at 04:42:50PM +0300, Imre Deak wrote: > On Wed, 2014-06-04 at 13:45 -0700, Jesse Barnes wrote: > > This allows the system to enter the lowest power mode during system freeze. > > > > v2: delete force wake timer at suspend (Imre) > > v3: add GT work suspend function (Imre) > > v4: use uncore forcewake reset (Daniel) > > > > Signed-off-by: Kristen Carlson Accardi > > Signed-off-by: Jesse Barnes > > --- > > drivers/gpu/drm/i915/i915_drv.c | 4 ++-- > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > drivers/gpu/drm/i915/intel_drv.h| 1 + > > drivers/gpu/drm/i915/intel_pm.c | 20 > > drivers/gpu/drm/i915/intel_uncore.c | 2 +- > > 5 files changed, 25 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index 66c6ffb..7148eac 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -521,7 +521,7 @@ static int i915_drm_freeze(struct drm_device *dev) > > drm_irq_uninstall(dev); > > dev_priv->enable_hotplug_processing = false; > > > > - intel_disable_gt_powersave(dev); > > + intel_suspend_gt_powersave(dev); > > I realized now that we actually do need to enable RC6 explicitly. If we > get here right after runtime resume, the deferred RC6 enabling might be > still pending and so RC6 might not be enabled. Hm, for runtime pm we might schedule the delayed rps work with a 0 delay, just to get the gpu going faster. Just an aside. Also some runtime pm vs system suspend tests in igt would be good if we don't have them yet. And if we have them some analysis why this didn't blow up in testing (and something to address that gap if feasible). -Daniel > > --Imre > > > > > /* > > * Disable CRTCs directly since we want to preserve sw state > > @@ -542,8 +542,8 @@ static int i915_drm_freeze(struct drm_device *dev) > > > > i915_save_state(dev); > > > > + intel_uncore_forcewake_reset(dev, false); > > intel_opregion_fini(dev); > > - intel_uncore_fini(dev); > > > > console_lock(); > > intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index bea9ab40..89d6b47 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -2084,6 +2084,7 @@ extern void intel_uncore_early_sanitize(struct > > drm_device *dev); > > extern void intel_uncore_init(struct drm_device *dev); > > extern void intel_uncore_check_errors(struct drm_device *dev); > > extern void intel_uncore_fini(struct drm_device *dev); > > +extern void intel_uncore_forcewake_reset(struct drm_device *dev, bool > > restore); > > > > void > > i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe, > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > b/drivers/gpu/drm/i915/intel_drv.h > > index c597b0d..74fbe4d 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -957,6 +957,7 @@ void intel_init_gt_powersave(struct drm_device *dev); > > void intel_cleanup_gt_powersave(struct drm_device *dev); > > void intel_enable_gt_powersave(struct drm_device *dev); > > void intel_disable_gt_powersave(struct drm_device *dev); > > +void intel_suspend_gt_powersave(struct drm_device *dev); > > void intel_reset_gt_powersave(struct drm_device *dev); > > void ironlake_teardown_rc6(struct drm_device *dev); > > void gen6_update_ring_freq(struct drm_device *dev); > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > b/drivers/gpu/drm/i915/intel_pm.c > > index 1840d15..139eebe 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -4864,6 +4864,26 @@ void intel_cleanup_gt_powersave(struct drm_device > > *dev) > > valleyview_cleanup_gt_powersave(dev); > > } > > > > +/** > > + * intel_suspend_gt_powersave - suspend PM work and helper threads > > + * @dev: drm device > > + * > > + * We don't want to disable RC6 or other features here, we just want > > + * to make sure any work we've queued has finished and won't bother > > + * us while we're suspended. > > + */ > > +void intel_suspend_gt_powersave(struct drm_device *dev) > > +{ > > + struct drm_i915_private *dev_priv = dev->dev_private; > > + > > + /* Interrupts should be disabled already to avoid re-arming. */ > > + WARN_ON(dev->irq_enabled); > > + > > + flush_delayed_work(&dev_priv->rps.delayed_resume_work); > > + > > + cancel_work_sync(&dev_priv->rps.work); > > +} > > + > > void intel_disable_gt_powersave(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c > > b/drivers/gpu/drm/i915/intel_uncore.c > > index 871c284..741a4e3 100644 > > --- a/drivers/gpu/drm/i915/intel_uncore.c > > +++ b/drivers/gpu/drm/i915/intel_unco
Re: [Intel-gfx] [PATCH 1/5] drm/i915: preserve SSC if previously set v2
On Thu, Jun 05, 2014 at 11:24:27AM -0700, Jesse Barnes wrote: > Some machines may have a broken VBT or no VBT at all, but we still want > to use SSC there. So check for it and keep it enabled if we see it > already on. Based on an earlier fix from Kristian. > > v2: honor modparam if set too (Daniel) > read out at init time and store for panel_use_ssc() use (Jesse) > > Reported-by: Kristian Høgsberg > Signed-off-by: Jesse Barnes Ugh. This means the BIOS idea of when we need SSC and ours are diverging, which isn't good. And I think we should further lock this down by tracking the "uses SSC refclk" state in the pipe config, just to make sure we don't fumble this. -Daniel > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_display.c | 11 ++- > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 8631fb3..f57b752 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1404,6 +1404,8 @@ struct drm_i915_private { > struct intel_opregion opregion; > struct intel_vbt_data vbt; > > + bool bios_ssc; /* BIOS had SSC enabled at boot? */ > + > /* overlay */ > struct intel_overlay *overlay; > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index b5cbb28..0e8c9bc 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5355,7 +5355,7 @@ static inline bool intel_panel_use_ssc(struct > drm_i915_private *dev_priv) > { > if (i915.panel_use_ssc >= 0) > return i915.panel_use_ssc != 0; > - return dev_priv->vbt.lvds_use_ssc > + return (dev_priv->vbt.lvds_use_ssc || dev_priv->bios_ssc) > && !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE); > } > > @@ -12478,6 +12478,7 @@ void intel_modeset_setup_hw_state(struct drm_device > *dev, > > void intel_modeset_gem_init(struct drm_device *dev) > { > + struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_crtc *c; > struct intel_framebuffer *fb; > > @@ -12485,6 +12486,14 @@ void intel_modeset_gem_init(struct drm_device *dev) > intel_init_gt_powersave(dev); > mutex_unlock(&dev->struct_mutex); > > + /* > + * There may be no VBT; and if the BIOS enabled SSC we can > + * just keep using it to avoid unnecessary flicker. > + */ > + if ((HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) && > + (I915_READ(PCH_DREF_CONTROL) & DREF_SSC1_ENABLE)) > + dev_priv->bios_ssc = true; > + > intel_modeset_init_hw(dev); > > intel_setup_overlay(dev); > -- > 1.8.3.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] drm/i915: preserve swizzle settings if necessary v3
On Thu, Jun 05, 2014 at 11:24:28AM -0700, Jesse Barnes wrote: > Some machines (like MBAs) might use a tiled framebuffer but not enable > display swizzling at boot time. We want to preserve that configuration > if possible to prevent a boot time mode set. On IVB+ it shouldn't > affect performance anyway since the memory controller does internal > swizzling anyway. > > For most other configs we'll be able to enable swizzling at boot time, > since the initial framebuffer won't be tiled, thus we won't see any > corruption when we enable it. > > v2: preserve swizzling if BIOS had it set (Daniel) > v3: preserve swizzling only if we inherited a tiled framebuffer (Daniel) > check display swizzle setting in detect_bit_6_swizzle (Daniel) > use gen6 as cutoff point (Daniel) > > Reported-by: Kristian Høgsberg > Signed-off-by: Jesse Barnes > --- > drivers/gpu/drm/i915/i915_drv.h| 1 + > drivers/gpu/drm/i915/i915_gem.c| 3 +++ > drivers/gpu/drm/i915/i915_gem_tiling.c | 38 > +++--- > drivers/gpu/drm/i915/intel_display.c | 3 +++ > 4 files changed, 28 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index f57b752..f49fdcb 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1405,6 +1405,7 @@ struct drm_i915_private { > struct intel_vbt_data vbt; > > bool bios_ssc; /* BIOS had SSC enabled at boot? */ > + bool preserve_bios_swizzle; > > /* overlay */ > struct intel_overlay *overlay; > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index bfc7af0..0b168fb 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4580,6 +4580,9 @@ void i915_gem_init_swizzling(struct drm_device *dev) > dev_priv->mm.bit_6_swizzle_x == I915_BIT_6_SWIZZLE_NONE) > return; > > + if (INTEL_INFO(dev)->gen >= 6 && dev_priv->preserve_bios_swizzle) > + return; > + Above two hunks shouldnt be needed if the setup in i915_gem_detect_bit_6_swizzle works correctly. > I915_WRITE(DISP_ARB_CTL, I915_READ(DISP_ARB_CTL) | >DISP_TILE_SURFACE_SWIZZLING); > > diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c > b/drivers/gpu/drm/i915/i915_gem_tiling.c > index cb150e8..73005ad 100644 > --- a/drivers/gpu/drm/i915/i915_gem_tiling.c > +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c > @@ -91,26 +91,30 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev) > uint32_t swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; > uint32_t swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; > > - if (IS_VALLEYVIEW(dev)) { > - swizzle_x = I915_BIT_6_SWIZZLE_NONE; > - swizzle_y = I915_BIT_6_SWIZZLE_NONE; > - } else if (INTEL_INFO(dev)->gen >= 6) { > + if (INTEL_INFO(dev)->gen >= 6) { > uint32_t dimm_c0, dimm_c1; > - dimm_c0 = I915_READ(MAD_DIMM_C0); > - dimm_c1 = I915_READ(MAD_DIMM_C1); > - dimm_c0 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_B_SIZE_MASK; > - dimm_c1 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_B_SIZE_MASK; > - /* Enable swizzling when the channels are populated with > - * identically sized dimms. We don't need to check the 3rd > - * channel because no cpu with gpu attached ships in that > - * configuration. Also, swizzling only makes sense for 2 > - * channels anyway. */ > - if (dimm_c0 == dimm_c1) { > - swizzle_x = I915_BIT_6_SWIZZLE_9_10; > - swizzle_y = I915_BIT_6_SWIZZLE_9; > - } else { > + > + /* Make sure to honor boot time display configuration */ > + if (!(I915_READ(DISP_ARB_CTL) & DISP_TILE_SURFACE_SWIZZLING)) { Not quite what I had in mind. Imo we need to check for whether any inherited fbs are tiled and if so also inherit the swizzle setting unconditionally, whether it is swizzled or unswizzled. See http://patchwork.freedesktop.org/patch/22204/ Cheers, Daniel > swizzle_x = I915_BIT_6_SWIZZLE_NONE; > swizzle_y = I915_BIT_6_SWIZZLE_NONE; > + } else { > + dimm_c0 = I915_READ(MAD_DIMM_C0); > + dimm_c1 = I915_READ(MAD_DIMM_C1); > + dimm_c0 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_B_SIZE_MASK; > + dimm_c1 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_B_SIZE_MASK; > + /* Enable swizzling when the channels are populated with > + * identically sized dimms. We don't need to check the > + * 3rd channel because no cpu with gpu attached ships > + * in that configuration. Also, swizzling only makes > + * sense for 2 channels anyway. */ > + if (dimm_c0 == dimm
Re: [Intel-gfx] [PATCH] drm/i915: Init important ns2501 registers
On Tue, Jun 10, 2014 at 12:29:20AM +0200, Thomas Richter wrote: > Hi Ville, > > thanks for the latest patch. As said, the screen did not come back quite > correctly. I checked the register values > again, and I am sorry to say that I was wrong - register values do > differ. Apparently, the code configures now > the wrong pipe to generate output to the DVO and thus the DVO does not > seem to synchronize correctly > anymore. Please find the two register dumps attached. Either pipe can drive DVO just fine. Looks like it's using pipe A in your register dump, and all the registers look fine to me. Well, DPLL B VCO enable is off since we don't currently have a mechanism to kick pipe B into action during resume/load. In theory that would need to be enabled as well. Can you see if a simple 'intel_reg_write 0x6018 0xc08b' fixes the problem? And if not, I'd like to see a diff of register dumps between working and non working setups. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/5] drm/i915: use current mode if the size matches the preferred mode
On Thu, Jun 05, 2014 at 11:24:30AM -0700, Jesse Barnes wrote: > From: Kristian Høgsberg > > The BIOS may set a native mode that doesn't quite match the preferred > mode timings. It should be ok to use however if it uses the same size, > so try to avoid a mode set in that case. > > Signed-off-by: Kristian Høgsberg > Signed-off-by: Jesse Barnes Not sure we want this since this seems to override the cmdline options to force a specific edid. Also not sure whether we shouldn't just add this as the preferred mode when probing (before the preferred mode the vbt/edid provides ofc). What exactly is the mismatch here? It could be DRRS or something fancy, too. Not sure what to do here really. -Daniel > --- > drivers/gpu/drm/i915/intel_fbdev.c | 47 > +++--- > 1 file changed, 18 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c > b/drivers/gpu/drm/i915/intel_fbdev.c > index 088fe93..09819ae 100644 > --- a/drivers/gpu/drm/i915/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > @@ -379,42 +379,31 @@ static bool intel_fb_initial_config(struct > drm_fb_helper *fb_helper, > /* go for command line mode first */ > modes[i] = drm_pick_cmdline_mode(fb_conn, width, height); > > - /* try for preferred next */ > + /* try for preferred next or match current */ > if (!modes[i]) { > - DRM_DEBUG_KMS("looking for preferred mode on connector > %s\n", > - connector->name); > - modes[i] = drm_has_preferred_mode(fb_conn, width, > - height); > - } > + struct drm_display_mode *preferred; > > - /* No preferred mode marked by the EDID? Are there any modes? */ > - if (!modes[i] && !list_empty(&connector->modes)) { > - DRM_DEBUG_KMS("using first mode listed on connector > %s\n", > + DRM_DEBUG_KMS("looking for preferred mode on connector > %s\n", > connector->name); > - modes[i] = list_first_entry(&connector->modes, > - struct drm_display_mode, > - head); > - } > + preferred = drm_has_preferred_mode(fb_conn, width, > +height); > > - /* last resort: use current mode */ > - if (!modes[i]) { > - /* > - * IMPORTANT: We want to use the adjusted mode (i.e. > - * after the panel fitter upscaling) as the initial > - * config, not the input mode, which is what crtc->mode > - * usually contains. But since our current fastboot > - * code puts a mode derived from the post-pfit timings > - * into crtc->mode this works out correctly. We don't > - * use hwmode anywhere right now, so use it for this > - * since the fb helper layer wants a pointer to > - * something we own. > - */ > - DRM_DEBUG_KMS("looking for current mode on connector > %s\n", > - connector->name); > intel_mode_from_pipe_config(&encoder->crtc->hwmode, > > &to_intel_crtc(encoder->crtc)->config); > - modes[i] = &encoder->crtc->hwmode; > + modes[i] = &encoder->crtc->hwmode; > + > + if (preferred && > + !drm_mode_same_size(preferred, modes[i])) { > + DRM_DEBUG_KMS("using preferred mode %s " > + "instead of current mode %s " > + "on connector %d\n", > + preferred->name, > + modes[i]->name, > + fb_conn->connector->base.id); > + modes[i] = preferred; > + } > } > + > crtcs[i] = new_crtc; > > DRM_DEBUG_KMS("connector %s on pipe %d [CRTC:%d]: %dx%d%s\n", > -- > 1.8.3.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: enable fastboot by default
On Thu, Jun 05, 2014 at 11:24:31AM -0700, Jesse Barnes wrote: > Let them eat mincemeat pie. > > Signed-off-by: Jesse Barnes > --- > drivers/gpu/drm/i915/i915_params.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_params.c > b/drivers/gpu/drm/i915/i915_params.c > index d05a2af..081ab2f 100644 > --- a/drivers/gpu/drm/i915/i915_params.c > +++ b/drivers/gpu/drm/i915/i915_params.c > @@ -41,7 +41,7 @@ struct i915_params i915 __read_mostly = { > .preliminary_hw_support = > IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), > .disable_power_well = 1, > .enable_ips = 1, > - .fastboot = 0, > + .fastboot = 42, > .prefault_disable = 0, > .reset = true, > .invert_brightness = 0, > @@ -132,7 +132,7 @@ MODULE_PARM_DESC(enable_ips, "Enable IPS (default: > true)"); > > module_param_named(fastboot, i915.fastboot, bool, 0600); > MODULE_PARM_DESC(fastboot, > - "Try to skip unnecessary mode sets at boot time (default: false)"); > + "Try to skip unnecessary mode sets at boot time (default: true)"); Nah, that wasn't the intention of this option. It was meant as a hack to experiment around with fastboot and get things going, but imo we need to really do the full modeset and short-circuit if the state matches. And there's still a bunch of things we don't track like infoframes which we either need to fix up (similar to the pfit fixup) or quirk to disallow fastboot. -Daniel > > module_param_named(prefault_disable, i915.prefault_disable, bool, 0600); > MODULE_PARM_DESC(prefault_disable, > -- > 1.8.3.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: enable fastboot by default
On Fri, Jun 06, 2014 at 02:12:44PM +0300, Jani Nikula wrote: > On Thu, 05 Jun 2014, Jesse Barnes wrote: > > Let them eat mincemeat pie. > > > > Signed-off-by: Jesse Barnes > > --- > > drivers/gpu/drm/i915/i915_params.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_params.c > > b/drivers/gpu/drm/i915/i915_params.c > > index d05a2af..081ab2f 100644 > > --- a/drivers/gpu/drm/i915/i915_params.c > > +++ b/drivers/gpu/drm/i915/i915_params.c > > @@ -41,7 +41,7 @@ struct i915_params i915 __read_mostly = { > > .preliminary_hw_support = > > IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), > > .disable_power_well = 1, > > .enable_ips = 1, > > - .fastboot = 0, > > + .fastboot = 42, > > The answer to the ultimate question of life, the universe, and > everything should simply be "true" here. Personally, I don't think it's > a bad answer. > > > .prefault_disable = 0, > > .reset = true, > > .invert_brightness = 0, > > @@ -132,7 +132,7 @@ MODULE_PARM_DESC(enable_ips, "Enable IPS (default: > > true)"); > > > > module_param_named(fastboot, i915.fastboot, bool, 0600); > > Side note, why do we allow the param to be changed after boot? Because fastboot works for every modeset, or at least should. E.g. if you go from an upscale panel resolotion to the full one we still want to just fixup the pfit, even when it's a normal modeset. So fastboot really is a bit a misnomer, since the leftover bits (after I've convinced Jesse that the fb takeover should be done unconditionally really) is just some modeset fastpaths. Aside: We still have a leak on the inherited fb, see https://bugs.freedesktop.org/show_bug.cgi?id=77511 -Daniel > > BR, > Jani. > > > MODULE_PARM_DESC(fastboot, > > - "Try to skip unnecessary mode sets at boot time (default: false)"); > > + "Try to skip unnecessary mode sets at boot time (default: true)"); > > > > module_param_named(prefault_disable, i915.prefault_disable, bool, 0600); > > MODULE_PARM_DESC(prefault_disable, > > -- > > 1.8.3.2 > > > > ___ > > dri-devel mailing list > > dri-de...@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Jani Nikula, Intel Open Source Technology Center > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [i-g-t 2/7] lib: remove /** from comments that are not API documentation
These comments are not gtk-doc comments, so replacing /** with /* prevents any gtk-doc warnings. Signed-off-by: Thomas Wood --- lib/i915_3d.h | 26 ++-- lib/intel_reg.h | 404 +- lib/rendercopy_gen8.c | 6 +- 3 files changed, 218 insertions(+), 218 deletions(-) diff --git a/lib/i915_3d.h b/lib/i915_3d.h index 04531f3..eea379c 100644 --- a/lib/i915_3d.h +++ b/lib/i915_3d.h @@ -324,7 +324,7 @@ enum i915_fs_channel { (z##_CHANNEL_VAL << Z_CHANNEL_SHIFT) | \ (w##_CHANNEL_VAL << W_CHANNEL_SHIFT) -/** +/* * Construct an operand description for using a register with no swizzling */ #define i915_fs_operand_reg(reg) \ @@ -333,17 +333,17 @@ enum i915_fs_channel { #define i915_fs_operand_reg_negate(reg) \ i915_fs_operand(reg, NEG_X, NEG_Y, NEG_Z, NEG_W) -/** +/* * Returns an operand containing (0.0, 0.0, 0.0, 0.0). */ #define i915_fs_operand_zero() i915_fs_operand(FS_R0, ZERO, ZERO, ZERO, ZERO) -/** +/* * Returns an unused operand */ #define i915_fs_operand_none() i915_fs_operand_zero() -/** +/* * Returns an operand containing (1.0, 1.0, 1.0, 1.0). */ #define i915_fs_operand_one() i915_fs_operand(FS_R0, ONE, ONE, ONE, ONE) @@ -351,7 +351,7 @@ enum i915_fs_channel { #define i915_get_hardware_channel_val(val, shift, negate) \ (((val & 0x7) << shift) | ((val & 0x8) ? negate : 0)) -/** +/* * Outputs a fragment shader command to declare a sampler or texture register. */ #define i915_fs_dcl(reg) \ @@ -519,19 +519,19 @@ enum i915_fs_channel { i915_fs_operand_none(), \ i915_fs_operand_none()) -/** Add operand0 and operand1 and put the result in dest_reg */ +/* Add operand0 and operand1 and put the result in dest_reg */ #define i915_fs_add(dest_reg, operand0, operand1) \ i915_fs_arith (ADD, dest_reg, \ operand0, operand1, \ i915_fs_operand_none()) -/** Multiply operand0 and operand1 and put the result in dest_reg */ +/* Multiply operand0 and operand1 and put the result in dest_reg */ #define i915_fs_mul(dest_reg, operand0, operand1) \ i915_fs_arith (MUL, dest_reg, \ operand0, operand1, \ i915_fs_operand_none()) -/** Computes 1/sqrt(operand0.replicate_swizzle) puts the result in dest_reg */ +/* Computes 1/sqrt(operand0.replicate_swizzle) puts the result in dest_reg */ #define i915_fs_rsq(dest_reg, dest_mask, operand0) \ do { \ if (dest_mask) { \ @@ -547,13 +547,13 @@ enum i915_fs_channel { } \ } while (0) -/** Puts the minimum of operand0 and operand1 in dest_reg */ +/* Puts the minimum of operand0 and operand1 in dest_reg */ #define i915_fs_min(dest_reg, operand0, operand1) \ i915_fs_arith (MIN, dest_reg, \ operand0, operand1, \ i915_fs_operand_none()) -/** Puts the maximum of operand0 and operand1 in dest_reg */ +/* Puts the maximum of operand0 and operand1 in dest_reg */ #define i915_fs_max(dest_reg, operand0, operand1) \ i915_fs_arith (MAX, dest_reg, \ operand0, operand1, \ @@ -562,7 +562,7 @@ enum i915_fs_channel { #define i915_fs_cmp(dest_reg, operand0, operand1, operand2)\ i915_fs_arith (CMP, dest_reg, operand0, operand1, operand2) -/** Perform operand0 * operand1 + operand2 and put the result in dest_reg */ +/* Perform operand0 * operand1 + operand2 and put the result in dest_reg */ #define i915_fs_mad(dest_reg, dest_mask, op0, op1, op2)\ do { \ if (dest_mask) { \ @@ -581,7 +581,7 @@ enum i915_fs_channel { } \ } while (0) -/** +/* * Perform a 3-component dot-product of operand0 and operand1 and put the * resulting scalar in the channels of dest_reg specified by the dest_mask. */ @@ -597,7 +597,7 @@ enum i915_fs_channel { } \ } while (0) -/** +/* * Sets up local state for accumulating a fragment shader buffer. * * \param x maximum number of shader commands that may be used between diff --git a/lib/intel_reg.h b/lib/intel_reg.h index 84e05e4..f8ad71f 100644 --- a/lib/intel_reg.h +++ b/lib/intel_reg.h @@ -26,7 +26,7 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. **/ -/** @file +/* @file * Register names and field
[Intel-gfx] [i-g-t 3/7] README: update the section on modifying and rebuilding documentation
Signed-off-by: Thomas Wood --- README | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/README b/README index cfa186d..5e98565 100644 --- a/README +++ b/README @@ -108,16 +108,14 @@ docs/ reference documenation in docs/reference/ You need to have the gtk doc tools installed to generate this API documentation. - Note that the currrent gtk-docs integration sucks a bit wrt regenerating - the html files. You need at least + To regenerate the html files when updating documentation, use: $ make clean -C docs && make -C docs - to regenerate them on any change. If you've added/changed/removed a - symbol or anything else that changes the overall structure or indexes, - you need a full rebuild: - - $ git clean -dfx && ./autogen.sh --enable-gtk-doc && make -C docs + If you've added/changed/removed a symbol or anything else that changes + the overall structure or indexes, this needs to be reflected in + intel-gpu-tools-sections.txt. Entirely new sections will also need to be + added to intel-gpu-tools-docs.xml in the appropriate place. DEPENDENCIES This is a non-exchaustive list of package dependencies required for -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [i-g-t 5/7] gitignore: add missing files and keep lists sorted
Signed-off-by: Thomas Wood --- tests/.gitignore| 11 +++ tools/null_state_gen/.gitignore | 1 + 2 files changed, 8 insertions(+), 4 deletions(-) create mode 100644 tools/null_state_gen/.gitignore diff --git a/tests/.gitignore b/tests/.gitignore index d7ad054..a61d025 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -1,6 +1,6 @@ # Please keep sorted alphabetically -core_getclient core_get_client_auth +core_getclient core_getstats core_getversion ddi_compute_wrpll @@ -41,8 +41,8 @@ gem_exec_nop gem_exec_params gem_exec_parse gem_fd_exhaustion -gem_fenced_exec_thrash gem_fence_thrash +gem_fenced_exec_thrash gem_flink gem_flink_race gem_gtt_cpu_tlb @@ -77,9 +77,9 @@ gem_render_copy_redux gem_render_linear_blits gem_render_tiled_blits gem_reset_stats -gem_ringfill gem_ring_sync_copy gem_ring_sync_loop +gem_ringfill gem_seqno_wrap gem_set_tiling_vs_blt gem_set_tiling_vs_gtt @@ -118,17 +118,20 @@ igt_simulation kms_addfb kms_cursor_crc kms_fbc_crc +kms_fence_pin_leak kms_flip kms_flip_tiling +kms_mmio_vs_cs_flip kms_pipe_crc_basic kms_plane kms_render kms_setmode +kms_sink_crc_basic multi-tests.txt pm_lpsp -pm_rpm pm_psr pm_rc6_residency +pm_rpm pm_rps prime_nv_api prime_nv_pcopy diff --git a/tools/null_state_gen/.gitignore b/tools/null_state_gen/.gitignore new file mode 100644 index 000..170bcef --- /dev/null +++ b/tools/null_state_gen/.gitignore @@ -0,0 +1 @@ +intel_null_state_gen -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [i-g-t 6/7] lib: various documentation fixes
Fix some documentation comments and mark some struct members private. Signed-off-by: Thomas Wood --- lib/igt_aux.c | 5 ++--- lib/igt_core.c | 10 +- lib/igt_kms.h | 2 ++ lib/intel_batchbuffer.h | 5 + 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/igt_aux.c b/lib/igt_aux.c index c0088d5..7b277be 100644 --- a/lib/igt_aux.c +++ b/lib/igt_aux.c @@ -430,10 +430,9 @@ bool igt_setup_runtime_pm(void) } /** - * igt_runtime_pm_status: + * igt_get_runtime_pm_status: * - * Returns: - * The current runtime PM status. + * Returns: The current runtime PM status. */ enum igt_runtime_pm_status igt_get_runtime_pm_status(void) { diff --git a/lib/igt_core.c b/lib/igt_core.c index 56eacf2..7ac7ebe 100644 --- a/lib/igt_core.c +++ b/lib/igt_core.c @@ -1031,11 +1031,11 @@ static void fatal_sig_handler(int sig) * @fn: exit handler function * * Set a handler that will be called either when the process calls exit() or - * returns from the main function, or one of the signals in 'handled_signals' - * is raised. MAX_EXIT_HANDLERS handlers can be installed, each of which will - * be called only once, even if a subsequent signal is raised. If the exit - * handlers are called due to a signal, the signal will be re-raised with the - * original signal disposition after all handlers returned. + * returns from the main function, or one of the signals in + * 'handled_signals' is raised. MAX_EXIT_HANDLERS handlers can be installed, + * each of which will be called only once, even if a subsequent signal is + * raised. If the exit handlers are called due to a signal, the signal will be + * re-raised with the original signal disposition after all handlers returned. * * The handler will be passed the signal number if called due to a signal, or * 0 otherwise. Exit handlers can also be used from test children spawned with diff --git a/lib/igt_kms.h b/lib/igt_kms.h index 8e80d4b..17bf0a2 100644 --- a/lib/igt_kms.h +++ b/lib/igt_kms.h @@ -99,6 +99,7 @@ typedef struct igt_pipe igt_pipe_t; typedef uint32_t igt_fixed_t; /* 16.16 fixed point */ typedef struct { + /*< private >*/ igt_pipe_t *pipe; int index; unsigned int is_primary : 1; @@ -127,6 +128,7 @@ struct igt_pipe { }; typedef struct { + /*< private >*/ igt_display_t *display; uint32_t id;/* KMS id */ struct kmstest_connector_config config; diff --git a/lib/intel_batchbuffer.h b/lib/intel_batchbuffer.h index 49dbcf0..3715161 100644 --- a/lib/intel_batchbuffer.h +++ b/lib/intel_batchbuffer.h @@ -204,14 +204,10 @@ void intel_copy_bo(struct intel_batchbuffer *batch, * @tiling: tiling mode bits * @data: pointer to the memory mapping of the buffer * @size: size of the buffer object - * @num_tiles: number of tiles of the buffer object * * This is a i-g-t buffer object wrapper structure which augments the baseline * libdrm buffer object with suitable data needed by the render copy and the * media fill functions. - * - * Note that @num_tiles is only used by gem_stress.c internally and can be - * ignored. */ struct igt_buf { drm_intel_bo *bo; @@ -219,6 +215,7 @@ struct igt_buf { uint32_t tiling; uint32_t *data; uint32_t size; +/*< private >*/ unsigned num_tiles; }; -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [i-g-t 1/7] README: update piglit instructions
Piglit now has a top level "piglit" command and the location of the tests can now be read from an environment variable. Signed-off-by: Thomas Wood --- README | 27 ++- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/README b/README index 2cfb5c5..cfa186d 100644 --- a/README +++ b/README @@ -34,32 +34,25 @@ tests/ git://anongit.freedesktop.org/piglit - and build it (no need to install anything). Then we need to link up the - i-g-t sources with piglit + There is no need to build and install piglit if it is only going to be + used for running i-g-t tests. + + Set the IGT_TEST_ROOT environment variable to point to the tests + directory or link up the i-g-t sources with piglit using a symlink: piglit-sources $ cd bin piglit-sources/bin $ ln $i-g-t-sources igt -s - To avoid some hassles with piglit's use of Waffle just disable it. Run - - piglit-sources $ cmake -D PIGLIT_USE_WAFFLE=OFF . - - With - - piglit-sources $ ccmake . - - you can check your configuration with a curses interface, too. - - The tests in the i-g-t sources need to have been built already. Then we - can run the testcases with (as usual as root, no other drm clients - running): + In both cases, the tests in the i-g-t sources need to have been built + already. Then we can run the testcases with (as usual as root, no other + drm clients running): - piglit-sources # ./piglit-run.py igt + piglit-sources # ./piglit run igt The testlist is built at runtime, so no need to update anything in piglit when adding new tests. See - piglit-sources $ ./piglit-run.py -h + piglit-sources $ ./piglit run -h for some useful options. -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [i-g-t 7/7] docs: add missing sections to intel-gpu-tools-docs.xml
Signed-off-by: Thomas Wood --- docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml | 4 1 file changed, 4 insertions(+) diff --git a/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml b/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml index dcfff33..96cf77f 100644 --- a/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml +++ b/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml @@ -15,6 +15,7 @@ Intel GPU Tools + @@ -22,9 +23,12 @@ + + + -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [i-g-t 4/7] docs: add the sections file
This file can contain custom changes to the control the documentation output and therefore should be included in the repository. Signed-off-by: Thomas Wood --- docs/reference/intel-gpu-tools/.gitignore | 1 - .../intel-gpu-tools/intel-gpu-tools-sections.txt | 378 + 2 files changed, 378 insertions(+), 1 deletion(-) create mode 100644 docs/reference/intel-gpu-tools/intel-gpu-tools-sections.txt diff --git a/docs/reference/intel-gpu-tools/.gitignore b/docs/reference/intel-gpu-tools/.gitignore index 9415974..9fadbfc 100644 --- a/docs/reference/intel-gpu-tools/.gitignore +++ b/docs/reference/intel-gpu-tools/.gitignore @@ -6,7 +6,6 @@ /intel-gpu-tools-decl-list.txt /intel-gpu-tools-decl.txt /intel-gpu-tools-overrides.txt -/intel-gpu-tools-sections.txt /intel-gpu-tools-undeclared.txt /intel-gpu-tools-undocumented.txt /intel-gpu-tools-unused.txt diff --git a/docs/reference/intel-gpu-tools/intel-gpu-tools-sections.txt b/docs/reference/intel-gpu-tools/intel-gpu-tools-sections.txt new file mode 100644 index 000..de6c76c --- /dev/null +++ b/docs/reference/intel-gpu-tools/intel-gpu-tools-sections.txt @@ -0,0 +1,378 @@ + +debug +DEBUG_PROTOCOL_VERSION +COMMUNICATION_OFFSET +COMMUNICATION_QWORD +STATE_EU_MSG +STATE_CPU_ACK +STATE_OFFSET +STATE_QWORD +TX_OFFSET +TX_QWORD +RX_OFFSET +RX_QWORD +grf +mrf +cr +sr +DWORD8 + + + +drmtest +mmap64 +ARRAY_SIZE +ALIGN +drm_get_card +drm_open_any +drm_open_any_render +gem_quiescent_gpu +do_or_die +do_ioctl + + + +igt_aux +igt_fork_signal_helper +igt_stop_signal_helper +igt_exchange_int +igt_permute_array +igt_progress +igt_check_boolean_env_var +igt_aub_dump_enabled +igt_init_aperture_trashers +igt_trash_aperture +igt_cleanup_aperture_trashers +igt_system_suspend_autoresume +igt_drop_root +igt_wait_for_keypress +igt_runtime_pm_status +igt_setup_runtime_pm +igt_get_runtime_pm_status +igt_wait_for_pm_status +intel_purge_vm_caches +intel_get_avail_ram_mb +intel_get_total_ram_mb +intel_get_total_swap_mb +intel_check_memory +CHECK_RAM +CHECK_SWAP + + + +igt_core +IGT_EXIT_TIMEOUT +IGT_EXIT_SKIP +IGT_EXIT_SUCCESS +igt_fixture +igt_subtest_init +igt_opt_handler_t +igt_subtest_init_parse_opts +igt_tokencat +igt_subtest +igt_subtest_f +igt_subtest_name +igt_only_list_subtests +igt_main +igt_simple_init +igt_simple_main +igt_skip +igt_success +igt_fail +igt_exit +igt_assert +igt_assert_f +igt_assert_cmpint +igt_require +igt_skip_on +igt_require_f +igt_skip_on_f +igt_fork +igt_waitchildren +igt_helper_process +igt_fork_helper +igt_wait_helper +igt_stop_helper +igt_exit_handler_t +igt_install_exit_handler +igt_enable_exit_handler +igt_disable_exit_handler +igt_run_in_simulation +SLOW_QUICK +igt_skip_on_simulation +igt_log_level +igt_log +igt_vlog +igt_debug +igt_info +igt_warn +igt_warn_on +igt_warn_on_f +igt_set_timeout + + + +igt_debugfs +igt_debugfs_open +igt_debugfs_fopen +igt_pipe_crc_t +igt_crc_t +intel_pipe_crc_source +igt_crc_is_null +igt_crc_equal +igt_crc_to_string +igt_require_pipe_crc +igt_pipe_crc_new +igt_pipe_crc_free +igt_pipe_crc_start +igt_pipe_crc_stop +igt_pipe_crc_get_crcs +igt_pipe_crc_collect_crc +DROP_UNBOUND +DROP_BOUND +DROP_RETIRE +DROP_ACTIVE +DROP_ALL +igt_drop_caches_set +igt_disable_prefault +igt_enable_prefault +igt_open_forcewake_handle +stop_ring_flags +igt_to_stop_ring_flag +igt_set_stop_rings +igt_get_stop_rings + + + +igt_fb +cairo_surface_t +cairo_t +igt_fb +igt_text_align +igt_create_fb_with_bo_size +igt_create_fb +igt_create_color_fb +igt_remove_fb +igt_get_cairo_ctx +igt_paint_color +igt_paint_color_alpha +igt_paint_color_gradient +igt_paint_test_pattern +igt_paint_image +igt_write_fb_to_png +igt_cairo_printf_line +igt_bpp_depth_to_drm_format +igt_drm_format_to_bpp +igt_format_str +igt_get_all_formats + + + +igt_kms +pipe +pipe_name +igt_plane +plane_name +sprite_name +port +port_name +kmstest_connector_config +kmstest_get_connector_default_mode +kmstest_get_connector_config +kmstest_free_connector_config +kmstest_dump_mode +kmstest_get_pipe_from_crtc_id +kmstest_pipe_str +kmstest_encoder_type_str +kmstest_connector_status_str +kmstest_connector_type_str +kmstest_set_connector_dpms +igt_display_t +igt_pipe_t +igt_fixed_t +igt_plane_t +igt_pipe +igt_output_t +igt_display +igt_set_vt_graphics_mode +igt_display_init +igt_display_fini +igt_display_commit +igt_display_get_n_pipes +igt_output_name +igt_output_get_mode +igt_output_set_pipe +igt_output_get_plane +igt_plane_set_fb +igt_plane_set_position +igt_wait_for_vblank +for_each_connected_output +PIPE_ANY +IGT_FIXED + + + +instdone +MAX_INSTDONE_BITS +instdone_bit +num_instdone_bits +init_instdone_definitions + + + +intel_batchbuffer +BATCH_SZ +BATCH_RESERVED +intel_batchbuffer +intel_batchbuffer_alloc +intel_batchbuffer_free +intel_batchbuffer_flush +intel_batchbuffer_flush_on_ring +intel_batchbuffer_flush_with_context +intel_batchbuffer_reset +intel_batchbuffer_data +intel_batchbuffer_emit_reloc +intel_batchbuffer_space +intel_batchbuffer_emit_dword +intel
Re: [Intel-gfx] [i-g-t 1/7] README: update piglit instructions
On Tue, Jun 10, 2014 at 03:30:51PM +0100, Thomas Wood wrote: > Piglit now has a top level "piglit" command and the location of the > tests can now be read from an environment variable. > > Signed-off-by: Thomas Wood > --- > README | 27 ++- > 1 file changed, 10 insertions(+), 17 deletions(-) > > diff --git a/README b/README > index 2cfb5c5..cfa186d 100644 > --- a/README > +++ b/README > @@ -34,32 +34,25 @@ tests/ > > git://anongit.freedesktop.org/piglit > > - and build it (no need to install anything). Then we need to link up the > - i-g-t sources with piglit > + There is no need to build and install piglit if it is only going to be > + used for running i-g-t tests. > + > + Set the IGT_TEST_ROOT environment variable to point to the tests > + directory or link up the i-g-t sources with piglit using a symlink: > > piglit-sources $ cd bin > piglit-sources/bin $ ln $i-g-t-sources igt -s > > - To avoid some hassles with piglit's use of Waffle just disable it. Run > - > - piglit-sources $ cmake -D PIGLIT_USE_WAFFLE=OFF . > - > - With > - > - piglit-sources $ ccmake . > - > - you can check your configuration with a curses interface, too. Hm, why did you drop this? I run into "lack of waffle" every time I try to use piglit ... Otherwise looks good. -Daniel > - > - The tests in the i-g-t sources need to have been built already. Then we > - can run the testcases with (as usual as root, no other drm clients > - running): > + In both cases, the tests in the i-g-t sources need to have been built > + already. Then we can run the testcases with (as usual as root, no other > + drm clients running): > > - piglit-sources # ./piglit-run.py igt > + piglit-sources # ./piglit run igt > > The testlist is built at runtime, so no need to update anything in > piglit when adding new tests. See > > - piglit-sources $ ./piglit-run.py -h > + piglit-sources $ ./piglit run -h > > for some useful options. > > -- > 1.9.3 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [i-g-t 3/7] README: update the section on modifying and rebuilding documentation
On Tue, Jun 10, 2014 at 03:30:53PM +0100, Thomas Wood wrote: > Signed-off-by: Thomas Wood > --- > README | 12 +--- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/README b/README > index cfa186d..5e98565 100644 > --- a/README > +++ b/README > @@ -108,16 +108,14 @@ docs/ > reference documenation in docs/reference/ You need to have the gtk doc > tools installed to generate this API documentation. > > - Note that the currrent gtk-docs integration sucks a bit wrt regenerating > - the html files. You need at least > + To regenerate the html files when updating documentation, use: > > $ make clean -C docs && make -C docs > > - to regenerate them on any change. If you've added/changed/removed a > - symbol or anything else that changes the overall structure or indexes, > - you need a full rebuild: > - > - $ git clean -dfx && ./autogen.sh --enable-gtk-doc && make -C docs This is still requried afaik when you add new .c/.h files with api docs in them. -Daniel > + If you've added/changed/removed a symbol or anything else that changes > + the overall structure or indexes, this needs to be reflected in > + intel-gpu-tools-sections.txt. Entirely new sections will also need to be > + added to intel-gpu-tools-docs.xml in the appropriate place. > > DEPENDENCIES > This is a non-exchaustive list of package dependencies required for > -- > 1.9.3 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [i-g-t 4/7] docs: add the sections file
On Tue, Jun 10, 2014 at 03:30:54PM +0100, Thomas Wood wrote: > This file can contain custom changes to the control the documentation > output and therefore should be included in the repository. > > Signed-off-by: Thomas Wood Doesn't that mean we need to update this when adding new symbols? Imo forcing autogeneration is better since you can control the order also by moving functions around in the .h files. Or what exactly is this for? -Daniel > --- > docs/reference/intel-gpu-tools/.gitignore | 1 - > .../intel-gpu-tools/intel-gpu-tools-sections.txt | 378 > + > 2 files changed, 378 insertions(+), 1 deletion(-) > create mode 100644 > docs/reference/intel-gpu-tools/intel-gpu-tools-sections.txt > > diff --git a/docs/reference/intel-gpu-tools/.gitignore > b/docs/reference/intel-gpu-tools/.gitignore > index 9415974..9fadbfc 100644 > --- a/docs/reference/intel-gpu-tools/.gitignore > +++ b/docs/reference/intel-gpu-tools/.gitignore > @@ -6,7 +6,6 @@ > /intel-gpu-tools-decl-list.txt > /intel-gpu-tools-decl.txt > /intel-gpu-tools-overrides.txt > -/intel-gpu-tools-sections.txt > /intel-gpu-tools-undeclared.txt > /intel-gpu-tools-undocumented.txt > /intel-gpu-tools-unused.txt > diff --git a/docs/reference/intel-gpu-tools/intel-gpu-tools-sections.txt > b/docs/reference/intel-gpu-tools/intel-gpu-tools-sections.txt > new file mode 100644 > index 000..de6c76c > --- /dev/null > +++ b/docs/reference/intel-gpu-tools/intel-gpu-tools-sections.txt > @@ -0,0 +1,378 @@ > + > +debug > +DEBUG_PROTOCOL_VERSION > +COMMUNICATION_OFFSET > +COMMUNICATION_QWORD > +STATE_EU_MSG > +STATE_CPU_ACK > +STATE_OFFSET > +STATE_QWORD > +TX_OFFSET > +TX_QWORD > +RX_OFFSET > +RX_QWORD > +grf > +mrf > +cr > +sr > +DWORD8 > + > + > + > +drmtest > +mmap64 > +ARRAY_SIZE > +ALIGN > +drm_get_card > +drm_open_any > +drm_open_any_render > +gem_quiescent_gpu > +do_or_die > +do_ioctl > + > + > + > +igt_aux > +igt_fork_signal_helper > +igt_stop_signal_helper > +igt_exchange_int > +igt_permute_array > +igt_progress > +igt_check_boolean_env_var > +igt_aub_dump_enabled > +igt_init_aperture_trashers > +igt_trash_aperture > +igt_cleanup_aperture_trashers > +igt_system_suspend_autoresume > +igt_drop_root > +igt_wait_for_keypress > +igt_runtime_pm_status > +igt_setup_runtime_pm > +igt_get_runtime_pm_status > +igt_wait_for_pm_status > +intel_purge_vm_caches > +intel_get_avail_ram_mb > +intel_get_total_ram_mb > +intel_get_total_swap_mb > +intel_check_memory > +CHECK_RAM > +CHECK_SWAP > + > + > + > +igt_core > +IGT_EXIT_TIMEOUT > +IGT_EXIT_SKIP > +IGT_EXIT_SUCCESS > +igt_fixture > +igt_subtest_init > +igt_opt_handler_t > +igt_subtest_init_parse_opts > +igt_tokencat > +igt_subtest > +igt_subtest_f > +igt_subtest_name > +igt_only_list_subtests > +igt_main > +igt_simple_init > +igt_simple_main > +igt_skip > +igt_success > +igt_fail > +igt_exit > +igt_assert > +igt_assert_f > +igt_assert_cmpint > +igt_require > +igt_skip_on > +igt_require_f > +igt_skip_on_f > +igt_fork > +igt_waitchildren > +igt_helper_process > +igt_fork_helper > +igt_wait_helper > +igt_stop_helper > +igt_exit_handler_t > +igt_install_exit_handler > +igt_enable_exit_handler > +igt_disable_exit_handler > +igt_run_in_simulation > +SLOW_QUICK > +igt_skip_on_simulation > +igt_log_level > +igt_log > +igt_vlog > +igt_debug > +igt_info > +igt_warn > +igt_warn_on > +igt_warn_on_f > +igt_set_timeout > + > + > + > +igt_debugfs > +igt_debugfs_open > +igt_debugfs_fopen > +igt_pipe_crc_t > +igt_crc_t > +intel_pipe_crc_source > +igt_crc_is_null > +igt_crc_equal > +igt_crc_to_string > +igt_require_pipe_crc > +igt_pipe_crc_new > +igt_pipe_crc_free > +igt_pipe_crc_start > +igt_pipe_crc_stop > +igt_pipe_crc_get_crcs > +igt_pipe_crc_collect_crc > +DROP_UNBOUND > +DROP_BOUND > +DROP_RETIRE > +DROP_ACTIVE > +DROP_ALL > +igt_drop_caches_set > +igt_disable_prefault > +igt_enable_prefault > +igt_open_forcewake_handle > +stop_ring_flags > +igt_to_stop_ring_flag > +igt_set_stop_rings > +igt_get_stop_rings > + > + > + > +igt_fb > +cairo_surface_t > +cairo_t > +igt_fb > +igt_text_align > +igt_create_fb_with_bo_size > +igt_create_fb > +igt_create_color_fb > +igt_remove_fb > +igt_get_cairo_ctx > +igt_paint_color > +igt_paint_color_alpha > +igt_paint_color_gradient > +igt_paint_test_pattern > +igt_paint_image > +igt_write_fb_to_png > +igt_cairo_printf_line > +igt_bpp_depth_to_drm_format > +igt_drm_format_to_bpp > +igt_format_str > +igt_get_all_formats > + > + > + > +igt_kms > +pipe > +pipe_name > +igt_plane > +plane_name > +sprite_name > +port > +port_name > +kmstest_connector_config > +kmstest_get_connector_default_mode > +kmstest_get_connector_config > +kmstest_free_connector_config > +kmstest_dump_mode > +kmstest_get_pipe_from_crtc_id > +kmstest_pipe_str > +kmstest_encoder_type_str > +kmstest_connector_status_str > +kmstest_connector_type_str > +kmstest_set_connector_dpms > +igt_display_t > +igt_pipe_t > +igt_fixed_t > +igt_plane_t > +igt_pipe > +igt_output_t > +igt_display > +igt_
Re: [Intel-gfx] [PATCH] drm/i915: leave rc6 enabled at suspend time v4
On Tue, 2014-06-10 at 15:57 +0200, Daniel Vetter wrote: > On Tue, Jun 10, 2014 at 04:42:50PM +0300, Imre Deak wrote: > > On Wed, 2014-06-04 at 13:45 -0700, Jesse Barnes wrote: > > > This allows the system to enter the lowest power mode during system > > > freeze. > > > > > > v2: delete force wake timer at suspend (Imre) > > > v3: add GT work suspend function (Imre) > > > v4: use uncore forcewake reset (Daniel) > > > > > > Signed-off-by: Kristen Carlson Accardi > > > Signed-off-by: Jesse Barnes > > > --- > > > drivers/gpu/drm/i915/i915_drv.c | 4 ++-- > > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > > drivers/gpu/drm/i915/intel_drv.h| 1 + > > > drivers/gpu/drm/i915/intel_pm.c | 20 > > > drivers/gpu/drm/i915/intel_uncore.c | 2 +- > > > 5 files changed, 25 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > b/drivers/gpu/drm/i915/i915_drv.c > > > index 66c6ffb..7148eac 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -521,7 +521,7 @@ static int i915_drm_freeze(struct drm_device *dev) > > > drm_irq_uninstall(dev); > > > dev_priv->enable_hotplug_processing = false; > > > > > > - intel_disable_gt_powersave(dev); > > > + intel_suspend_gt_powersave(dev); > > > > I realized now that we actually do need to enable RC6 explicitly. If we > > get here right after runtime resume, the deferred RC6 enabling might be > > still pending and so RC6 might not be enabled. > > Hm, for runtime pm we might schedule the delayed rps work with a 0 delay, > just to get the gpu going faster. Just an aside. > > Also some runtime pm vs system suspend tests in igt would be good if we > don't have them yet. And if we have them some analysis why this didn't > blow up in testing (and something to address that gap if feasible). To clarify the above is about the new functionality to enable deeper system wide power states. Atm, we don't have a bug related to the above runtime resume->system suspend sequence, since we explicitly disable gt power saving/RC6. For deeper power states we have to do the opposite and leave RC6 enabled and that's what I commented on. --Imre signature.asc Description: This is a digitally signed message part ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [i-g-t 6/7] lib: various documentation fixes
On Tue, Jun 10, 2014 at 03:30:56PM +0100, Thomas Wood wrote: > Fix some documentation comments and mark some struct members private. > > Signed-off-by: Thomas Wood > --- > lib/igt_aux.c | 5 ++--- > lib/igt_core.c | 10 +- > lib/igt_kms.h | 2 ++ > lib/intel_batchbuffer.h | 5 + > 4 files changed, 10 insertions(+), 12 deletions(-) > > diff --git a/lib/igt_aux.c b/lib/igt_aux.c > index c0088d5..7b277be 100644 > --- a/lib/igt_aux.c > +++ b/lib/igt_aux.c > @@ -430,10 +430,9 @@ bool igt_setup_runtime_pm(void) > } > > /** > - * igt_runtime_pm_status: > + * igt_get_runtime_pm_status: > * > - * Returns: > - * The current runtime PM status. > + * Returns: The current runtime PM status. > */ > enum igt_runtime_pm_status igt_get_runtime_pm_status(void) > { > diff --git a/lib/igt_core.c b/lib/igt_core.c > index 56eacf2..7ac7ebe 100644 > --- a/lib/igt_core.c > +++ b/lib/igt_core.c > @@ -1031,11 +1031,11 @@ static void fatal_sig_handler(int sig) > * @fn: exit handler function > * > * Set a handler that will be called either when the process calls exit() or > - * returns from the main function, or one of the signals in 'handled_signals' > - * is raised. MAX_EXIT_HANDLERS handlers can be installed, each of which will > - * be called only once, even if a subsequent signal is raised. If the exit > - * handlers are called due to a signal, the signal will be re-raised with the > - * original signal disposition after all handlers returned. > + * returns from the main function, or one of the signals in Ugh, this is ugly. Maybe put a "prevent gtkdoc from parsing the returns" into the html comment block? > + * 'handled_signals' is raised. MAX_EXIT_HANDLERS handlers can be installed, > + * each of which will be called only once, even if a subsequent signal is > + * raised. If the exit handlers are called due to a signal, the signal will > be > + * re-raised with the original signal disposition after all handlers > returned. > * > * The handler will be passed the signal number if called due to a signal, or > * 0 otherwise. Exit handlers can also be used from test children spawned > with > diff --git a/lib/igt_kms.h b/lib/igt_kms.h > index 8e80d4b..17bf0a2 100644 > --- a/lib/igt_kms.h > +++ b/lib/igt_kms.h > @@ -99,6 +99,7 @@ typedef struct igt_pipe igt_pipe_t; > typedef uint32_t igt_fixed_t;/* 16.16 fixed point */ > > typedef struct { > + /*< private >*/ > igt_pipe_t *pipe; > int index; > unsigned int is_primary : 1; > @@ -127,6 +128,7 @@ struct igt_pipe { > }; > > typedef struct { > + /*< private >*/ > igt_display_t *display; > uint32_t id;/* KMS id */ > struct kmstest_connector_config config; > diff --git a/lib/intel_batchbuffer.h b/lib/intel_batchbuffer.h > index 49dbcf0..3715161 100644 > --- a/lib/intel_batchbuffer.h > +++ b/lib/intel_batchbuffer.h > @@ -204,14 +204,10 @@ void intel_copy_bo(struct intel_batchbuffer *batch, > * @tiling: tiling mode bits > * @data: pointer to the memory mapping of the buffer > * @size: size of the buffer object > - * @num_tiles: number of tiles of the buffer object > * > * This is a i-g-t buffer object wrapper structure which augments the > baseline > * libdrm buffer object with suitable data needed by the render copy and the > * media fill functions. > - * > - * Note that @num_tiles is only used by gem_stress.c internally and can be > - * ignored. > */ > struct igt_buf { > drm_intel_bo *bo; > @@ -219,6 +215,7 @@ struct igt_buf { > uint32_t tiling; > uint32_t *data; > uint32_t size; > +/*< private >*/ > unsigned num_tiles; > }; > > -- > 1.9.3 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915 : Removed the unconditional cross engine/ring update of MBOX registers
On Tue, 2014-06-10 at 07:24 +, Chris Wilson wrote: > On Tue, Jun 10, 2014 at 12:48:44PM +0530, sourab.gu...@intel.com wrote: > > From: Akash Goel > > > > Removed the unconditional cross engine/ring update of MBOX registers. > > The MBox update will done only when needed when the actual inter ring > > dependency has been ascertained. Although this late sync could affect > > the Media performance slightly but it shall improve the residency time > > of individual power wells in C6 state. > > NAK. Did you even consider the deadlocks above and beyond the issues with > latency? Maybe suggest that the hardware guys consider a reordering write > FIFO next time like elsewhere on the chip. > -Chris > Hi Chris, We had thought about the deadlock scenarios between two rings but it didn't seem plausible. Below scenario was considered: Let us say Ring1 has to sync for obj1 which is being processed on Ring2. So it inserts Wait command on Ring1 and corresponding Signal command on Ring2. Now, Ring1 will be deadlocked only in the case when Ring2 is waiting for some obj2 to be processed on Ring1. That would mean that Ring2 would have inserted a corresponding signal command on Ring1. Now, this signal command for obj2 on Ring1 has to be has to be there before the wait command for obj1 on Ring1 (because wait/signal command pair is inserted together). So, Ring2 should go ahead to process the Signal command for obj1. Are there any missing points in this scenario we considered? Or, some other scenario for deadlock? Thanks, Sourab ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [i-g-t 7/7] docs: add missing sections to intel-gpu-tools-docs.xml
On Tue, Jun 10, 2014 at 03:30:57PM +0100, Thomas Wood wrote: > Signed-off-by: Thomas Wood I've intentionally left these out since they're not really part of the core test library ... E.g. all public rendercopy functions are documented as part of the "intel batchbuffer" library. -Daniel > --- > docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml | 4 > 1 file changed, 4 insertions(+) > > diff --git a/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml > b/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml > index dcfff33..96cf77f 100644 > --- a/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml > +++ b/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml > @@ -15,6 +15,7 @@ > > > Intel GPU Tools > + > > > > @@ -22,9 +23,12 @@ > > > > + > > > > + > + > > > > -- > 1.9.3 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [i-g-t 1/7] README: update piglit instructions
On 10 June 2014 15:37, Daniel Vetter wrote: > On Tue, Jun 10, 2014 at 03:30:51PM +0100, Thomas Wood wrote: >> Piglit now has a top level "piglit" command and the location of the >> tests can now be read from an environment variable. >> >> Signed-off-by: Thomas Wood >> --- >> README | 27 ++- >> 1 file changed, 10 insertions(+), 17 deletions(-) >> >> diff --git a/README b/README >> index 2cfb5c5..cfa186d 100644 >> --- a/README >> +++ b/README >> @@ -34,32 +34,25 @@ tests/ >> >> git://anongit.freedesktop.org/piglit >> >> - and build it (no need to install anything). Then we need to link up the >> - i-g-t sources with piglit >> + There is no need to build and install piglit if it is only going to be >> + used for running i-g-t tests. >> + >> + Set the IGT_TEST_ROOT environment variable to point to the tests >> + directory or link up the i-g-t sources with piglit using a symlink: >> >> piglit-sources $ cd bin >> piglit-sources/bin $ ln $i-g-t-sources igt -s >> >> - To avoid some hassles with piglit's use of Waffle just disable it. Run >> - >> - piglit-sources $ cmake -D PIGLIT_USE_WAFFLE=OFF . >> - >> - With >> - >> - piglit-sources $ ccmake . >> - >> - you can check your configuration with a curses interface, too. > > Hm, why did you drop this? I run into "lack of waffle" every time I try to > use piglit ... Otherwise looks good. Since there's no need to actually build the Piglit test cases when running intel-gpu-tools tests, there's no requirement for waffle either. > -Daniel > >> - >> - The tests in the i-g-t sources need to have been built already. Then we >> - can run the testcases with (as usual as root, no other drm clients >> - running): >> + In both cases, the tests in the i-g-t sources need to have been built >> + already. Then we can run the testcases with (as usual as root, no other >> + drm clients running): >> >> - piglit-sources # ./piglit-run.py igt >> + piglit-sources # ./piglit run igt >> >> The testlist is built at runtime, so no need to update anything in >> piglit when adding new tests. See >> >> - piglit-sources $ ./piglit-run.py -h >> + piglit-sources $ ./piglit run -h >> >> for some useful options. >> >> -- >> 1.9.3 >> >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Update PSR on resume.
would you be ok by a patch that doesn't trigger the psr_update but just set psr.setup_done = false on resume? I don't want to do more setup than really needed. Or you mean move even psr.setup_done to crtc_enable? On Thu, Jun 5, 2014 at 2:15 AM, Daniel Vetter wrote: > On Wed, Jun 04, 2014 at 12:17:14PM -0700, Rodrigo Vivi wrote: > > On Wed, May 28, 2014 at 5:57 AM, Daniel Vetter wrote: > > > > > On Tue, May 27, 2014 at 04:50:14PM -0700, Rodrigo Vivi wrote: > > > > Some registers set during setup might not be persistent after > > > suspend/resume. > > > > This was causing bugs for some people that was unable to get PSR > entry > > > state > > > > after resume cycle. > > > > > > > > v2: Adding some comments and better commit message explaining why > this > > > is needed. > > > > > > > > Signed-off-by: Rodrigo Vivi > > > > --- > > > > drivers/gpu/drm/i915/i915_suspend.c | 6 ++ > > > > 1 file changed, 6 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_suspend.c > > > b/drivers/gpu/drm/i915/i915_suspend.c > > > > index 56785e8..1923708 100644 > > > > --- a/drivers/gpu/drm/i915/i915_suspend.c > > > > +++ b/drivers/gpu/drm/i915/i915_suspend.c > > > > @@ -288,6 +288,12 @@ static void i915_restore_display(struct > drm_device > > > *dev) > > > > I915_WRITE(PP_CONTROL, > dev_priv->regfile.savePP_CONTROL); > > > > } > > > > > > > > + /* Forcing a full init sequence after resume to make sure all > > > > + * registers are properly set. Some might not be persistent > after > > > > + * suspend/resume cycle. */ > > > > + dev_priv->psr.setup_done = false; > > > > + intel_edp_psr_update(dev); > > > > > > No, crtc_enable should take care of this. There's more places (like > after > > > runtime pm) where the hw has potentially lost all register contents, so > > > crtc_enabl is the right place for this. > > > -Daniel > > > > > > > crtc_enable takes care of the update, but not the setup part that is done > > only once... > > I do believe that only the setup_done = false is really needed here, but > > doesn't heart to trigger the update right here > > and fixes the bug... > > restore_display hurts. If there's some setup we need to do, we _really_ > need to do it in crtc_enable. Splattering setup code all over the code is > one of the prime sources of bugs we have wrt rpm, s/r and driver load. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Update PSR on resume.
On Tue, Jun 10, 2014 at 08:11:41AM -0700, Rodrigo Vivi wrote: > would you be ok by a patch that doesn't trigger the psr_update but just set > psr.setup_done = false on resume? > > I don't want to do more setup than really needed. > > Or you mean move even psr.setup_done to crtc_enable? Yeah, if there's some setup we need to do (e.g. figure out whether we can do psr or set up the hw) we should do it in crtc_enable. That's the only common function for modeset stuff shared between system resume and runtime resume, and we'll loose register values in both cases. -Daniel > > > On Thu, Jun 5, 2014 at 2:15 AM, Daniel Vetter wrote: > > > On Wed, Jun 04, 2014 at 12:17:14PM -0700, Rodrigo Vivi wrote: > > > On Wed, May 28, 2014 at 5:57 AM, Daniel Vetter wrote: > > > > > > > On Tue, May 27, 2014 at 04:50:14PM -0700, Rodrigo Vivi wrote: > > > > > Some registers set during setup might not be persistent after > > > > suspend/resume. > > > > > This was causing bugs for some people that was unable to get PSR > > entry > > > > state > > > > > after resume cycle. > > > > > > > > > > v2: Adding some comments and better commit message explaining why > > this > > > > is needed. > > > > > > > > > > Signed-off-by: Rodrigo Vivi > > > > > --- > > > > > drivers/gpu/drm/i915/i915_suspend.c | 6 ++ > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_suspend.c > > > > b/drivers/gpu/drm/i915/i915_suspend.c > > > > > index 56785e8..1923708 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_suspend.c > > > > > +++ b/drivers/gpu/drm/i915/i915_suspend.c > > > > > @@ -288,6 +288,12 @@ static void i915_restore_display(struct > > drm_device > > > > *dev) > > > > > I915_WRITE(PP_CONTROL, > > dev_priv->regfile.savePP_CONTROL); > > > > > } > > > > > > > > > > + /* Forcing a full init sequence after resume to make sure all > > > > > + * registers are properly set. Some might not be persistent > > after > > > > > + * suspend/resume cycle. */ > > > > > + dev_priv->psr.setup_done = false; > > > > > + intel_edp_psr_update(dev); > > > > > > > > No, crtc_enable should take care of this. There's more places (like > > after > > > > runtime pm) where the hw has potentially lost all register contents, so > > > > crtc_enabl is the right place for this. > > > > -Daniel > > > > > > > > > > crtc_enable takes care of the update, but not the setup part that is done > > > only once... > > > I do believe that only the setup_done = false is really needed here, but > > > doesn't heart to trigger the update right here > > > and fixes the bug... > > > > restore_display hurts. If there's some setup we need to do, we _really_ > > need to do it in crtc_enable. Splattering setup code all over the code is > > one of the prime sources of bugs we have wrt rpm, s/r and driver load. > > -Daniel > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > > > > > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: leave rc6 enabled at suspend time v4
On Tue, Jun 10, 2014 at 05:41:49PM +0300, Imre Deak wrote: > On Tue, 2014-06-10 at 15:57 +0200, Daniel Vetter wrote: > > On Tue, Jun 10, 2014 at 04:42:50PM +0300, Imre Deak wrote: > > > On Wed, 2014-06-04 at 13:45 -0700, Jesse Barnes wrote: > > > > This allows the system to enter the lowest power mode during system > > > > freeze. > > > > > > > > v2: delete force wake timer at suspend (Imre) > > > > v3: add GT work suspend function (Imre) > > > > v4: use uncore forcewake reset (Daniel) > > > > > > > > Signed-off-by: Kristen Carlson Accardi > > > > Signed-off-by: Jesse Barnes > > > > --- > > > > drivers/gpu/drm/i915/i915_drv.c | 4 ++-- > > > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > > > drivers/gpu/drm/i915/intel_drv.h| 1 + > > > > drivers/gpu/drm/i915/intel_pm.c | 20 > > > > drivers/gpu/drm/i915/intel_uncore.c | 2 +- > > > > 5 files changed, 25 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > > b/drivers/gpu/drm/i915/i915_drv.c > > > > index 66c6ffb..7148eac 100644 > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > @@ -521,7 +521,7 @@ static int i915_drm_freeze(struct drm_device *dev) > > > > drm_irq_uninstall(dev); > > > > dev_priv->enable_hotplug_processing = false; > > > > > > > > - intel_disable_gt_powersave(dev); > > > > + intel_suspend_gt_powersave(dev); > > > > > > I realized now that we actually do need to enable RC6 explicitly. If we > > > get here right after runtime resume, the deferred RC6 enabling might be > > > still pending and so RC6 might not be enabled. > > > > Hm, for runtime pm we might schedule the delayed rps work with a 0 delay, > > just to get the gpu going faster. Just an aside. > > > > Also some runtime pm vs system suspend tests in igt would be good if we > > don't have them yet. And if we have them some analysis why this didn't > > blow up in testing (and something to address that gap if feasible). > > To clarify the above is about the new functionality to enable deeper > system wide power states. Atm, we don't have a bug related to the above > runtime resume->system suspend sequence, since we explicitly disable gt > power saving/RC6. For deeper power states we have to do the opposite and > leave RC6 enabled and that's what I commented on. Well I just want to make sure that we have test coverage. If Jesse has run all the rpm tests with piglit run -t rpm and it didn't blow up we need to fix this gap. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/6] Cursor support with universal planes (v4)
Rebasing to the latest drm-intel-nightly and resending at the request of the reviewers. Daniel has already provided an r-b for the first five patches here, so #6 is the main functionality awaiting review. Matt Roper (6): drm: Refactor framebuffer creation to allow internal use (v2) drm: Refactor setplane to allow internal use (v3) drm: Support legacy cursor ioctls via universal planes when possible (v4) drm: Allow drivers to register cursor planes with crtc drm/i915: Add intel_crtc_cursor_set_obj() to set cursor buffer (v2) drm/i915: Switch to unified plane cursor handling (v4) drivers/gpu/drm/drm_crtc.c | 357 +-- drivers/gpu/drm/i915/intel_display.c | 170 + drivers/gpu/drm/i915/intel_drv.h | 1 - include/drm/drm_crtc.h | 6 +- 4 files changed, 390 insertions(+), 144 deletions(-) -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/6] drm: Refactor framebuffer creation to allow internal use (v2)
Refactor DRM framebuffer creation into a new function that returns a struct drm_framebuffer directly. The upcoming universal cursor support will want to create framebuffers internally to wrap cursor buffers, so we want to be able to share that framebuffer creation with the drm_mode_addfb2 ioctl handler. v2: Take struct drm_mode_fb_cmd2 parameter directly rather than void* Reviewed-by: Daniel Vetter Signed-off-by: Matt Roper --- drivers/gpu/drm/drm_crtc.c | 69 -- 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index fe94cc1..5a88267 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -41,6 +41,10 @@ #include "drm_crtc_internal.h" +static struct drm_framebuffer *add_framebuffer_internal(struct drm_device *dev, + struct drm_mode_fb_cmd2 *r, + struct drm_file *file_priv); + /** * drm_modeset_lock_all - take all modeset locks * @dev: drm device @@ -2827,56 +2831,38 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r) return 0; } -/** - * drm_mode_addfb2 - add an FB to the graphics configuration - * @dev: drm device for the ioctl - * @data: data pointer for the ioctl - * @file_priv: drm file for the ioctl call - * - * Add a new FB to the specified CRTC, given a user request with format. This is - * the 2nd version of the addfb ioctl, which supports multi-planar framebuffers - * and uses fourcc codes as pixel format specifiers. - * - * Called by the user via ioctl. - * - * Returns: - * Zero on success, errno on failure. - */ -int drm_mode_addfb2(struct drm_device *dev, - void *data, struct drm_file *file_priv) +static struct drm_framebuffer *add_framebuffer_internal(struct drm_device *dev, + struct drm_mode_fb_cmd2 *r, + struct drm_file *file_priv) { - struct drm_mode_fb_cmd2 *r = data; struct drm_mode_config *config = &dev->mode_config; struct drm_framebuffer *fb; int ret; - if (!drm_core_check_feature(dev, DRIVER_MODESET)) - return -EINVAL; - if (r->flags & ~DRM_MODE_FB_INTERLACED) { DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags); - return -EINVAL; + return ERR_PTR(-EINVAL); } if ((config->min_width > r->width) || (r->width > config->max_width)) { DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <= %d\n", r->width, config->min_width, config->max_width); - return -EINVAL; + return ERR_PTR(-EINVAL); } if ((config->min_height > r->height) || (r->height > config->max_height)) { DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d && <= %d\n", r->height, config->min_height, config->max_height); - return -EINVAL; + return ERR_PTR(-EINVAL); } ret = framebuffer_check(r); if (ret) - return ret; + return ERR_PTR(ret); fb = dev->mode_config.funcs->fb_create(dev, file_priv, r); if (IS_ERR(fb)) { DRM_DEBUG_KMS("could not create framebuffer\n"); - return PTR_ERR(fb); + return fb; } mutex_lock(&file_priv->fbs_lock); @@ -2885,8 +2871,37 @@ int drm_mode_addfb2(struct drm_device *dev, DRM_DEBUG_KMS("[FB:%d]\n", fb->base.id); mutex_unlock(&file_priv->fbs_lock); + return fb; +} - return ret; +/** + * drm_mode_addfb2 - add an FB to the graphics configuration + * @dev: drm device for the ioctl + * @data: data pointer for the ioctl + * @file_priv: drm file for the ioctl call + * + * Add a new FB to the specified CRTC, given a user request with format. This is + * the 2nd version of the addfb ioctl, which supports multi-planar framebuffers + * and uses fourcc codes as pixel format specifiers. + * + * Called by the user via ioctl. + * + * Returns: + * Zero on success, errno on failure. + */ +int drm_mode_addfb2(struct drm_device *dev, + void *data, struct drm_file *file_priv) +{ + struct drm_framebuffer *fb; + + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return -EINVAL; + + fb = add_framebuffer_internal(dev, data, file_priv); + if (IS_ERR(fb)) + return PTR_ERR(fb); + + return 0; } /** -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/6] drm: Support legacy cursor ioctls via universal planes when possible (v4)
If drivers support universal planes and have registered a cursor plane with the DRM core, we should use that universal plane support when handling legacy cursor ioctls. Drivers that transition to universal planes won't have to maintain separate legacy ioctl handling; drivers that don't transition to universal planes will continue to operate without any change to behavior. Note that there's a bit of a mismatch between the legacy cursor ioctls and the universal plane API's --- legacy ioctl's use driver buffer handles directly whereas the universal plane API takes drm_framebuffers. Since there's no way to recover the driver handle from a drm_framebuffer, we can implement legacy ioctl's in terms of universal plane interfaces, but cannot implement universal plane interfaces in terms of legacy ioctls. Specifically, there's no way to create a general cursor helper in the way we previously created a primary plane helper. It's important to land this patch before any patches that add universal cursor support to individual drivers so that drivers don't have to worry about juggling two different styles of reference counting for cursor buffers when userspace mixes and matches legacy and universal cursor calls. With this patch, a driver that switches to universal cursor support may assume that all cursor buffers are wrapped in a drm_framebuffer and can rely on framebuffer reference counting for all cursor operations. v4: - Add comments pointing out setplane_internal's reference-eating semantics. v3: - Drop drm_mode_rmfb() call that is no longer needed now that we're using setplane_internal(), which takes care of deref'ing the appropriate framebuffer. v2: - Use new add_framebuffer_internal() function to create framebuffer rather than trying to call directly into the ioctl interface and look up the handle returned. - Use new setplane_internal() function to update the cursor plane rather than calling through the ioctl interface. Note that since we're no longer looking up an fb_id, no extra reference will be taken here. - Grab extra reference to fb under lock in !BO case to avoid issues where racing userspace could cause the fb to be destroyed out from under us after we grab the fb pointer. Reviewed-by: Daniel Vetter Signed-off-by: Matt Roper --- drivers/gpu/drm/drm_crtc.c | 107 + include/drm/drm_crtc.h | 4 ++ 2 files changed, 111 insertions(+) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 27eae03..b5bce5b 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2288,6 +2288,10 @@ int drm_mode_setplane(struct drm_device *dev, void *data, crtc = obj_to_crtc(obj); } + /* +* setplane_internal will take care of deref'ing either the old or new +* framebuffer depending on success. +*/ return setplane_internal(crtc, plane, fb, plane_req->crtc_x, plane_req->crtc_y, plane_req->crtc_w, plane_req->crtc_h, @@ -2541,6 +2545,102 @@ out: return ret; } +/** + * drm_mode_cursor_universal - translate legacy cursor ioctl call into a + * universal plane handler call + * @crtc: crtc to update cursor for + * @req: data pointer for the ioctl + * @file_priv: drm file for the ioctl call + * + * Legacy cursor ioctl's work directly with driver buffer handles. To + * translate legacy ioctl calls into universal plane handler calls, we need to + * wrap the native buffer handle in a drm_framebuffer. + * + * Note that we assume any handle passed to the legacy ioctls was a 32-bit ARGB + * buffer with a pitch of 4*width; the universal plane interface should be used + * directly in cases where the hardware can support other buffer settings and + * userspace wants to make use of these capabilities. + * + * Returns: + * Zero on success, errno on failure. + */ +static int drm_mode_cursor_universal(struct drm_crtc *crtc, +struct drm_mode_cursor2 *req, +struct drm_file *file_priv) +{ + struct drm_device *dev = crtc->dev; + struct drm_framebuffer *fb = NULL; + struct drm_mode_fb_cmd2 fbreq = { + .width = req->width, + .height = req->height, + .pixel_format = DRM_FORMAT_ARGB, + .pitches = { req->width * 4 }, + .handles = { req->handle }, + }; + int32_t crtc_x, crtc_y; + uint32_t crtc_w = 0, crtc_h = 0; + uint32_t src_w = 0, src_h = 0; + int ret = 0; + + BUG_ON(!crtc->cursor); + + /* +* Obtain fb we'll be using (either new or existing) and take an extra +* reference to it if fb != null. setplane will take care of dropping +* the reference if the plane update fails. +*/ + if (req->flags & DRM_MODE_CURSOR_BO) { + if (req->han
[Intel-gfx] [PATCH 5/6] drm/i915: Add intel_crtc_cursor_set_obj() to set cursor buffer (v2)
Refactor cursor buffer setting such that the code to actually update the cursor lives in a new function, intel_crtc_cursor_set_obj(), and takes a GEM object as a parameter. The existing legacy cursor ioctl handler, intel_crtc_cursor_set() will now perform the userspace handle lookup and then call this new function. This refactoring is in preparation for the universal plane cursor support where we'll want to update the cursor with an actual GEM buffer object (obtained via drm_framebuffer) rather than a userspace handle. v2: Drop obvious kerneldoc and replace with note about function's reference consumption Reviewed-by: Daniel Vetter Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/intel_display.c | 42 ++-- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b5cbb28..1beeb2a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8089,21 +8089,26 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, intel_crtc->cursor_base = base; } -static int intel_crtc_cursor_set(struct drm_crtc *crtc, -struct drm_file *file, -uint32_t handle, -uint32_t width, uint32_t height) +/* + * intel_crtc_cursor_set_obj - Set cursor to specified GEM object + * + * Note that the object's reference will be consumed if the update fails. If + * the update succeeds, the reference of the old object (if any) will be + * consumed. + */ +static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, +struct drm_i915_gem_object *obj, +uint32_t width, uint32_t height) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct drm_i915_gem_object *obj; unsigned old_width; uint32_t addr; int ret; /* if we want to turn off the cursor ignore width and height */ - if (!handle) { + if (!obj) { DRM_DEBUG_KMS("cursor off\n"); addr = 0; obj = NULL; @@ -8119,12 +8124,8 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc, return -EINVAL; } - obj = to_intel_bo(drm_gem_object_lookup(dev, file, handle)); - if (&obj->base == NULL) - return -ENOENT; - if (obj->base.size < width * height * 4) { - DRM_DEBUG_KMS("buffer is to small\n"); + DRM_DEBUG_KMS("buffer is too small\n"); ret = -ENOMEM; goto fail; } @@ -8207,6 +8208,25 @@ fail: return ret; } +static int intel_crtc_cursor_set(struct drm_crtc *crtc, +struct drm_file *file, +uint32_t handle, +uint32_t width, uint32_t height) +{ + struct drm_device *dev = crtc->dev; + struct drm_i915_gem_object *obj; + + if (handle) { + obj = to_intel_bo(drm_gem_object_lookup(dev, file, handle)); + if (&obj->base == NULL) + return -ENOENT; + } else { + obj = NULL; + } + + return intel_crtc_cursor_set_obj(crtc, obj, width, height); +} + static int intel_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) { struct intel_crtc *intel_crtc = to_intel_crtc(crtc); -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] drm/i915: Switch to unified plane cursor handling (v4)
The DRM core will translate calls to legacy cursor ioctls into universal cursor calls automatically, so there's no need to maintain the legacy cursor support. This greatly simplifies the transition since we don't have to handle reference counting differently depending on which cursor interface was called. The aim here is to transition to the universal plane interface with minimal code change. There's a lot of cleanup that can be done (e.g., using state stored in crtc->cursor->fb rather than intel_crtc) that is left to future patches. v4: - Drop drm_gem_object_unreference() that is no longer needed now that we receive the GEM obj directly rather than looking up the ID. v3: - Pass cursor obj to intel_crtc_cursor_set_obj() if cursor fb changes, even if 'visible' is false. intel_crtc_cursor_set_obj() will notice that the cursor isn't visible and disable it properly, but we still need to get intel_crtc->cursor_addr set properly so that we behave properly if the cursor becomes visible again in the future without changing the cursor buffer (noted by Chris Wilson and verified via i-g-t kms_cursor_crc). - s/drm_plane_init/drm_universal_plane_init/. Due to type compatibility between enum and bool, everything actually works correctly with the wrong init call, except for the type of plane that gets exposed to userspace (it shows up as type 'primary' rather than type 'cursor'). v2: - Remove duplicate dimension checks on cursor - Drop explicit cursor disable from crtc destroy (fb & plane destruction will take care of that now) - Use DRM plane helper to check update parameters Cc: intel-gfx@lists.freedesktop.org Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/intel_display.c | 166 +-- drivers/gpu/drm/i915/intel_drv.h | 1 - 2 files changed, 118 insertions(+), 49 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1beeb2a..1880c18 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -68,6 +68,11 @@ static const uint32_t intel_primary_formats_gen4[] = { DRM_FORMAT_ABGR2101010, }; +/* Cursor formats */ +static const uint32_t intel_cursor_formats[] = { + DRM_FORMAT_ARGB, +}; + #define DIV_ROUND_CLOSEST_ULL(ll, d) \ ({ unsigned long long _tmp = (ll)+(d)/2; do_div(_tmp, d); _tmp; }) @@ -8044,8 +8049,8 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); int pipe = intel_crtc->pipe; - int x = intel_crtc->cursor_x; - int y = intel_crtc->cursor_y; + int x = crtc->cursor_x; + int y = crtc->cursor_y; u32 base = 0, pos = 0; if (on) @@ -8180,7 +8185,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, if (intel_crtc->cursor_bo) { if (!INTEL_INFO(dev)->cursor_needs_physical) i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo); - drm_gem_object_unreference(&intel_crtc->cursor_bo->base); } mutex_unlock(&dev->struct_mutex); @@ -8208,38 +8212,6 @@ fail: return ret; } -static int intel_crtc_cursor_set(struct drm_crtc *crtc, -struct drm_file *file, -uint32_t handle, -uint32_t width, uint32_t height) -{ - struct drm_device *dev = crtc->dev; - struct drm_i915_gem_object *obj; - - if (handle) { - obj = to_intel_bo(drm_gem_object_lookup(dev, file, handle)); - if (&obj->base == NULL) - return -ENOENT; - } else { - obj = NULL; - } - - return intel_crtc_cursor_set_obj(crtc, obj, width, height); -} - -static int intel_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) -{ - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - - intel_crtc->cursor_x = clamp_t(int, x, SHRT_MIN, SHRT_MAX); - intel_crtc->cursor_y = clamp_t(int, y, SHRT_MIN, SHRT_MAX); - - if (intel_crtc->active) - intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL); - - return 0; -} - static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue, uint32_t start, uint32_t size) { @@ -8885,8 +8857,6 @@ static void intel_crtc_destroy(struct drm_crtc *crtc) kfree(work); } - intel_crtc_cursor_set(crtc, NULL, 0, 0, 0); - drm_crtc_cleanup(crtc); kfree(intel_crtc); @@ -10942,8 +10912,6 @@ out_config: } static const struct drm_crtc_funcs intel_crtc_funcs = { - .cursor_set = intel_crtc_cursor_set, - .cursor_move = intel_crtc_cursor_move, .gamma_set = intel_crtc_gamma_set, .set_config = intel_cr
[Intel-gfx] [PATCH 4/6] drm: Allow drivers to register cursor planes with crtc
Universal plane support had placeholders for cursor planes, but didn't actually do anything with them. Save the cursor plane reference inside the crtc and update the cursor plane parameter from void* to drm_plane. Reviewed-by: Daniel Vetter Signed-off-by: Matt Roper --- drivers/gpu/drm/drm_crtc.c | 5 - include/drm/drm_crtc.h | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index b5bce5b..74ad72f 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -727,7 +727,7 @@ DEFINE_WW_CLASS(crtc_ww_class); */ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, struct drm_plane *primary, - void *cursor, + struct drm_plane *cursor, const struct drm_crtc_funcs *funcs) { struct drm_mode_config *config = &dev->mode_config; @@ -752,8 +752,11 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, config->num_crtc++; crtc->primary = primary; + crtc->cursor = cursor; if (primary) primary->possible_crtcs = 1 << drm_crtc_index(crtc); + if (cursor) + cursor->possible_crtcs = 1 << drm_crtc_index(crtc); out: drm_modeset_unlock_all(dev); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index b8c7a9a..4ee7e26 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -856,7 +856,7 @@ struct drm_prop_enum_list { extern int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, struct drm_plane *primary, -void *cursor, +struct drm_plane *cursor, const struct drm_crtc_funcs *funcs); extern int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/6] drm: Refactor setplane to allow internal use (v3)
Refactor DRM setplane code into a new setplane_internal() function that takes DRM objects directly as parameters rather than looking them up by ID. We'll use this in a future patch when we implement legacy cursor ioctls on top of the universal plane interface. v3: - Move integer overflow checking from setplane_internal to setplane ioctl. The upcoming legacy cursor support via universal planes needs to maintain current cursor ioctl semantics and not return error for these extreme values (found via intel-gpu-tools kms_cursor_crc test). v2: - Allow planes to be disabled without a valid crtc again (and add mention of this to setplane's kerneldoc, since it doesn't seem to be mentioned anywhere else). - Reformat some parameter line wrap Reviewed-by: Daniel Vetter Signed-off-by: Matt Roper --- drivers/gpu/drm/drm_crtc.c | 176 ++--- 1 file changed, 102 insertions(+), 74 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 5a88267..27eae03 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2122,45 +2122,32 @@ out: return ret; } -/** - * drm_mode_setplane - configure a plane's configuration - * @dev: DRM device - * @data: ioctl data* - * @file_priv: DRM file info +/* + * setplane_internal - setplane handler for internal callers * - * Set plane configuration, including placement, fb, scaling, and other factors. - * Or pass a NULL fb to disable. + * Note that we assume an extra reference has already been taken on fb. If the + * update fails, this reference will be dropped before return; if it succeeds, + * the previous framebuffer (if any) will be unreferenced instead. * - * Returns: - * Zero on success, errno on failure. + * src_{x,y,w,h} are provided in 16.16 fixed point format */ -int drm_mode_setplane(struct drm_device *dev, void *data, - struct drm_file *file_priv) +static int setplane_internal(struct drm_crtc *crtc, +struct drm_plane *plane, +struct drm_framebuffer *fb, +int32_t crtc_x, int32_t crtc_y, +uint32_t crtc_w, uint32_t crtc_h, +/* src_{x,y,w,h} values are 16.16 fixed point */ +uint32_t src_x, uint32_t src_y, +uint32_t src_w, uint32_t src_h) { - struct drm_mode_set_plane *plane_req = data; - struct drm_plane *plane; - struct drm_crtc *crtc; - struct drm_framebuffer *fb = NULL, *old_fb = NULL; + struct drm_device *dev = crtc->dev; + struct drm_framebuffer *old_fb = NULL; int ret = 0; unsigned int fb_width, fb_height; int i; - if (!drm_core_check_feature(dev, DRIVER_MODESET)) - return -EINVAL; - - /* -* First, find the plane, crtc, and fb objects. If not available, -* we don't bother to call the driver. -*/ - plane = drm_plane_find(dev, plane_req->plane_id); - if (!plane) { - DRM_DEBUG_KMS("Unknown plane ID %d\n", - plane_req->plane_id); - return -ENOENT; - } - /* No fb means shut it down */ - if (!plane_req->fb_id) { + if (!fb) { drm_modeset_lock_all(dev); old_fb = plane->fb; ret = plane->funcs->disable_plane(plane); @@ -2174,14 +2161,6 @@ int drm_mode_setplane(struct drm_device *dev, void *data, goto out; } - crtc = drm_crtc_find(dev, plane_req->crtc_id); - if (!crtc) { - DRM_DEBUG_KMS("Unknown crtc ID %d\n", - plane_req->crtc_id); - ret = -ENOENT; - goto out; - } - /* Check whether this plane is usable on this CRTC */ if (!(plane->possible_crtcs & drm_crtc_mask(crtc))) { DRM_DEBUG_KMS("Invalid crtc for plane\n"); @@ -2189,14 +2168,6 @@ int drm_mode_setplane(struct drm_device *dev, void *data, goto out; } - fb = drm_framebuffer_lookup(dev, plane_req->fb_id); - if (!fb) { - DRM_DEBUG_KMS("Unknown framebuffer ID %d\n", - plane_req->fb_id); - ret = -ENOENT; - goto out; - } - /* Check whether this plane supports the fb pixel format. */ for (i = 0; i < plane->format_count; i++) if (fb->pixel_format == plane->format_types[i]) @@ -2212,43 +2183,25 @@ int drm_mode_setplane(struct drm_device *dev, void *data, fb_height = fb->height << 16; /* Make sure source coordinates are inside the fb. */ - if (plane_req->src_w > fb_width || - plane_req->src_x > fb_width - plane_req->src_w || - plane_req->src_h > fb_height || - plane_req->src_y > fb_height - plane_req->src_h) {
Re: [Intel-gfx] [i-g-t 1/7] README: update piglit instructions
On Tue, Jun 10, 2014 at 04:06:17PM +0100, Thomas Wood wrote: > On 10 June 2014 15:37, Daniel Vetter wrote: > > On Tue, Jun 10, 2014 at 03:30:51PM +0100, Thomas Wood wrote: > >> Piglit now has a top level "piglit" command and the location of the > >> tests can now be read from an environment variable. > >> > >> Signed-off-by: Thomas Wood > >> --- > >> README | 27 ++- > >> 1 file changed, 10 insertions(+), 17 deletions(-) > >> > >> diff --git a/README b/README > >> index 2cfb5c5..cfa186d 100644 > >> --- a/README > >> +++ b/README > >> @@ -34,32 +34,25 @@ tests/ > >> > >> git://anongit.freedesktop.org/piglit > >> > >> - and build it (no need to install anything). Then we need to link up > >> the > >> - i-g-t sources with piglit > >> + There is no need to build and install piglit if it is only going to > >> be > >> + used for running i-g-t tests. > >> + > >> + Set the IGT_TEST_ROOT environment variable to point to the tests > >> + directory or link up the i-g-t sources with piglit using a symlink: > >> > >> piglit-sources $ cd bin > >> piglit-sources/bin $ ln $i-g-t-sources igt -s > >> > >> - To avoid some hassles with piglit's use of Waffle just disable it. > >> Run > >> - > >> - piglit-sources $ cmake -D PIGLIT_USE_WAFFLE=OFF . > >> - > >> - With > >> - > >> - piglit-sources $ ccmake . > >> - > >> - you can check your configuration with a curses interface, too. > > > > Hm, why did you drop this? I run into "lack of waffle" every time I try to > > use piglit ... Otherwise looks good. > > Since there's no need to actually build the Piglit test cases when > running intel-gpu-tools tests, there's no requirement for waffle > either. Oh right ;-) Can you please mention this for dummies like me? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/chv: WaDisablePwrmtrEvent:chv on CHV only
On Mon, 09 Jun 2014, Damien Lespiau wrote: > On Mon, Jun 09, 2014 at 10:06:49AM -0700, Tom.O'rou...@intel.com wrote: >> From: Tom O'Rourke >> >> In gen8_enable_rps, don't write CHV registers unless IS_CHERRYVIEW. >> >> Signed-off-by: Tom O'Rourke > > A lovely catch. Sadly gen8_enable_rps does not get called on chv, so the fix is wrong. BR, Jani. > > Reviewed-by: Damien Lespiau > > -- > Damien > >> --- >> drivers/gpu/drm/i915/intel_pm.c |6 -- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c >> b/drivers/gpu/drm/i915/intel_pm.c >> index d9c5918..3d3e402 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -3522,8 +3522,10 @@ static void gen8_enable_rps(struct drm_device *dev) >> I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10); >> >> /* WaDisablePwrmtrEvent:chv (pre-production hw) */ >> -I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ff); >> -I915_WRITE(0xA810, I915_READ(0xA810) & 0xff00); >> +if (IS_CHERRYVIEW(dev)) { >> +I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ff); >> +I915_WRITE(0xA810, I915_READ(0xA810) & 0xff00); >> +} >> >> /* 5: Enable RPS */ >> I915_WRITE(GEN6_RP_CONTROL, >> -- >> 1.7.9.5 >> >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/chv: WaDisablePwrmtrEvent:chv on CHV only
On Tue, 10 Jun 2014, Jani Nikula wrote: > On Mon, 09 Jun 2014, Damien Lespiau wrote: >> On Mon, Jun 09, 2014 at 10:06:49AM -0700, Tom.O'rou...@intel.com wrote: >>> From: Tom O'Rourke >>> >>> In gen8_enable_rps, don't write CHV registers unless IS_CHERRYVIEW. >>> >>> Signed-off-by: Tom O'Rourke >> >> A lovely catch. > > Sadly gen8_enable_rps does not get called on chv, so the fix is wrong. To elaborate, I think we need a patch dropping the wa altogether (which we can queue for 3.15 through stable because the change affects broadwell) and another patch, if needed, adding the wa in the chv specific function. Thanks, Jani. > > BR, > Jani. > >> >> Reviewed-by: Damien Lespiau >> >> -- >> Damien >> >>> --- >>> drivers/gpu/drm/i915/intel_pm.c |6 -- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_pm.c >>> b/drivers/gpu/drm/i915/intel_pm.c >>> index d9c5918..3d3e402 100644 >>> --- a/drivers/gpu/drm/i915/intel_pm.c >>> +++ b/drivers/gpu/drm/i915/intel_pm.c >>> @@ -3522,8 +3522,10 @@ static void gen8_enable_rps(struct drm_device *dev) >>> I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10); >>> >>> /* WaDisablePwrmtrEvent:chv (pre-production hw) */ >>> - I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ff); >>> - I915_WRITE(0xA810, I915_READ(0xA810) & 0xff00); >>> + if (IS_CHERRYVIEW(dev)) { >>> + I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ff); >>> + I915_WRITE(0xA810, I915_READ(0xA810) & 0xff00); >>> + } >>> >>> /* 5: Enable RPS */ >>> I915_WRITE(GEN6_RP_CONTROL, >>> -- >>> 1.7.9.5 >>> >>> ___ >>> Intel-gfx mailing list >>> Intel-gfx@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] DevSNB PCI Config Space Registers Reference
Hi, Does anyone have access to the Sandy Bridge 2011 Intel HD Graphics PCI Config space register reference? I cannot seem to find it anywhere in the PRM docs. There is only a reference to a register called MMAPA at office 14h in all DevSNB manuals. I do see PCI config space documentation for Ivy Bridge and Haswell Intel HD Graphics. Otherwise, pointing me in the right direction w/ regard to the drm-intel source code would also help. Thanks, Nick ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: add register and unregister functions for connectors
Hi On Thu, May 29, 2014 at 5:57 PM, Thomas Wood wrote: > Introduce generic functions to register and unregister connectors. This > provides a common place to add and remove associated user space > interfaces. > > Signed-off-by: Thomas Wood > --- > Documentation/DocBook/drm.tmpl| 6 +++--- > drivers/gpu/drm/armada/armada_output.c| 4 ++-- > drivers/gpu/drm/ast/ast_mode.c| 4 ++-- > drivers/gpu/drm/bridge/ptn3460.c | 2 +- > drivers/gpu/drm/drm_crtc.c| 30 > ++- > drivers/gpu/drm/drm_sysfs.c | 2 -- > drivers/gpu/drm/exynos/exynos_dp_core.c | 2 +- > drivers/gpu/drm/exynos/exynos_drm_connector.c | 6 +++--- > drivers/gpu/drm/exynos/exynos_drm_dpi.c | 4 ++-- > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 2 +- > drivers/gpu/drm/exynos/exynos_drm_vidi.c | 2 +- > drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- > drivers/gpu/drm/gma500/cdv_intel_crt.c| 4 ++-- > drivers/gpu/drm/gma500/cdv_intel_dp.c | 4 ++-- > drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 4 ++-- > drivers/gpu/drm/gma500/cdv_intel_lvds.c | 4 ++-- > drivers/gpu/drm/gma500/mdfld_dsi_output.c | 4 ++-- > drivers/gpu/drm/gma500/oaktrail_hdmi.c| 2 +- > drivers/gpu/drm/gma500/oaktrail_lvds.c| 2 +- > drivers/gpu/drm/gma500/psb_intel_lvds.c | 4 ++-- > drivers/gpu/drm/gma500/psb_intel_sdvo.c | 4 ++-- > drivers/gpu/drm/i915/intel_crt.c | 2 +- > drivers/gpu/drm/i915/intel_display.c | 2 +- > drivers/gpu/drm/i915/intel_dp.c | 4 ++-- > drivers/gpu/drm/i915/intel_dsi.c | 2 +- > drivers/gpu/drm/i915/intel_dvo.c | 2 +- > drivers/gpu/drm/i915/intel_hdmi.c | 2 +- > drivers/gpu/drm/i915/intel_lvds.c | 2 +- > drivers/gpu/drm/i915/intel_sdvo.c | 10 - > drivers/gpu/drm/i915/intel_tv.c | 2 +- > drivers/gpu/drm/mgag200/mgag200_mode.c| 2 +- > drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 4 ++-- > drivers/gpu/drm/nouveau/nouveau_connector.c | 4 ++-- > drivers/gpu/drm/omapdrm/omap_connector.c | 4 ++-- > drivers/gpu/drm/qxl/qxl_display.c | 4 ++-- > drivers/gpu/drm/radeon/radeon_connectors.c| 6 +++--- > drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 4 ++-- > drivers/gpu/drm/rcar-du/rcar_du_vgacon.c | 4 ++-- > drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 6 +++--- > drivers/gpu/drm/tegra/output.c| 4 ++-- > drivers/gpu/drm/tilcdc/tilcdc_panel.c | 2 +- > drivers/gpu/drm/tilcdc/tilcdc_slave.c | 2 +- > drivers/gpu/drm/tilcdc/tilcdc_tfp410.c| 2 +- > drivers/gpu/drm/udl/udl_connector.c | 4 ++-- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +- > drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 2 +- > drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 +- > drivers/staging/imx-drm/imx-drm-core.c| 6 +++--- You even caught imx.. and you removed the EXPORT_SYMBOL. So looks all good to me. I like that refactoring and I don't think we need an ACK from all driver authors. This is: Reviewed-by: David Herrmann Maybe Daniel or Dave can pick this up? Thanks David > include/drm/drm_crtc.h| 2 ++ > 49 files changed, 110 insertions(+), 82 deletions(-) > > diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl > index 00f1c25..0f96b25 100644 > --- a/Documentation/DocBook/drm.tmpl > +++ b/Documentation/DocBook/drm.tmpl > @@ -1574,7 +1574,7 @@ int max_width, max_height; >The connector is then registered with a call to >drm_connector_init with a pointer to the > connector >functions and a connector type, and exposed through sysfs with a > call to > - drm_sysfs_connector_add. > + drm_connector_register. > > >Supported connector types are > @@ -1732,7 +1732,7 @@ int max_width, max_height; > (drm_encoder_cleanup) and connectors > (drm_connector_cleanup). Furthermore, connectors > that have been added to sysfs must be removed by a call to > - drm_sysfs_connector_remove before calling > + drm_connector_unregister before calling > drm_connector_cleanup. > > > @@ -1777,7 +1777,7 @@ void intel_crt_init(struct drm_device *dev) > drm_encoder_helper_add(&intel_output->enc, &intel_crt_helper_funcs); > drm_connector_helper_add(connector, > &intel_crt_connector_helper_funcs); > > - drm_sysfs_connector_add(connector); > + drm_connector_register(connector); > }]]> > > In the example above (taken from the i915 driver), a CRTC, connector > and > diff --git a/drivers/gpu/drm/armada/armada_output.c > b/drivers/gpu/drm/armada/armada_output.c > index d685a54..abbc309 100644 > --- a
Re: [Intel-gfx] [PATCH v2 01/16] drm/i915: Keep vblank interrupts enabled while enabling/disabling planes
On Wed, Jun 04, 2014 at 11:30:47AM +0530, Arun Murthy wrote: > On Mon, May 26, 2014 at 7:26 PM, Daniel Vetter wrote: > > On Thu, May 22, 2014 at 05:48:06PM +0300, ville.syrj...@linux.intel.com > > wrote: > >> From: Ville Syrjälä > >> > >> Because of the upcoming vblank interrupt driven watermark update > >> mechanism we will have use for vblank interrupts during plane > >> enabling/disabling. So don't call drm_vblank_off() until planes > >> are off, and call drm_vblank_on() just before we start to enable > >> the planes. > > Since watermark and display control registers are double buffered > both of them get updated on next blank and hence in sync. > Can you let me know the need for vblank driven watermark updates? Watermark registers aren't double buffered. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/2] drm/i915: fix possible refcount leak when resetting forcewake
On Fri, 06 Jun 2014, Daniel Vetter wrote: > On Fri, Jun 6, 2014 at 10:44 PM, Imre Deak wrote: >> Let's say that forcewake timer is pending, holding the runtime pm ref. >> System suspend is called - it's not prevented by either this ref or the >> above autosuspend delay - in the suspend handler we eventually call >> force_wake_reset which cancels the timer, leaking the runtime pm ref. > > Hm, I indeed mixed things up. I guess the window is small with the > short timeout we have for the forcewake timer, but still the first > patch makes sense for -fixes. Jani? v2 of 1/2 pushed to -fixes, thanks for that patch and review. BR, Jani. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Reorder semaphore deadlock check
On Fri, 06 Jun 2014, Mika Kuoppala wrote: > Chris Wilson writes: > >> If a semaphore is waiting on another ring, which in turn happens to be >> waiting on the first ring, but that second semaphore has been signalled, >> we will be able to kick the second ring and so can treat the first ring >> as a valid WAIT and not as HUNG. >> >> v2: Be paranoid and cap the potential recursion depth whilst visiting >> the semaphore signallers. (Mika) >> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=54226 >> References: https://bugs.freedesktop.org/show_bug.cgi?id=75502 >> Signed-off-by: Chris Wilson >> Cc: Mika Kuoppala >> Cc: sta...@vger.kernel.org >> --- > > Reviewed-by: Mika Kuoppala Pushed to -fixes, thanks for the patch and review. BR, Jani. > >> drivers/gpu/drm/i915/i915_irq.c | 18 ++ >> drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- >> 2 files changed, 15 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c >> b/drivers/gpu/drm/i915/i915_irq.c >> index a702303eab08..2974698f8f27 100644 >> --- a/drivers/gpu/drm/i915/i915_irq.c >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> @@ -2831,10 +2831,14 @@ static int semaphore_passed(struct intel_engine_cs >> *ring) >> struct intel_engine_cs *signaller; >> u32 seqno, ctl; >> >> -ring->hangcheck.deadlock = true; >> +ring->hangcheck.deadlock++; >> >> signaller = semaphore_waits_for(ring, &seqno); >> -if (signaller == NULL || signaller->hangcheck.deadlock) >> +if (signaller == NULL) >> +return -1; >> + >> +/* Prevent pathological recursion due to driver bugs */ >> +if (signaller->hangcheck.deadlock >= I915_NUM_RINGS) >> return -1; >> >> /* cursory check for an unkickable deadlock */ >> @@ -2842,7 +2846,13 @@ static int semaphore_passed(struct intel_engine_cs >> *ring) >> if (ctl & RING_WAIT_SEMAPHORE && semaphore_passed(signaller) < 0) >> return -1; >> >> -return i915_seqno_passed(signaller->get_seqno(signaller, false), seqno); >> +if (i915_seqno_passed(signaller->get_seqno(signaller, false), seqno)) >> +return 1; >> + >> +if (signaller->hangcheck.deadlock) >> +return -1; >> + >> +return 0; >> } >> >> static void semaphore_clear_deadlocks(struct drm_i915_private *dev_priv) >> @@ -2851,7 +2861,7 @@ static void semaphore_clear_deadlocks(struct >> drm_i915_private *dev_priv) >> int i; >> >> for_each_ring(ring, dev_priv, i) >> -ring->hangcheck.deadlock = false; >> +ring->hangcheck.deadlock = 0; >> } >> >> static enum intel_ring_hangcheck_action >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h >> b/drivers/gpu/drm/i915/intel_ringbuffer.h >> index 24357c597b54..31c2aab8ad2c 100644 >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h >> @@ -55,7 +55,7 @@ struct intel_ring_hangcheck { >> u32 seqno; >> int score; >> enum intel_ring_hangcheck_action action; >> -bool deadlock; >> +int deadlock; >> }; >> >> struct intel_ringbuffer { >> -- >> 2.0.0 > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/debugfs: add a "force" file per connector
Hi On Thu, May 29, 2014 at 5:57 PM, Thomas Wood wrote: > Add a file to debugfs for each connector to enable modification of the > "force" connector attribute. This allows connectors to be enabled or > disabled for testing and debugging purposes. > > Signed-off-by: Thomas Wood > --- > drivers/gpu/drm/drm_crtc.c| 17 ++- > drivers/gpu/drm/drm_debugfs.c | 101 > ++ > include/drm/drmP.h| 11 + > include/drm/drm_crtc.h| 2 + > 4 files changed, 130 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 998663c..59a2784 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -841,6 +841,8 @@ int drm_connector_init(struct drm_device *dev, > drm_object_attach_property(&connector->base, > dev->mode_config.dpms_property, 0); > > + connector->debugfs_entry = NULL; > + > out_put: > if (ret) > drm_mode_object_put(dev, &connector->base); > @@ -891,7 +893,19 @@ EXPORT_SYMBOL(drm_connector_cleanup); > */ > int drm_connector_register(struct drm_connector *connector) > { > - return drm_sysfs_connector_add(connector); > + int ret; > + > + ret = drm_sysfs_connector_add(connector); > + if (ret != 0) nitpick: I've never seen "xy != 0" in DRM code, I think "if (ret)" or "if (ret < 0)" is what we usually use.. > + return ret; > + > + ret = drm_debugfs_connector_add(connector); > + if (ret != 0) { > + drm_sysfs_connector_remove(connector); > + return ret; > + } > + > + return 0; > } > EXPORT_SYMBOL(drm_connector_register); > > @@ -904,6 +918,7 @@ EXPORT_SYMBOL(drm_connector_register); > void drm_connector_unregister(struct drm_connector *connector) > { > drm_sysfs_connector_remove(connector); > + drm_debugfs_connector_remove(connector); > } > EXPORT_SYMBOL(drm_connector_unregister); > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > index b4b51d4..b57b614 100644 > --- a/drivers/gpu/drm/drm_debugfs.c > +++ b/drivers/gpu/drm/drm_debugfs.c > @@ -237,5 +237,106 @@ int drm_debugfs_cleanup(struct drm_minor *minor) > return 0; > } > > +static int connector_show(struct seq_file *m, void *data) > +{ > + struct drm_connector *connector = m->private; > + const char *status; > + > + switch (connector->force) { > + case DRM_FORCE_ON: > + status = "on\n"; > + break; > + > + case DRM_FORCE_ON_DIGITAL: > + status = "digital\n"; > + break; > + > + case DRM_FORCE_OFF: > + status = "off\n"; > + break; > + > + case DRM_FORCE_UNSPECIFIED: > + status = "unspecified\n"; > + break; > + > + default: > + return 0; > + } > + > + seq_puts(m, status); > + > + return 0; > +} > + > +static int connector_open(struct inode *inode, struct file *file) > +{ > + struct drm_connector *dev = inode->i_private; > + > + return single_open(file, connector_show, dev); > +} > + > +static ssize_t connector_write(struct file *file, const char __user *ubuf, > + size_t len, loff_t *offp) > +{ > + struct seq_file *m = file->private_data; > + struct drm_connector *connector = m->private; > + > + if (strncmp(ubuf, "on", len) == 0) That hits on "o" as strncmp("o", "on", 1) == 0. We really need a "startswith()" macro in the kernel.. same below. > + connector->force = DRM_FORCE_ON; > + else if (strncmp(ubuf, "digital", len) == 0) > + connector->force = DRM_FORCE_ON_DIGITAL; > + else if (strncmp(ubuf, "off", len) == 0) > + connector->force = DRM_FORCE_OFF; > + else if (strncmp(ubuf, "unspecified", len) == 0) > + connector->force = DRM_FORCE_UNSPECIFIED; > + else > + return -EINVAL; > + > + return len; > +} > + > +static const struct file_operations drm_connector_fops = { > + .owner = THIS_MODULE, > + .open = connector_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > + .write = connector_write > +}; > + > +int drm_debugfs_connector_add(struct drm_connector *connector) > +{ > + struct drm_minor *minor = connector->dev->primary; > + struct dentry *root, *ent; > + > + if (!minor->debugfs_root) > + return -1; > + > + root = debugfs_create_dir(drm_get_connector_name(connector), > + minor->debugfs_root); > + if (!root) > + return -ENOMEM; > + > + connector->debugfs_entry = root; > + > + /* force */ > + ent = debugfs_create_file("force", S_IRUGO, root, connector, > +
Re: [Intel-gfx] [PATCH] drm/i915: Init important ns2501 registers
Hi Ville, Either pipe can drive DVO just fine. Looks like it's using pipe A in your register dump, and all the registers look fine to me. Well, DPLL B VCO enable is off since we don't currently have a mechanism to kick pipe B into action during resume/load. In theory that would need to be enabled as well. Can you see if a simple 'intel_reg_write 0x6018 0xc08b' fixes the problem? Nope. I created a little script that wrote the previous data back into the chipset, but that did not cure the problem. The only register I could not write to was CACHEMODE (IIRC, this was the name intel_reg_dump gave). The plane pointers and cursor pointers were different, too, but that should not be critical. And if not, I'd like to see a diff of register dumps between working and non working setups. I afraid the S6010 is out of reach for the next three weeks, but I should have a complete register dump attached to my previous mail from yesterday night (or this morning, to be precise). I can now try on the R31, but I don't remember having seen anything like it there. It does *not* look like the flickering of the misaligned watermark registers - on the console it really looks like bad HSYNC on a TV (tearing across horizontal lines, and massive misalignments of the very first lines of the screen). Within X, it causes the screen to jump to the right by about 64(?)128(?) pixels. The problem does not disappear by using a different resolution or restarting X. It remains permanent until the next boot. Greetings, Thomas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i95: Initialize active ring->pid to -1
On Tue, 10 Jun 2014, Daniel Vetter wrote: > On Tue, Jun 10, 2014 at 12:09:29PM +0100, Chris Wilson wrote: >> Otherwise we print out spurious processes on unused rings in the error >> state. >> >> Signed-off-by: Chris Wilson >> Cc: sta...@vger.kernel.org > > Personally would have done that in the dumper code, not the recording code > - this here looks a bit inconsistent. Either way this is > Reviewed-by: Daniel Vetter Pushed to -fixes, thanks for the patch and review. BR, Jani. >> --- >> drivers/gpu/drm/i915/i915_gpu_error.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c >> b/drivers/gpu/drm/i915/i915_gpu_error.c >> index 87ec60e181a7..66cf41765bf9 100644 >> --- a/drivers/gpu/drm/i915/i915_gpu_error.c >> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c >> @@ -888,6 +888,8 @@ static void i915_gem_record_rings(struct drm_device *dev, >> for (i = 0; i < I915_NUM_RINGS; i++) { >> struct intel_engine_cs *ring = &dev_priv->ring[i]; >> >> +error->ring[i].pid = -1; >> + >> if (ring->dev == NULL) >> continue; >> >> @@ -895,7 +897,6 @@ static void i915_gem_record_rings(struct drm_device *dev, >> >> i915_record_ring_state(dev, ring, &error->ring[i]); >> >> -error->ring[i].pid = -1; >> request = i915_gem_find_active_request(ring); >> if (request) { >> /* We need to copy these to an anonymous buffer >> -- >> 2.0.0 >> >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: set backlight duty cycle after backlight enable for gen4
On Tue, 10 Jun 2014, Daniel Vetter wrote: > On Mon, Jun 09, 2014 at 06:24:34PM +0300, Jani Nikula wrote: >> For reasons I can't claim to fully understand gen4 seems to require >> backlight duty cycle setting after the backlight has been enabled, or >> else black screen follows. I don't have documentation for the correct >> sequence on gen4 either. Confirmed on Dell Latitude D630 and MacBook4,1. >> >> This fixes a regression introduced by >> commit b35684b8fa94e04f55fd38bf672b737741d2f9e2 >> Author: Jani Nikula >> Date: Thu Nov 14 12:13:41 2013 +0200 >> >> drm/i915: do full backlight setup at enable time >> >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=75791 >> Reported-and-tested-by: m...@lm7.fr >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79423 >> Reported-and-tested-by: Marc Milgram >> Cc: Stable # 3.14+ >> Signed-off-by: Jani Nikula > > Yeah, docs don't mention anything. But if it works it's fine, so > Acked-by: Daniel Vetter With that, pushed to -fixes. BR, Jani. > >> --- >> drivers/gpu/drm/i915/intel_panel.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_panel.c >> b/drivers/gpu/drm/i915/intel_panel.c >> index 5e6c888b4928..38a98570d10c 100644 >> --- a/drivers/gpu/drm/i915/intel_panel.c >> +++ b/drivers/gpu/drm/i915/intel_panel.c >> @@ -798,9 +798,6 @@ static void i965_enable_backlight(struct intel_connector >> *connector) >> ctl = freq << 16; >> I915_WRITE(BLC_PWM_CTL, ctl); >> >> -/* XXX: combine this into above write? */ >> -intel_panel_actually_set_backlight(connector, panel->backlight.level); >> - >> ctl2 = BLM_PIPE(pipe); >> if (panel->backlight.combination_mode) >> ctl2 |= BLM_COMBINATION_MODE; >> @@ -809,6 +806,8 @@ static void i965_enable_backlight(struct intel_connector >> *connector) >> I915_WRITE(BLC_PWM_CTL2, ctl2); >> POSTING_READ(BLC_PWM_CTL2); >> I915_WRITE(BLC_PWM_CTL2, ctl2 | BLM_PWM_ENABLE); >> + >> +intel_panel_actually_set_backlight(connector, panel->backlight.level); >> } >> >> static void vlv_enable_backlight(struct intel_connector *connector) >> -- >> 1.9.1 >> >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Avoid div-by-zero when pixel_multiplier is zero
On Tue, 10 Jun 2014, Daniel Vetter wrote: > On Mon, Jun 09, 2014 at 04:20:46PM +0300, ville.syrj...@linux.intel.com wrote: >> From: Ville Syrjälä >> >> On certain platforms pixel_multiplier is read out in >> .get_pipe_config(), but it also gets used to calculate the >> pixel clock in intel_sdvo_get_config(). If the pipe is disable >> but some SDVO outputs are active, we may end up dividing by zero >> in intel_sdvo_get_config(). >> >> To avoid the problem simply check for zero pixel_multiplier and skip >> the division. Another attempt at fixing this involved populating >> pixel_multiplier to 1 even for disabled pipes, but that triggered a >> WARN because SDVO_CMD_GET_CLOCK_RATE_MULT command failed and thus >> encoder_pixel_multiplier was left at zero and didn't match >> pipe_config->pixel_multiplier. >> >> The "divide by pixel_multiplier" operation got introduced here: >> commit 18442d08786472c63a0a80c27f92b033dffc26de >> Author: Ville Syrjälä >> Date: Fri Sep 13 16:00:08 2013 +0300 >> >> drm/i915: Fix port_clock and adjusted_mode.clock readout all over >> >> and it has caused a regression on certain machines since they would >> hit the div-by-zero during resume. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76520 >> Cc: # 3.13+ >> Tested-by: Tim Richardson >> Signed-off-by: Ville Syrjälä > > Reviewed-by: Daniel Vetter Pushed to -fixes, thanks for the patch and review. BR, Jani. > >> --- >> drivers/gpu/drm/i915/intel_sdvo.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c >> b/drivers/gpu/drm/i915/intel_sdvo.c >> index 6a4d5bc..20375cc 100644 >> --- a/drivers/gpu/drm/i915/intel_sdvo.c >> +++ b/drivers/gpu/drm/i915/intel_sdvo.c >> @@ -1385,7 +1385,9 @@ static void intel_sdvo_get_config(struct intel_encoder >> *encoder, >> >> SDVO_PORT_MULTIPLY_SHIFT) + 1; >> } >> >> -dotclock = pipe_config->port_clock / pipe_config->pixel_multiplier; >> +dotclock = pipe_config->port_clock; >> +if (pipe_config->pixel_multiplier) >> +dotclock /= pipe_config->pixel_multiplier; >> >> if (HAS_PCH_SPLIT(dev)) >> ironlake_check_encoder_dotclock(pipe_config, dotclock); >> -- >> 1.8.5.5 >> >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Disable FBC on Haswell
On Tue, 10 Jun 2014, Daniel Vetter wrote: > On Mon, Jun 09, 2014 at 10:06:29PM +0300, Jani Nikula wrote: >> On Mon, 09 Jun 2014, Ville Syrjälä wrote: >> > On Mon, Jun 09, 2014 at 09:12:18PM +0300, Jani Nikula wrote: >> >> On Fri, 06 Jun 2014, Chris Wilson wrote: >> >> > It causes black screen on bootup and is approximately 100x slower than >> >> > running with FBC disabled, so the GPU runs at a high frequency for much >> >> > longer - completely contrary to the power saving claims. It also still >> >> > has mutex deadlocks in multi-head scenarios, which can lead to a >> >> > system/X lockup. These bugs were known before FBC was enabled by default >> >> > on Haswell and still have not been fixed. >> >> > >> >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79716 >> >> > Reported-and-tested-by: Jon Kristensen >> >> > Signed-off-by: Chris Wilson >> >> > Cc: sta...@vger.kernel.org >> >> > --- >> >> > drivers/gpu/drm/i915/intel_pm.c | 3 +-- >> >> > 1 file changed, 1 insertion(+), 2 deletions(-) >> >> > >> >> > diff --git a/drivers/gpu/drm/i915/intel_pm.c >> >> > b/drivers/gpu/drm/i915/intel_pm.c >> >> > index e403010540a5..0b8a6010427e 100644 >> >> > --- a/drivers/gpu/drm/i915/intel_pm.c >> >> > +++ b/drivers/gpu/drm/i915/intel_pm.c >> >> > @@ -511,8 +511,7 @@ void intel_update_fbc(struct drm_device *dev) >> >> > obj = intel_fb->obj; >> >> > adjusted_mode = &intel_crtc->config.adjusted_mode; >> >> > >> >> > - if (i915.enable_fbc < 0 && >> >> > - INTEL_INFO(dev)->gen <= 7 && !IS_HASWELL(dev)) { >> >> > + if (i915.enable_fbc < 0) { >> >> >> >> Not only does this disable FBC by default on Haswell but also on all >> >> current and future platforms, including Broadwell. Shouldn't you leave >> >> the INTEL_INFO(dev)->gen <= 7 part intact? >> > >> > The current FBC code is universally broken. >> >> Well, then the commit message should reflect that! :p > > s/Haswell/Haswell+/ and it's correct ;-) Pushed to -fixes with a little more verbose change. Thanks for the patch and review. BR, Jani. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/debugfs: add an "edid_override" file per connector
Hi On Thu, May 29, 2014 at 5:57 PM, Thomas Wood wrote: > Add a file to debugfs for each connector that allows the edid data to be > overridden. > > Signed-off-by: Thomas Wood > --- > drivers/gpu/drm/drm_crtc.c | 4 +++ > drivers/gpu/drm/drm_debugfs.c | 56 > ++ > drivers/gpu/drm/drm_probe_helper.c | 9 +- > include/drm/drm_crtc.h | 1 + > 4 files changed, 69 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 59a2784..8543eac 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -3701,6 +3701,10 @@ int drm_mode_connector_update_edid_property(struct > drm_connector *connector, > struct drm_device *dev = connector->dev; > int ret, size; > > + /* ignore requests to set edid when overridden */ > + if (connector->override_edid) > + return 0; > + > if (connector->edid_blob_ptr) > drm_property_destroy_blob(dev, connector->edid_blob_ptr); > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > index b57b614..2c666ba 100644 > --- a/drivers/gpu/drm/drm_debugfs.c > +++ b/drivers/gpu/drm/drm_debugfs.c > @@ -35,6 +35,7 @@ > #include > #include > #include > +#include > > #if defined(CONFIG_DEBUG_FS) > > @@ -295,6 +296,55 @@ static ssize_t connector_write(struct file *file, const > char __user *ubuf, > return len; > } > > +static int edid_show(struct seq_file *m, void *data) > +{ > + struct drm_connector *connector = m->private; > + struct drm_property_blob *edid = connector->edid_blob_ptr; > + > + if (connector->override_edid && edid) > + seq_write(m, edid->data, edid->length); > + > + return 0; > +} > + > +static int edid_open(struct inode *inode, struct file *file) > +{ > + struct drm_connector *dev = inode->i_private; > + > + return single_open(file, edid_show, dev); > +} > + > +static ssize_t edid_write(struct file *file, const char __user *ubuf, > + size_t len, loff_t *offp) > +{ > + struct seq_file *m = file->private_data; > + struct drm_connector *connector = m->private; Please copy "ubuf" before accessing it.. that's a user-space buffer. > + > + if (len >= EDID_LENGTH) { What? So if you write any invalid EDID, you reset it? That's wrong.. You should return an error in those cases. > + drm_mode_connector_update_edid_property(connector, > + (struct edid *) ubuf); > + connector->override_edid = true; Please check the error-code of update_edid_property() before setting override_edid. You should also return that code if it is an error. Furthermore, you must force override_edid = false; before calling update_edid_property, otherwise writing twice to this file will discard the second write (because update_edid_property() will do nothing as override_edid is already true). Last but not least, you *must* do an overflow check here! > + } else { > + if (connector->override_edid) { > + connector->override_edid = false; > + drm_mode_connector_update_edid_property(connector, > + NULL); > + } > + } > + > + return len; Maybe you want something like this (untested!): char *buf; struct edid *edid; int ret; buf = memdup_user(ubuf, len); if (IS_ERR(buf)) return PTR_ERR(buf); edid = buf; connector->override_edid = false; if (len == 5 && !strcmp(buf, "reset")) { ret = drm_mode_connector_update_edid_property(connector, NULL); } else if (len < EDID_LENGTH || EDID_LENGTH * (1 + edid->extensions) > len) { ret = -EINVAL; } else { ret = drm_mode_connector_update_edid_property(connector, (struct edid*)buf); if (!ret) connector->override_edid = true; } kfree(buf); return ret ? : len; Thanks David > +} > + > +static const struct file_operations drm_edid_fops = { > + .owner = THIS_MODULE, > + .open = edid_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > + .write = edid_write > +}; > + > + > static const struct file_operations drm_connector_fops = { > .owner = THIS_MODULE, > .open = connector_open, > @@ -325,6 +375,12 @@ int drm_debugfs_connector_add(struct drm_connector > *connector) > if (!ent) > return -ENOMEM; > > + /* edid */ > + ent = debugfs_create_file("edid_override", S_IRUGO, root, connector, IRUGO? Shouldn't this be writable for root? > + &drm_edid_fops); > + if (!ent) > + return -ENOMEM; > + > return 0; > } > > diff --git a/drivers/gpu/drm/drm_probe_helper.c > b/drivers/gpu/drm/drm_probe_h
Re: [Intel-gfx] Sluggish performance after resume//Re: Bug Report - [Acer Aspire V5-122P] Unable to adjust screen brightness
On Tue, Jun 10, 2014 at 01:33:51PM +0800, Aaron Lu wrote: > +Ben Widawsky & Daniel Vetter > > On 06/09/2014 03:38 PM, Lewis Toohey wrote: > > On 3 June 2014 02:22, Aaron Lu wrote: > >> On 05/30/2014 09:12 PM, Lewis Toohey wrote: > >>> Aaron > >>> > >>> I am in the process of performing this bisection, however, I need a > >>> bit of advice. > >>> > >>> I have got a mix of results following suspend right through from (i) > >>> system reboot; (ii) "low graphics mode" error; (iii) restore but > >>> sluggish performance; and (iv) restore but *very* sluggish > >>> performance. > >> > >> Is the sluggish performance experienced with a GUI environment? > >> > >>> > >>> What should qualify as a bad commit? Anything other than a "perfect" > >>> restore? > >> > >> I think ii/iii/iv all qualify as bad, if there is no such problem > >> in previous kernels. And yes, a perfect restore is expected, assume > >> the system is able to do a perfect restore with old kernels. > >> > >> Thanks, > >> Aaron > >> > >>> > >>> Many thanks > >>> > > > > Hi Aaron > > > > Firstly, yes the sluggish performance I was referring to is > > experienced in the GUI environment. Jerky graphics and CPU fan appears > > to max out and stay there. Old kernels (e.g. the Ubuntu default > > kernel) do not do this and just restore perfectly. > > > > I have completed the bisect as requested. Please find the full log > > below. I am slightly unconvinced, as building the previous commit in > > the log still seems to have the same problem, however, that commit is > > a "merge" and I don't really know what this means. > > > > Many thanks > > > > Bisect Log: > > git bisect start > > # good: [455c6fdbd219161bd09b1165f11699d6d73de11c] Linux 3.14 > > git bisect good 455c6fdbd219161bd09b1165f11699d6d73de11c > > # bad: [c9eaa447e77efe77b7fa4c953bd62de8297fd6c5] Linux 3.15-rc1 > > git bisect bad c9eaa447e77efe77b7fa4c953bd62de8297fd6c5 > > # good: [cd6362befe4cc7bf589a5236d2a780af2d47bcc9] Merge > > git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next > > git bisect good cd6362befe4cc7bf589a5236d2a780af2d47bcc9 > > # good: [d2b150d0647e055d7a71b1c33140280550b27dd6] Merge tag 'sh-3.15' > > of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc > > git bisect good d2b150d0647e055d7a71b1c33140280550b27dd6 > > # good: [5fb6b953bb7aa86a9c8ea760934982cedc45c52b] > > include/linux/syscalls.h: add sys_renameat2() prototype > > git bisect good 5fb6b953bb7aa86a9c8ea760934982cedc45c52b > > # bad: [ffddc5fd19b219f557fd4a81168ce8784a4faced] fs/ncpfs/dir.c: fix > > indenting in ncp_lookup() > > git bisect bad ffddc5fd19b219f557fd4a81168ce8784a4faced > > # bad: [978c6050165bba52eab7ef3581d447eb215def77] Merge branch > > 'drm-docs' of ssh://people.freedesktop.org/~danvet/drm into drm-next > > git bisect bad 978c6050165bba52eab7ef3581d447eb215def77 > > # bad: [262de1453184f65e5ccfe45790f93d41f7339d49] drm/i915: Directly > > return the vma from bind_to_vm > > git bisect bad 262de1453184f65e5ccfe45790f93d41f7339d49 > > # bad: [031994ee8dedfa69d3a7caa43e93f3c282bc38f9] drm/i915: Implement > > WaIncreaseL3CreditsForVLVB0:vlv > > git bisect bad 031994ee8dedfa69d3a7caa43e93f3c282bc38f9 > > # bad: [f72d21eddfa900bfa2674195dcc0203e18d0cc62] drm/i915: Place the > > Global GTT VM first in the list of VM > > git bisect bad f72d21eddfa900bfa2674195dcc0203e18d0cc62 > > # bad: [d6660add648d10e7e35085d8c7d2653e0f9f61b7] drm/i915: Generalize > > PPGTT init > > git bisect bad d6660add648d10e7e35085d8c7d2653e0f9f61b7 > > # bad: [b731d33d05dd5ce6b387cbadb0d9d24cb3732b40] drm/i915: relax > > context alignment > > git bisect bad b731d33d05dd5ce6b387cbadb0d9d24cb3732b40 > > # bad: [a7b910789f77afa40ae0816d22339e9d25723c6e] drm/i915: Add vm to > > error BO capture > > git bisect bad a7b910789f77afa40ae0816d22339e9d25723c6e > > # bad: [6e164c3382314a1f63526fa7a4322a17318d0e32] drm/i915: Allow ggtt > > lookups to not WARN > > git bisect bad 6e164c3382314a1f63526fa7a4322a17318d0e32 > > # bad: [6f425321e02a1b6c5e90b70f8fab7c140fcaeefb] drm/i915: Don't > > unconditionally try to deref aliasing ppgtt > > git bisect bad 6f425321e02a1b6c5e90b70f8fab7c140fcaeefb > > # bad: [e178f7057b81c87a7ceaae0ca204487b6f7eedcf] drm/i915: Provide > > PDP updates via MMIO > > git bisect bad e178f7057b81c87a7ceaae0ca204487b6f7eedcf > > # first bad commit: [e178f7057b81c87a7ceaae0ca204487b6f7eedcf] > > drm/i915: Provide PDP updates via MMIO > > > > The commit looks like related, I've added the commit author. > > Ben, > Do you have any suggestions? Does the above commit have any chance of > causing sluggish performance problem after a resume? > > Thanks, > Aaron What this comment actually does is use MMIO writes for the page tables after a GPU hang/reset. (What a poorly named commit message). Can you please provide the full dmesg with the drm.debug=0x2? -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedeskto
Re: [Intel-gfx] [PATCH] drm/i915/chv: WaDisablePwrmtrEvent:chv on CHV only
On Tue, Jun 10, 2014 at 06:34:18PM +0300, Jani Nikula wrote: > On Tue, 10 Jun 2014, Jani Nikula wrote: > > On Mon, 09 Jun 2014, Damien Lespiau wrote: > >> On Mon, Jun 09, 2014 at 10:06:49AM -0700, Tom.O'rou...@intel.com wrote: > >>> From: Tom O'Rourke > >>> > >>> In gen8_enable_rps, don't write CHV registers unless IS_CHERRYVIEW. > >>> > >>> Signed-off-by: Tom O'Rourke > >> > >> A lovely catch. > > > > Sadly gen8_enable_rps does not get called on chv, so the fix is wrong. > > To elaborate, I think we need a patch dropping the wa altogether (which > we can queue for 3.15 through stable because the change affects > broadwell) and another patch, if needed, adding the wa in the chv > specific function. This is just a merge mishap in one the chv patches. Someone just needs to send a patch that moves the misapplied stuff to the appropriate chv function. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx