Re: [Intel-gfx] [PATCH 10/10] drm/i915: Use plane_name() in gen7_enable_fbc()
On Wed, Nov 20, 2013 at 03:01:03PM -0800, Rodrigo Vivi wrote: > Reviewed-by: Rodrigo Vivi > > On Wed, Nov 06, 2013 at 11:02:25PM +0200, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > All the other .enable_fbc() funcs use plane_name(). Make > > gen7_enable_fbc() do the same. > > > > Signed-off-by: Ville Syrjälä I've picked up a few of the reviewed patches here. Overall we still have the issue that enabling the fbc tracking totally wreaks havoc with the blitter performance on gen6+ ... I still think the way to fix that is to do s/w based tracking and forgo all the neat hw support. -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 9/9] drm/i915: Enable pipe gamma for sprites
On Tue, Nov 19, 2013 at 09:42:55AM -0800, Rodrigo Vivi wrote: > Reviewed-by: Rodrigo Vivi > > On Mon, Nov 18, 2013 at 6:32 PM, Rodrigo Vivi wrote: > > From: Ville Syrjälä > > > > We send the primary and cursor plane data through the gamma unit. > > In order to get matching output from sprites, also send the sprite > > data through the gamma unit. > > > > In the future we should add some properties to control this > > explicitly, and also add properties for the per-sprite gamma ramps > > what have you, but for now this seems like a reasonable thing to do. > > > > Signed-off-by: Ville Syrjälä > > Signed-off-by: Rodrigo Vivi Eventually we want testcases for this, but I think that's only really useful once we expose a "bypasss gamma" or similar property. And that is best done together with exposing all the other csc stuff I'd say. So merged. -Daniel > > --- > > drivers/gpu/drm/i915/i915_reg.h | 2 +- > > drivers/gpu/drm/i915/intel_sprite.c | 18 ++ > > 2 files changed, 19 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index 8f4916d..7b454d2 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -3745,7 +3745,7 @@ > > > > #define _SPACNTR (VLV_DISPLAY_BASE + 0x72180) > > #define SP_ENABLE(1<<31) > > -#define SP_GEAMMA_ENABLE (1<<30) > > +#define SP_GAMMA_ENABLE (1<<30) > > #define SP_PIXFORMAT_MASK(0xf<<26) > > #define SP_FORMAT_YUV422 (0<<26) > > #define SP_FORMAT_BGR565 (5<<26) > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > > b/drivers/gpu/drm/i915/intel_sprite.c > > index 8afaad6..cb7ffd3 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -104,6 +104,12 @@ vlv_update_plane(struct drm_plane *dplane, struct > > drm_crtc *crtc, > > break; > > } > > > > + /* > > +* Enable gamma to match primary/cursor plane behaviour. > > +* FIXME should be user controllable via propertiesa. > > +*/ > > + sprctl |= SP_GAMMA_ENABLE; > > + > > if (obj->tiling_mode != I915_TILING_NONE) > > sprctl |= SP_TILED; > > > > @@ -257,6 +263,12 @@ ivb_update_plane(struct drm_plane *plane, struct > > drm_crtc *crtc, > > BUG(); > > } > > > > + /* > > +* Enable gamma to match primary/cursor plane behaviour. > > +* FIXME should be user controllable via propertiesa. > > +*/ > > + sprctl |= SPRITE_GAMMA_ENABLE; > > + > > if (obj->tiling_mode != I915_TILING_NONE) > > sprctl |= SPRITE_TILED; > > > > @@ -453,6 +465,12 @@ ilk_update_plane(struct drm_plane *plane, struct > > drm_crtc *crtc, > > BUG(); > > } > > > > + /* > > +* Enable gamma to match primary/cursor plane behaviour. > > +* FIXME should be user controllable via propertiesa. > > +*/ > > + dvscntr |= DVS_GAMMA_ENABLE; > > + > > if (obj->tiling_mode != I915_TILING_NONE) > > dvscntr |= DVS_TILED; > > > > -- > > 1.8.3.1 > > > > > > -- > 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 -- 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 01/10] drm/i915: Grab struct_mutex around all FBC updates
On Wed, Nov 20, 2013 at 11:39 PM, Rodrigo Vivi wrote: > On Wed, Nov 06, 2013 at 11:02:16PM +0200, ville.syrj...@linux.intel.com wrote: >> From: Ville Syrjälä >> >> We need some protection for the FBC state, and since struct_mutex >> is it currently in most places, make sure all FBC update/disable >> calles are protected by it. > > Why don't you create a new mutex only for fbc update? Yeah, if it's not core gem state please don't spread the usage of dev->struct_mutex. Eventually we need to slash that one into pieces, but until that happens making things worse doesn't help. -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 00/10] drm/i915: FBC fixes v2
(just noticed mutt behind tsocks missed this email) On Wed, Nov 6, 2013 at 1:02 PM, wrote: > One more attempt at making FBC suck a bit less. > > The main thing as before is getting the LRI based render/blitter > tracking in place. > > In this updates series I decided the way to avoid the kms locks in > execbuffer is to keep an extra reference to the fb. > > I added a bunch of locking (struct_mutex and crtc->mutex) to some > places to hopefully make it bit less racy. I think/hope that it doesn't > make it more likely to deadlock. We do grab both locks in the fbc work > now, and we do cancel the work while holding at least one of the locks, > so it seems a bit bad. But we cancel the work in a lazy fashion (no _sync > or anything) so I don't think it should deadlock due to that. And we > already grabbed struct_mutex there anyway, and we already held it while > cancelling the work, so it's no worse at least :P > > The disable_fbc/update_fbc calls from the page flip code are lacking > kms lock protection, but we can't simply add it there due to possibility > of deadlocks. > > So at least the render/blitter tracking seems to work now without > upsetting lockdep, so it seems a bit better than what we had at least. > Maybe someone else will get inspired by this and fix this mess up for > real... > > I've only run this on my IVB, and just running X + some apps w/o > a compositor. No extensive tests or anything. But if I frob some > debug register to disable some tracking bits the screen gets > corrupted, so it seems to be doing its job. > > I pushed it here in case someone is interested in a git tree: > git://gitorious.org/vsyrjala/linux.git fbc_fixes I did quick test here on my IVB as well and everything worked fine. I was afraid that this could regress 1 bug on gnome environment fixed by that lri but it didn't! :) Also I think this series fixes: https://bugs.freedesktop.org/show_bug.cgi?id=65396 And if I'm wrong about not any and all frontbuffer rend this also closes jira 3018. Besides that: Some time ago I started a fbc rework getting it tied to pipe_config, but than Daniel suggested that maybe it would be better to put fbc and psr in new plane_config you were doing instead of pipe_config. What do you think about it? And what the latest on plane_config? Thanks, Rodrigo. > > Ville Syrjälä (10): > drm/i915: Use plane_name() in gen7_enable_fbc() > drm/i915: Grab struct_mutex around all FBC updates > drm/i915: Have FBC keep references to the fb > drm/i915: Grab crtc->mutex in intel_fbc_work_fn() > drm/i915: Limit FBC flush to post batch flush > drm/i915: Emit SRM after the MSG_FBC_REND_STATE LRI > drm/i915: Implement LRI based FBC tracking > drm/i915: Kill sandybridge_blit_fbc_update() > drm/i915: Don't write ILK/IVB_FBC_RT_BASE directly > drm/i915: Set has_fbc=true for all SNB+, except VLV > > drivers/gpu/drm/i915/i915_drv.c| 6 +- > drivers/gpu/drm/i915/i915_drv.h| 6 +- > drivers/gpu/drm/i915/i915_gem_context.c| 7 +++ > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 31 ++ > drivers/gpu/drm/i915/i915_reg.h| 1 + > drivers/gpu/drm/i915/intel_display.c | 27 +++- > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_pm.c| 98 > -- > drivers/gpu/drm/i915/intel_ringbuffer.c| 66 ++-- > drivers/gpu/drm/i915/intel_ringbuffer.h| 2 + > 10 files changed, 188 insertions(+), 57 deletions(-) > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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 10/10] drm/i915: Use plane_name() in gen7_enable_fbc()
On Thu, Nov 21, 2013 at 09:08:44AM +0100, Daniel Vetter wrote: > On Wed, Nov 20, 2013 at 03:01:03PM -0800, Rodrigo Vivi wrote: > > Reviewed-by: Rodrigo Vivi > > > > On Wed, Nov 06, 2013 at 11:02:25PM +0200, ville.syrj...@linux.intel.com > > wrote: > > > From: Ville Syrjälä > > > > > > All the other .enable_fbc() funcs use plane_name(). Make > > > gen7_enable_fbc() do the same. > > > > > > Signed-off-by: Ville Syrjälä > > I've picked up a few of the reviewed patches here. Overall we still have > the issue that enabling the fbc tracking totally wreaks havoc with the > blitter performance on gen6+ ... I still think the way to fix that is to > do s/w based tracking and forgo all the neat hw support. For the record, I noticed that FBC was enabled on Haswell because of the dramatic impact it had on RENDER performance... -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 01/10] drm/i915: Grab struct_mutex around all FBC updates
On Thu, Nov 21, 2013 at 09:22:43AM +0100, Daniel Vetter wrote: > On Wed, Nov 20, 2013 at 11:39 PM, Rodrigo Vivi wrote: > > On Wed, Nov 06, 2013 at 11:02:16PM +0200, ville.syrj...@linux.intel.com > > wrote: > >> From: Ville Syrjälä > >> > >> We need some protection for the FBC state, and since struct_mutex > >> is it currently in most places, make sure all FBC update/disable > >> calles are protected by it. > > > > Why don't you create a new mutex only for fbc update? > > Yeah, if it's not core gem state please don't spread the usage of > dev->struct_mutex. Eventually we need to slash that one into pieces, > but until that happens making things worse doesn't help. I don't think I'm making things worse. struct_mutex is the lock that protects the fbc state currently, except we forgot to grab it in a some places. I'd rather fix this first, and then as a followup someone can start to think about using a new lock for fbc. -- 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 10/10] drm/i915: Use plane_name() in gen7_enable_fbc()
On Thu, Nov 21, 2013 at 09:08:44AM +0100, Daniel Vetter wrote: > On Wed, Nov 20, 2013 at 03:01:03PM -0800, Rodrigo Vivi wrote: > > Reviewed-by: Rodrigo Vivi > > > > On Wed, Nov 06, 2013 at 11:02:25PM +0200, ville.syrj...@linux.intel.com > > wrote: > > > From: Ville Syrjälä > > > > > > All the other .enable_fbc() funcs use plane_name(). Make > > > gen7_enable_fbc() do the same. > > > > > > Signed-off-by: Ville Syrjälä > > I've picked up a few of the reviewed patches here. Overall we still have > the issue that enabling the fbc tracking totally wreaks havoc with the > blitter performance on gen6+ ... I still think the way to fix that is to > do s/w based tracking and forgo all the neat hw support. Maybe we can do both. The tracking code is not that intrusive IMO, so giving the user a choice would make it easier to figure out if there is a performance vs. power tradeoff between hardware and software tracking. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 01/10] drm/i915: Grab struct_mutex around all FBC updates
From: Ville Syrjälä We need some protection for the FBC state, and since struct_mutex is it currently in most places, make sure all FBC update/disable calles are protected by it. v2: Also protect intel_disable_fbc in i9xx_crtc_disable Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e85d838..326ceca 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3600,8 +3600,10 @@ static void haswell_crtc_disable_planes(struct drm_crtc *crtc) drm_vblank_off(dev, pipe); /* FBC must be disabled before disabling the plane on HSW. */ + mutex_lock(&dev->struct_mutex); if (dev_priv->fbc.plane == plane) intel_disable_fbc(dev); + mutex_unlock(&dev->struct_mutex); hsw_disable_ips(intel_crtc); @@ -3741,8 +3743,10 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc) intel_crtc_wait_for_pending_flips(crtc); drm_vblank_off(dev, pipe); + mutex_lock(&dev->struct_mutex); if (dev_priv->fbc.plane == plane) intel_disable_fbc(dev); + mutex_unlock(&dev->struct_mutex); intel_crtc_update_cursor(crtc, false); intel_disable_planes(crtc); @@ -4126,7 +4130,9 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) intel_enable_planes(crtc); intel_crtc_update_cursor(crtc, true); + mutex_lock(&dev->struct_mutex); intel_update_fbc(dev); + mutex_unlock(&dev->struct_mutex); for_each_encoder_on_crtc(dev, crtc, encoder) encoder->enable(encoder); @@ -4170,7 +4176,9 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) /* Give the overlay scaler a chance to enable if it's on this pipe */ intel_crtc_dpms_overlay(intel_crtc, true); + mutex_lock(&dev->struct_mutex); intel_update_fbc(dev); + mutex_unlock(&dev->struct_mutex); for_each_encoder_on_crtc(dev, crtc, encoder) encoder->enable(encoder); @@ -4210,8 +4218,10 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) intel_crtc_wait_for_pending_flips(crtc); drm_vblank_off(dev, pipe); + mutex_lock(&dev->struct_mutex); if (dev_priv->fbc.plane == plane) intel_disable_fbc(dev); + mutex_unlock(&dev->struct_mutex); intel_crtc_dpms_overlay(intel_crtc, false); intel_crtc_update_cursor(crtc, false); @@ -4234,7 +4244,9 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) intel_crtc->active = false; intel_update_watermarks(crtc); + mutex_lock(&dev->struct_mutex); intel_update_fbc(dev); + mutex_unlock(&dev->struct_mutex); } static void i9xx_crtc_off(struct drm_crtc *crtc) -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 02/10] drm/i915: Have FBC keep references to the fb
From: Ville Syrjälä In order to do the FBC tracking properly for execbuffer, we need to figure out if the object being rendered to is the current front buffer. However in order to do that purely based on the crtc, we'd need to grab crtc->mutex, but we can't since we're already holding struct_mutex. So let's keep the current fb around in dev_priv->fbc.fb and consult that one. We're holding a reference to it for the entire time FBC is enabled, so there's no danger of it disappearing while we're looking at it. Assuming FBC updates always happens while holding struct_mutex that is. Also change the delayed enable mechanism to grab a reference to the fb. v2: Fix fb refcounting in intel_setup_fbc() Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.h | 6 ++-- drivers/gpu/drm/i915/intel_pm.c | 63 - 2 files changed, 47 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 14f250a..becc95f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -371,7 +371,9 @@ struct dpll; struct drm_i915_display_funcs { bool (*fbc_enabled)(struct drm_device *dev); - void (*enable_fbc)(struct drm_crtc *crtc, unsigned long interval); + void (*enable_fbc)(struct drm_crtc *crtc, + struct drm_framebuffer *fb, + unsigned long interval); void (*disable_fbc)(struct drm_device *dev); int (*get_display_clock_speed)(struct drm_device *dev); int (*get_fifo_size)(struct drm_device *dev, int plane); @@ -678,7 +680,7 @@ struct i915_hw_context { struct i915_fbc { unsigned long size; - unsigned int fb_id; + struct drm_framebuffer *fb; enum plane plane; int y; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 04e9863..2d26c93 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -86,11 +86,12 @@ static void i8xx_disable_fbc(struct drm_device *dev) DRM_DEBUG_KMS("disabled FBC\n"); } -static void i8xx_enable_fbc(struct drm_crtc *crtc, unsigned long interval) +static void i8xx_enable_fbc(struct drm_crtc *crtc, + struct drm_framebuffer *fb, + unsigned long interval) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct drm_framebuffer *fb = crtc->fb; struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); struct drm_i915_gem_object *obj = intel_fb->obj; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); @@ -136,11 +137,12 @@ static bool i8xx_fbc_enabled(struct drm_device *dev) return I915_READ(FBC_CONTROL) & FBC_CTL_EN; } -static void g4x_enable_fbc(struct drm_crtc *crtc, unsigned long interval) +static void g4x_enable_fbc(struct drm_crtc *crtc, + struct drm_framebuffer *fb, + unsigned long interval) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct drm_framebuffer *fb = crtc->fb; struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); struct drm_i915_gem_object *obj = intel_fb->obj; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); @@ -205,11 +207,12 @@ static void sandybridge_blit_fbc_update(struct drm_device *dev) gen6_gt_force_wake_put(dev_priv); } -static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval) +static void ironlake_enable_fbc(struct drm_crtc *crtc, + struct drm_framebuffer *fb, + unsigned long interval) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct drm_framebuffer *fb = crtc->fb; struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); struct drm_i915_gem_object *obj = intel_fb->obj; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); @@ -265,11 +268,12 @@ static bool ironlake_fbc_enabled(struct drm_device *dev) return I915_READ(ILK_DPFC_CONTROL) & DPFC_CTL_EN; } -static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval) +static void gen7_enable_fbc(struct drm_crtc *crtc, + struct drm_framebuffer *fb, + unsigned long interval) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct drm_framebuffer *fb = crtc->fb; struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); struct drm_i915_gem_object *obj = intel_fb->obj; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); @@ -308,6 +312,22 @@ bool intel_fbc_enabled(struct drm_device *dev) return dev_priv->display.fbc_enabled(
[Intel-gfx] [PATCH v4 06/10] drm/i915: Implement LRI based FBC tracking
From: Ville Syrjälä As per the SNB and HSW PM guides, we should enable FBC render/blitter tracking only during batches targetting the front buffer. On SNB we must also update the FBC render tracking address whenever it changes. And since the register in question is stored in the context, we need to make sure we reload it with correct data after context switches. On IVB/HSW we use the render nuke mechanism, so no render tracking address updates are needed. Hoever on the blitter side we need to enable the blitter tracking like on SNB, and in addition we need to issue the cache clean messages, which we already did. v2: Introduce intel_fb_obj_has_fbc() Fix crtc locking around crtc->fb access Drop a hunk that was included by accident in v1 Set fbc_address_dirty=false not true after emitting the LRI v3: Now that fbc hangs on to the fb intel_fb_obj_has_fbc() doesn't need to upset lockdep anymore v4: Use |= instead of = to update fbc_address_dirty Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_gem_context.c| 7 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 31 drivers/gpu/drm/i915/intel_display.c | 17 +++-- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_ringbuffer.c| 58 +- drivers/gpu/drm/i915/intel_ringbuffer.h| 2 ++ 6 files changed, 113 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 2ec122a..4b55471 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -402,6 +402,13 @@ mi_set_context(struct intel_ring_buffer *ring, intel_ring_advance(ring); + /* +* FBC RT address is stored in the context, so we may have just +* restored it to an old value. Make sure we emit a new LRI +* to update the address. +*/ + ring->fbc_address_dirty = true; + return ret; } diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 885d595..2d96edf 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -886,6 +886,35 @@ validate_exec_list(struct drm_i915_gem_exec_object2 *exec, } static void +i915_gem_execbuffer_mark_fbc_dirty(struct intel_ring_buffer *ring, + struct list_head *vmas) +{ + struct i915_vma *vma; + struct drm_i915_gem_object *fbc_obj = NULL; + u32 fbc_address = -1; + + list_for_each_entry(vma, vmas, exec_list) { + struct drm_i915_gem_object *obj = vma->obj; + + if (obj->base.pending_write_domain && + intel_fb_obj_has_fbc(obj)) { + WARN_ON(fbc_obj && fbc_obj != obj); + fbc_obj = obj; + } + } + + if (fbc_obj) + fbc_address = i915_gem_obj_ggtt_offset(fbc_obj); + + /* need to nuke/cache_clean on IVB+? */ + ring->fbc_dirty = fbc_obj != NULL; + + /* need to update FBC tracking? */ + ring->fbc_address_dirty |= fbc_address != ring->fbc_address; + ring->fbc_address = fbc_address; +} + +static void i915_gem_execbuffer_move_to_active(struct list_head *vmas, struct intel_ring_buffer *ring) { @@ -1150,6 +1179,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, if (flags & I915_DISPATCH_SECURE && !batch_obj->has_global_gtt_mapping) i915_gem_gtt_bind_object(batch_obj, batch_obj->cache_level); + i915_gem_execbuffer_mark_fbc_dirty(ring, &eb->vmas); + ret = i915_gem_execbuffer_move_to_gpu(ring, &eb->vmas); if (ret) goto err; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 326ceca..4155814 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8152,6 +8152,21 @@ void intel_mark_idle(struct drm_device *dev) gen6_rps_idle(dev->dev_private); } +bool intel_fb_obj_has_fbc(struct drm_i915_gem_object *obj) +{ + struct drm_device *dev = obj->base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + + /* check for potential scanout */ + if (!obj->pin_display) + return false; + + if (!dev_priv->fbc.fb) + return false; + + return to_intel_framebuffer(dev_priv->fbc.fb)->obj == obj; +} + void intel_mark_fb_busy(struct drm_i915_gem_object *obj, struct intel_ring_buffer *ring) { @@ -8169,8 +8184,6 @@ void intel_mark_fb_busy(struct drm_i915_gem_object *obj, continue; intel_increase_pllclock(crtc); - if (ring && intel_fbc_enabled(dev)) - ring->fbc_dirty = true; } } diff --git a/drivers/gpu/drm/i915/intel
Re: [Intel-gfx] [PATCH v4 06/10] drm/i915: Implement LRI based FBC tracking
On Thu, Nov 21, 2013 at 01:14:10PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > As per the SNB and HSW PM guides, we should enable FBC render/blitter > tracking only during batches targetting the front buffer. > > On SNB we must also update the FBC render tracking address whenever it > changes. And since the register in question is stored in the context, > we need to make sure we reload it with correct data after context > switches. > > On IVB/HSW we use the render nuke mechanism, so no render tracking > address updates are needed. Hoever on the blitter side we need to > enable the blitter tracking like on SNB, and in addition we need > to issue the cache clean messages, which we already did. > > v2: Introduce intel_fb_obj_has_fbc() > Fix crtc locking around crtc->fb access > Drop a hunk that was included by accident in v1 > Set fbc_address_dirty=false not true after emitting the LRI > v3: Now that fbc hangs on to the fb intel_fb_obj_has_fbc() doesn't > need to upset lockdep anymore > v4: Use |= instead of = to update fbc_address_dirty Ah, should we do the same for fbc_dirty? In the past we could dispatch a batchbuffer but fail to add the request (and so fail to flush the rendering/fbc). We currently preallocate the request so that failure path is history, but we will more than likely be caught out again in the future. Like pc8, I'd like for a device (or crtc if you must) property whether or not indirect rendering is preferred. Other than that and the bikeshed to kill the redundant fbc_obj local variable and pack the dirty bits, it looks good to me. -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 17/19] drm/i915: fix VDD override off wait
From: Paulo Zanoni If we're disabling the VDD override bit and the panel is enabled, we don't need to wait for anything. If the panel is disabled, then we need to actually wait for panel_power_cycle_delay, not panel_power_down_delay, because the power down delay was already respected when we disabled the panel. Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_dp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index c2a89b2..b438e76 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1142,7 +1142,9 @@ static void ironlake_panel_vdd_off_sync(struct intel_dp *intel_dp) /* Make sure sequencer is idle before allowing subsequent activity */ DRM_DEBUG_KMS("PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n", I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg)); - msleep(intel_dp->panel_power_down_delay); + + if ((pp & POWER_TARGET_ON) == 0) + msleep(intel_dp->panel_power_cycle_delay); intel_runtime_pm_put(dev_priv); } -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/19] drm/i915: use the correct force_wake function at the PC8 code
From: Paulo Zanoni When I submitted the first patch adding these force wake functions, Chris Wilson observed that I was using the wrong functions, so I sent a second version of the patch to correct this problem. The problem is that v1 was merged instead of v2. I was able to notice the problem when running the debugfs-forcewake-user subtest of pm_pc8 from intel-gpu-tools. Cc: sta...@vger.kernel.org Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_display.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5566de5..5ea32a8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6578,7 +6578,7 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv) /* Make sure we're not on PC8 state before disabling PC8, otherwise * we'll hang the machine! */ - dev_priv->uncore.funcs.force_wake_get(dev_priv); + gen6_gt_force_wake_get(dev_priv); if (val & LCPLL_POWER_DOWN_ALLOW) { val &= ~LCPLL_POWER_DOWN_ALLOW; @@ -6612,7 +6612,7 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv) DRM_ERROR("Switching back to LCPLL failed\n"); } - dev_priv->uncore.funcs.force_wake_put(dev_priv); + gen6_gt_force_wake_put(dev_priv); } void hsw_enable_pc8_work(struct work_struct *__work) -- 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] drm/i915/ddi: set sink to power down mode on dp disable
On Mon, Nov 18, 2013 at 02:45:47PM -0200, Paulo Zanoni wrote: > 2013/11/15 Jani Nikula : > > Similar to > > commit fdbc3b1f639bb2cbfb32c612b2699e0ba373317d > > Author: Jani Nikula > > Date: Tue Nov 12 17:10:13 2013 +0200 > > > > drm/i915/dp: set sink to power down mode on dp disable > > > > but for DDI, where we've never done this. > > > > Signed-off-by: Jani Nikula > > Makes sense, but I wonder if the correct place is this or > intel_disable_ddi(), right after the call to > ironlake_edp_backlight_off(). The current place is still better than > nowhere, so: Reviewed-by: Paulo Zanoni > > > > > --- > > > > UNTESTED! > > I'll leave your patch applied on my tree for now. If anything explodes > and I survive, I'll tell you :) Picked up for -fixes, thanks for the patch. -Daniel > > > > --- > > drivers/gpu/drm/i915/intel_ddi.c |3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > b/drivers/gpu/drm/i915/intel_ddi.c > > index 1591576..6a63a09 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1158,9 +1158,10 @@ static void intel_ddi_post_disable(struct > > intel_encoder *intel_encoder) > > if (wait) > > intel_wait_ddi_buf_idle(dev_priv, port); > > > > - if (type == INTEL_OUTPUT_EDP) { > > + if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) { > > struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > > ironlake_edp_panel_vdd_on(intel_dp); > > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); > > ironlake_edp_panel_off(intel_dp); > > } > > > > -- > > 1.7.9.5 > > > > > > -- > Paulo Zanoni > ___ > 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
[Intel-gfx] [PATCH igt 2/2] kms_fbc_crc: Add a CRC based FBC test
From: Ville Syrjälä kms_fbc_crc will perform various write operations to the scanout buffer whilc FBC is enabled. CRC checks will be used to make sure the modifcations to scanout buffer are detected. The operations include: - page flip - pwrite - GTT mmap - CPU mmap - blit - rendercopy - context switch + rendercopy - combination of a page flip and each operation listed above Signed-off-by: Ville Syrjälä --- tests/Makefile.sources | 1 + tests/kms_fbc_crc.c| 544 + 2 files changed, 545 insertions(+) create mode 100644 tests/kms_fbc_crc.c diff --git a/tests/Makefile.sources b/tests/Makefile.sources index a02b93d..d201809 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -49,6 +49,7 @@ TESTS_progs_M = \ gem_write_read_ring_switch \ kms_addfb \ kms_cursor_crc \ + kms_fbc_crc \ kms_flip \ kms_pipe_crc_basic \ kms_render \ diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c new file mode 100644 index 000..355e33a --- /dev/null +++ b/tests/kms_fbc_crc.c @@ -0,0 +1,544 @@ +/* + * Copyright © 2013 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ + +#include +#include +#include +#include + +#include + +#include "drm_fourcc.h" + +#include "drmtest.h" +#include "igt_debugfs.h" +#include "igt_kms.h" +#include "rendercopy.h" + +enum test_mode { + TEST_PAGE_FLIP, + TEST_PWRITE, + TEST_MMAP_CPU, + TEST_MMAP_GTT, + TEST_BLT, + TEST_RENDER, + TEST_CONTEXT, + TEST_PAGE_FLIP_AND_PWRITE, + TEST_PAGE_FLIP_AND_MMAP_CPU, + TEST_PAGE_FLIP_AND_MMAP_GTT, + TEST_PAGE_FLIP_AND_BLT, + TEST_PAGE_FLIP_AND_RENDER, + TEST_PAGE_FLIP_AND_CONTEXT, +}; + +typedef struct { + struct kmstest_connector_config config; + drmModeModeInfo mode; + struct kmstest_fb fb[2]; +} connector_t; + +typedef struct { + int drm_fd; + igt_debugfs_t debugfs; + drmModeRes *resources; + FILE *ctl; + igt_crc_t ref_crc[2]; + igt_pipe_crc_t **pipe_crc; + drm_intel_bufmgr *bufmgr; + drm_intel_context *ctx[2]; + uint32_t devid; + uint32_t handle[2]; + uint32_t crtc_id; + uint32_t crtc_idx; + uint32_t fb_id[2]; +} data_t; + +static const char *test_mode_str(enum test_mode mode) +{ + static const char * const test_modes[] = { + [TEST_PAGE_FLIP] = "page_flip", + [TEST_PWRITE] = "pwrite", + [TEST_MMAP_CPU] = "mmap_cpu", + [TEST_MMAP_GTT] = "mmap_gtt", + [TEST_BLT] = "blt", + [TEST_RENDER] = "render", + [TEST_CONTEXT] = "context", + [TEST_PAGE_FLIP_AND_PWRITE] = "page_flip_and_pwrite", + [TEST_PAGE_FLIP_AND_MMAP_CPU] = "page_flip_and_mmap_cpu", + [TEST_PAGE_FLIP_AND_MMAP_GTT] = "page_flip_and_mmap_gtt", + [TEST_PAGE_FLIP_AND_BLT] = "page_flip_and_blt", + [TEST_PAGE_FLIP_AND_RENDER] = "page_flip_and_render", + [TEST_PAGE_FLIP_AND_CONTEXT] = "page_flip_and_context", + }; + + return test_modes[mode]; +} + +static uint32_t create_fb(data_t *data, + int w, int h, + double r, double g, double b, + struct kmstest_fb *fb) +{ + uint32_t fb_id; + cairo_t *cr; + + fb_id = kmstest_create_fb2(data->drm_fd, w, h, + DRM_FORMAT_XRGB, true, fb); + igt_assert(fb_id); + + cr = kmstest_get_cairo_ctx(data->drm_fd, fb); + kmstest_paint_color(cr, 0, 0, w, h, r, g, b); + igt_assert(cairo_status(cr) == 0); + + return fb_id; +} + +static bool +connector_set_mode(data_t *data, connector_t *connector, + drmModeModeInfo *mode, uint
Re: [Intel-gfx] [PATCH 12/19] drm/i915: release the GTT mmaps when going into D3
On Thu, Nov 21, 2013 at 01:47:26PM -0200, Paulo Zanoni wrote: > From: Paulo Zanoni > > So we'll get a fault when someone tries to access the mmap, then we'll > wake up from D3. Harsh. Very harsh. Is the GTT completely off-limits under pc8, or is it only the GTT access to the display engine? i.e. could we get away with only killing some of the mmaps? -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 15/19] drm/i915: don't enable VDD just to enable the panel
From: Paulo Zanoni We just don't need this. This saves 250ms from every modeset on my machine. Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_ddi.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 731a919..e868f5f 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1122,9 +1122,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder) if (type == INTEL_OUTPUT_EDP) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder); - ironlake_edp_panel_vdd_on(intel_dp); ironlake_edp_panel_on(intel_dp); - ironlake_edp_panel_vdd_off(intel_dp, true); } WARN_ON(intel_crtc->ddi_pll_sel == PORT_CLK_SEL_NONE); -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] AVI infoframes: default aspect ratio/VIC for CEA modes
On Thu, Nov 21, 2013 at 02:48:55AM +, Gohad, Tushar wrote: > > On Wed, Nov 20, 2013 at 11:45:03PM +, Gohad, Tushar wrote: > > > > > On Wed, Nov 20, 2013 at 09:48:26PM +, Gohad, Tushar wrote: > > > > > > Folks, > > > > > > > > > > > > When filling in an HDMI AVI infoframe, how does one correctly > > > > > > determine the "default" picture aspect ratio (and VIC) for CEA > > > > > > modes that support multiple (4:3 and 16:9) aspect ratios. > > > > > > 720x576p for example, corresponds to VIC 17 or 18: > > > > > > > > > > > > /* 17 - 720x576@50Hz */ > > > > > > { DRM_MODE("720x576", DRM_MODE_TYPE_DRIVER, 27000, 720, > > > > 732, > > > > > >796, 864, 0, 576, 581, 586, 625, 0, > > > > > >DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC), > > > > > > .vrefresh = 50, }, > > > > > > /* 18 - 720x576@50Hz */ > > > > > > { DRM_MODE("720x576", DRM_MODE_TYPE_DRIVER, 27000, 720, > > > > 732, > > > > > >796, 864, 0, 576, 581, 586, 625, 0, > > > > > >DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC), > > > > > > .vrefresh = 50, }, > > > > > > > > > > > > Should the "picture aspect ratio" information be derived from > > > > > > sink EDID (from detailed/cvt/standard timings)? .. possibly in > > > > > > drm_add_edid_modes() and store the picture aspect ratio in > > > > > > drm_display_mode perhaps, for later use? Trying to get a better > > > > > > understanding of how this usually works. > > > > > > > > > > Oh no! someone finally discovered it! Yes, we are totally missing > > > > > the picture aspect ratio information from the CEA modes. It's been > > > > > on my TODO list for a couple of month but not exactly high > > > > > priority. If we want to get a stab a it, we'll review the patches > > > > > :) > > > > > > > Thanks Damien. Sure, I can come up with something quick. Is the idea > > > then to store aspect ratio information in drm_display_mode, possibly > > > as a separate member or as a hint that's part of mode->flags? > > > > It seems natural to extend those flags to describe the picture aspect ratio > > (that > > why dri-devel is in Cc., touching core DRM). > > Cc: dri-devel > > To start with we can use a single bit in drm_display_mode->flags to > distinguish 16:9 vs 4:3. I must admit I haven't really looked into the matter, but do we actually need any mode bits? Would it be enoguh to just have a property that allows the user to specify the picture aspect ratio? -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/19] drm/i915: get a runtime PM reference when the panel VDD is on
From: Paulo Zanoni And put it when it's off. Otherwise, when you run pm_pc8 from intel-gpu-tools, and the delayed function that disables VDD runs, we'll get some messages saying we're touching registers while the HW is suspended. Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_dp.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 28fc070..9e9e3d6 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1092,6 +1092,8 @@ void ironlake_edp_panel_vdd_on(struct intel_dp *intel_dp) if (ironlake_edp_have_panel_vdd(intel_dp)) return; + intel_runtime_pm_get(dev_priv); + DRM_DEBUG_KMS("Turning eDP VDD on\n"); if (!ironlake_edp_have_panel_power(intel_dp)) @@ -1141,6 +1143,8 @@ static void ironlake_panel_vdd_off_sync(struct intel_dp *intel_dp) DRM_DEBUG_KMS("PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n", I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg)); msleep(intel_dp->panel_power_down_delay); + + intel_runtime_pm_put(dev_priv); } } @@ -1248,6 +1252,9 @@ void ironlake_edp_panel_off(struct intel_dp *intel_dp) intel_dp->want_panel_vdd = false; ironlake_wait_panel_off(intel_dp); + + /* We got a reference when we enabled the VDD. */ + intel_runtime_pm_put(dev_priv); } void ironlake_edp_backlight_on(struct intel_dp *intel_dp) -- 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 8/9] drm/i915: Hook up dirtyfb ioctl for FBC nuke
On Thu, Nov 21, 2013 at 09:29:52PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > FBC host modification tracking only works through GTT mmaps, so any > direct CPU access needs to manually nuke the compressed framebuffer > on modifications. Hook up the dirtyfb ioctl to do just that. But direct CPU access (not GTT) notification is done through SW_FINISH ioctl. Overzealously nuking FBC here makes it inconvenient for userpsace to use fb_dirty as part of its periodic-flush-scanout procedure. -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 9/9] drm/i915: Flush caches for scanout during cpu->gtt move
On Thu, Nov 21, 2013 at 09:29:53PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Flush the caches when moving a scanout buffer from CPU to GTT domain. > This allows us to move a scanout buffer to CPU write domain, do some > writes, and move it back to the GTT read domain. The display will then > see the correct data. In addition we still need to do the dirtyfb > ioctl to nuke FBC if that's enabled. > > Signed-off-by: Ville Syrjälä Reviewed-by: Chris Wilson But we could actually rework flush_cpu_write_domain to drop the force parameter now that we have obj->pin_display. -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/9] drm/i915: Don't set the fence number in DPFC_CTL on SNB
On Thu, Nov 21, 2013 at 09:29:45PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > SNB has another register where the actual FBC CPU fence number is > stored. The documenation explicitly states that the fence number > in DPFC_CTL must be 0 on SNB. And in fact when it's not zero, > the GTT tracking simply doesn't work. > > Signed-off-by: Ville Syrjälä Reviewed-by: Chris Wilson -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 05/19] drm/i915: add initial Runtime PM functions
From: Paulo Zanoni This patch adds the initial infrastructure to allow a Runtime PM implementation that sets the device to its D3 state. The patch just adds the necessary callbacks and the initial infrastructure. We still don't have any platform that actually uses this infrastructure, we still don't call get/put in all the places we need to, and we don't have any function to save/restore the state of the registers. This is not a problem since no platform uses the code added by this patch. We have a few people simultaneously working on runtime PM, so this initial code could help everybody make their plans. V2: - Move some functions to intel_pm.c - Remove useless pm_runtime_allow() call at init - Remove useless pm_runtime_mark_last_busy() call at get - Use pm_runtime_get_sync() instead of 2 calls - Add a WARN to check if we're really awake Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/i915_dma.c | 6 drivers/gpu/drm/i915/i915_drv.c | 42 drivers/gpu/drm/i915/i915_drv.h | 7 + drivers/gpu/drm/i915/intel_drv.h| 4 +++ drivers/gpu/drm/i915/intel_pm.c | 56 + drivers/gpu/drm/i915/intel_uncore.c | 9 ++ 6 files changed, 124 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 25acbb5..0ce9e38 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -42,6 +42,8 @@ #include #include #include +#include +#include #define LP_RING(d) (&((struct drm_i915_private *)(d))->ring[RCS]) @@ -1664,6 +1666,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) if (IS_GEN5(dev)) intel_gpu_ips_init(dev_priv); + intel_init_runtime_pm(dev_priv); + return 0; out_power_well: @@ -1704,6 +1708,8 @@ int i915_driver_unload(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; int ret; + intel_fini_runtime_pm(dev_priv); + intel_gpu_ips_teardown(); if (HAS_POWER_WELL(dev)) { diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index c2e00ed..d7ff095 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -500,6 +500,8 @@ static int i915_drm_freeze(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct drm_crtc *crtc; + intel_runtime_pm_get(dev_priv); + /* ignore lid events during suspend */ mutex_lock(&dev_priv->modeset_restore_lock); dev_priv->modeset_restore = MODESET_SUSPENDED; @@ -684,6 +686,8 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) mutex_lock(&dev_priv->modeset_restore_lock); dev_priv->modeset_restore = MODESET_DONE; mutex_unlock(&dev_priv->modeset_restore_lock); + + intel_runtime_pm_put(dev_priv); return error; } @@ -898,6 +902,42 @@ static int i915_pm_poweroff(struct device *dev) return i915_drm_freeze(drm_dev); } +static int i915_runtime_suspend(struct device *device) +{ + struct pci_dev *pdev = to_pci_dev(device); + struct drm_device *dev = pci_get_drvdata(pdev); + struct drm_i915_private *dev_priv = dev->dev_private; + + WARN_ON(!HAS_RUNTIME_PM(dev)); + + DRM_DEBUG_KMS("Suspending device\n"); + + dev_priv->pm.suspended = true; + + pci_save_state(pdev); + pci_set_power_state(pdev, PCI_D3cold); + + return 0; +} + +static int i915_runtime_resume(struct device *device) +{ + struct pci_dev *pdev = to_pci_dev(device); + struct drm_device *dev = pci_get_drvdata(pdev); + struct drm_i915_private *dev_priv = dev->dev_private; + + WARN_ON(!HAS_RUNTIME_PM(dev)); + + DRM_DEBUG_KMS("Resuming device\n"); + + pci_set_power_state(pdev, PCI_D0); + pci_restore_state(pdev); + + dev_priv->pm.suspended = false; + + return 0; +} + static const struct dev_pm_ops i915_pm_ops = { .suspend = i915_pm_suspend, .resume = i915_pm_resume, @@ -905,6 +945,8 @@ static const struct dev_pm_ops i915_pm_ops = { .thaw = i915_pm_thaw, .poweroff = i915_pm_poweroff, .restore = i915_pm_resume, + .runtime_suspend = i915_runtime_suspend, + .runtime_resume = i915_runtime_resume, }; static const struct vm_operations_struct i915_gem_vm_ops = { diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 14f250a..3877a68 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1275,6 +1275,10 @@ struct i915_package_c8 { } regsave; }; +struct i915_runtime_pm { + bool suspended; +}; + enum intel_pipe_crc_source { INTEL_PIPE_CRC_SOURCE_NONE, INTEL_PIPE_CRC_SOURCE_PLANE1, @@ -1505,6 +1509,8 @@ typedef struct drm_i915_private { struct i915_package_c8 pc8; +
[Intel-gfx] [PATCH 5/9] drm/i915: Reorder i915_gem_execbuffer_move_to_gpu() and i915_switch_context()
From: Ville Syrjälä The FBC RT address is stored in the context, and thus needs to be rewritten after a context switch before any batches are run. We emit the LRI to update the FBC RT address when we call the ring ->flush function to invalidate the caches. When a context switch is being performed we currently do the invalidate before the context switch, which means we're not updating the FBC RT address correctly. Move the i915_gem_execbuffer_move_to_gpu() call to happen after i915_switch_context() to make the LRI happen after the MI_SET_CONTEXT. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 2d96edf..7389e7a 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1181,10 +1181,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, i915_gem_execbuffer_mark_fbc_dirty(ring, &eb->vmas); - ret = i915_gem_execbuffer_move_to_gpu(ring, &eb->vmas); - if (ret) - goto err; - hs = i915_gem_context_get_hang_stats(dev, file, ctx_id); if (IS_ERR(hs)) { ret = PTR_ERR(hs); @@ -1200,6 +1196,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, if (ret) goto err; + ret = i915_gem_execbuffer_move_to_gpu(ring, &eb->vmas); + if (ret) + goto err; + if (ring == &dev_priv->ring[RCS] && mode != dev_priv->relative_constants_mode) { ret = intel_ring_begin(ring, 4); -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 13/19] drm: do not steal the display if we have a master
From: Paulo Zanoni Sometimes we want to disable all the screens on a system, because that will allow the graphics card to be put into low-power states. The problem is that, for example, while all screens are disabled, if we get a hotplug interrupt, fbcon will decide to set a mode instead of keeping everything disabled, which will remove us from our low power states. Let's assume that if there's a DRM master, it will be able to do whatever is appropriate when we get the hotplug. This problem can be reproduced by the runtime PM test program from intel-gpu-tools: we disable all the screens so the graphics device can be put into D3, then something triggers a hotplug interrupt, fbcon sets a mode and breaks our test suite. The problem can be reproduced more easily by the "i2c" subtest. Other approaches considered for the problem: - Return "false" if "bound == 0" and the caller of drm_fb_helper_is_bound is a hotplug handler. This would break the case where the machine boots with no outputs connected, then the user plugs a monitor. - Add a new IOCTL to force fbcon to not set modes. This would keep all the current applications behaving the same, but adding a new IOCTL is not always the greatest idea. Thanks to Daniel Vetter for the investigation, ideas and the implementation of the hotplug alternative. Credits-to: Daniel Vetter Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/drm_fb_helper.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 0a19401..199d0c0 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -368,6 +368,12 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper) if (bound < crtcs_bound) return false; + + /* Sometimes user space wants everything disabled, so don't steal the +* display if there's a master. */ + if (bound == 0 && dev->primary->master) + return false; + return true; } -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 19/19] drm/i915: init the DP panel power seq regs earlier
From: Paulo Zanoni When we call intel_dp_i2c_init we already get some I2C calls, which will trigger a VDD enable, which will make use of the panel power sequencing registers, so we need to have them ready by this time. The good side is that we were reading the values, but were not using them for anything (because we were just skipping the msleep(0) calls), so this "fix" shouldn't fix any real existing bugs. I was only able to identify the problem because I added some debug code to check how much time time we were saving with my previous patch. Regression introduced by: commit ed92f0b239ac971edc509169ae3d6955fbe0a188 Author: Paulo Zanoni Date: Wed Jun 12 17:27:24 2013 -0300 drm/i915: extract intel_edp_init_connector Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_dp.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 3a1ca80..23927a0 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3565,14 +3565,14 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, } static bool intel_edp_init_connector(struct intel_dp *intel_dp, -struct intel_connector *intel_connector) +struct intel_connector *intel_connector, +struct edp_power_seq *power_seq) { struct drm_connector *connector = &intel_connector->base; struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); struct drm_device *dev = intel_dig_port->base.base.dev; struct drm_i915_private *dev_priv = dev->dev_private; struct drm_display_mode *fixed_mode = NULL; - struct edp_power_seq power_seq = { 0 }; bool has_dpcd; struct drm_display_mode *scan; struct edid *edid; @@ -3580,8 +3580,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, if (!is_edp(intel_dp)) return true; - intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq); - /* Cache DPCD and EDID for edp. */ ironlake_edp_panel_vdd_on(intel_dp); has_dpcd = intel_dp_get_dpcd(intel_dp); @@ -3599,8 +3597,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, } /* We now know it's not a ghost, init power sequence regs. */ - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, - &power_seq); + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, power_seq); edid = drm_get_edid(connector, &intel_dp->adapter); if (edid) { @@ -3649,6 +3646,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, struct drm_device *dev = intel_encoder->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; enum port port = intel_dig_port->port; + struct edp_power_seq power_seq = { 0 }; const char *name = NULL; int type, error; @@ -3748,13 +3746,16 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, BUG(); } + if (is_edp(intel_dp)) + intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq); + error = intel_dp_i2c_init(intel_dp, intel_connector, name); WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n", error, port_name(port)); intel_dp->psr_setup_done = false; - if (!intel_edp_init_connector(intel_dp, intel_connector)) { + if (!intel_edp_init_connector(intel_dp, intel_connector, &power_seq)) { i2c_del_adapter(&intel_dp->adapter); if (is_edp(intel_dp)) { cancel_delayed_work_sync(&intel_dp->panel_vdd_work); -- 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 18/19] drm/i915: save some time when waiting the eDP timings
On Thu, Nov 21, 2013 at 01:47:32PM -0200, Paulo Zanoni wrote: > From: Paulo Zanoni > > The eDP spec defines some points where after you do action A, you have > to wait some time before action B. The thing is that in our driver > action B does not happen exactly after action A, but we still use > msleep() calls directly. What this patch happens is that we record the > timestamp of when action A happened, then, just before action B, we > look at how much time has passed and only sleep the remaining amount > needed. > > With this change, I am able to save about 5-20ms (out of the total > 200ms) of the backlight_off delay and completely skip the 1ms > backlight_on delay. The 600ms vdd_off delay doesn't happen during > normal usage anymore due to a previous patch. > > Signed-off-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/intel_dp.c | 38 +++--- > drivers/gpu/drm/i915/intel_drv.h | 3 +++ > 2 files changed, 38 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index b438e76..3a1ca80 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1051,12 +1051,41 @@ static void ironlake_wait_panel_off(struct intel_dp > *intel_dp) > ironlake_wait_panel_status(intel_dp, IDLE_OFF_MASK, IDLE_OFF_VALUE); > } > > +static void ironlake_wait_jiffies_delay(unsigned long timestamp, > + int to_wait_ms) This is not hw specific, so just intel_wait_until_after(timestamp_jiffies, to_wait_ms) > +{ > + unsigned long target = timestamp + msecs_to_jiffies(to_wait_ms); > + unsigned long diff; > + > + if (time_after(target, jiffies)) { > + diff = (long)target - (long)jiffies; > + msleep(diff); msleep() expects a duration in ms, diff is in jiffies. -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 16/19] drm/i915: don't touch the VDD when disabling the panel
From: Paulo Zanoni I don't see a reason to touch VDD when we're disabling the panel: since the panel is enabled, we don't need VDD. This saves a few sleep calls from the vdd_on and vdd_off functions at every modeset. Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_ddi.c | 1 - drivers/gpu/drm/i915/intel_dp.c | 10 +- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index e868f5f..5fc4b9e 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1165,7 +1165,6 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder) if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder); - ironlake_edp_panel_vdd_on(intel_dp); intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); ironlake_edp_panel_off(intel_dp); } diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 9e9e3d6..c2a89b2 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1237,24 +1237,17 @@ void ironlake_edp_panel_off(struct intel_dp *intel_dp) DRM_DEBUG_KMS("Turn eDP power off\n"); - WARN(!intel_dp->want_panel_vdd, "Need VDD to turn off panel\n"); - pp = ironlake_get_pp_control(intel_dp); /* We need to switch off panel power _and_ force vdd, for otherwise some * panels get very unhappy and cease to work. */ - pp &= ~(POWER_TARGET_ON | EDP_FORCE_VDD | PANEL_POWER_RESET | EDP_BLC_ENABLE); + pp &= ~(POWER_TARGET_ON | PANEL_POWER_RESET | EDP_BLC_ENABLE); pp_ctrl_reg = _pp_ctrl_reg(intel_dp); I915_WRITE(pp_ctrl_reg, pp); POSTING_READ(pp_ctrl_reg); - intel_dp->want_panel_vdd = false; - ironlake_wait_panel_off(intel_dp); - - /* We got a reference when we enabled the VDD. */ - intel_runtime_pm_put(dev_priv); } void ironlake_edp_backlight_on(struct intel_dp *intel_dp) @@ -1779,7 +1772,6 @@ static void intel_disable_dp(struct intel_encoder *encoder) /* Make sure the panel is off before trying to change the mode. But also * ensure that we have vdd while we switch off the panel. */ - ironlake_edp_panel_vdd_on(intel_dp); ironlake_edp_backlight_off(intel_dp); intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); ironlake_edp_panel_off(intel_dp); -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/19] drm/i915: do not assert DE_PCH_EVENT_IVB enabled
From: Paulo Zanoni The current code was checking if all bits of "val" were enabled and DE_PCH_EVENT_IVB was disabled. The new code doesn't care about the state of DE_PCH_EVENT_IVB: it just checks if everything else is 1. The goal is that future patches may completely disable interrupts, and the LCPLL-disabling code shouldn't care about the state of DE_PCH_EVENT_IVB. Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 846f2de..95e8831 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6499,7 +6499,7 @@ static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv) spin_lock_irqsave(&dev_priv->irq_lock, irqflags); val = I915_READ(DEIMR); - WARN((val & ~DE_PCH_EVENT_IVB) != val, + WARN((val | DE_PCH_EVENT_IVB) != 0x, "Unexpected DEIMR bits enabled: 0x%x\n", val); val = I915_READ(SDEIMR); WARN((val | SDE_HOTPLUG_MASK_CPT) != 0x, -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] i915, fbdev: Fix Kconfig typo
From: Borislav Petkov Too many t's. Cc: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org Cc: dri-de...@lists.freedesktop.org Signed-off-by: Borislav Petkov --- drivers/gpu/drm/i915/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 6199d0b5b958..7ca35ecfd952 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -43,7 +43,7 @@ config DRM_I915_KMS like intelfb. config DRM_I915_FBDEV - bool "Enable legacy fbdev support for the modesettting intel driver" + bool "Enable legacy fbdev support for the modesetting intel driver" depends on DRM_I915 select DRM_KMS_FB_HELPER select FB_CFB_FILLRECT -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 00/19] Haswell runtime PM support + D3
From: Paulo Zanoni Hi This series adds Haswell runtime PM support, which will put the graphics device in D3 state, saving a lot of power. Fore more information, see the previous cover letter: http://lists.freedesktop.org/archives/intel-gfx/2013-October/034910.html What changes from the previous series? Previously, the runtime PM test suite was separate from the PC8 test suite. Now, pm_pc8.c is able to recognize both features, so we can run all tests on both PC8-only or PC8+D3 cases. This code is all upstream. One of the consequences is that some of the new D3 tests triggered bugs in cases where we only had PC8, so patches 2, 3 and 4 solve these problems. Patches 5-14 add the runtime PM support on Haswell. They are basically the same thing as before, with a few new additions to catch the bugs I could reproduce with the test suite. The other difference is that even though runtime PM is not enabled by default, all you need to enable it is to "echo auto" to a sysfs file (or just run powertop and switch things to "good"). No more Kernel parameter is necessary. I could add it back if we want, no problem. Patches 15-19 are not really part of the runtime PM feature, but they are on top of that work, so I decided to send them anyway. While writing patch 9 I identified some problems on the VDD code, so I decided to fix them on patches 15-19. The big benefit is that we avoid some msleep() calls with these patches, so all eDP modesets get faster. In short: - Patches 1-4: fixes for PC8 code - Patches 5-14: add the runtime PM support - Patches 15-19: some fixes for eDP VDD on top of everything What are we still missing? - Merge PC8 and runtime D3: since PC8 without D3 doesn't make much sense, I plan to merge both features so they become a single thing. This will also make it easier for other platforms to add runtime PM support. - Support for PC8/D3 on DPMS. Jesse already started studying this problem, but there's a lot of rework we need to do on the modeset code before we reach this problem. - Support other platforms: adding support for another platform (e.g., VLV or IVB) will help us identify the code that needs to be shared and the code that's gen-specific. Still, none of these missing things is a blocker: IMHO we can merge the Haswell support now and worry about the other features later. Thanks, Paulo Paulo Zanoni (19): drm/i915: WARN if !HAS_PC8 when enabling/disabling PC8 drm/i915: use the correct force_wake function at the PC8 code drm/i915: get a PC8 reference when enabling the power well drm/i915: get/put PC8 when we get/put a CRTC drm/i915: add initial Runtime PM functions drm/i915: do adapter power state notification at runtime PM drm/i915: add runtime put/get calls at the basic places drm/i915: add some runtime PM get/put calls drm/i915: get a runtime PM reference when the panel VDD is on drm/i915: do not assert DE_PCH_EVENT_IVB enabled drm/i915: disable interrupts when enabling PC8 drm/i915: release the GTT mmaps when going into D3 drm: do not steal the display if we have a master drm/i915: add runtime PM support on Haswell drm/i915: don't enable VDD just to enable the panel drm/i915: don't touch the VDD when disabling the panel drm/i915: fix VDD override off wait drm/i915: save some time when waiting the eDP timings drm/i915: init the DP panel power seq regs earlier drivers/gpu/drm/drm_fb_helper.c| 6 +++ drivers/gpu/drm/i915/i915_debugfs.c| 45 -- drivers/gpu/drm/i915/i915_dma.c| 6 +++ drivers/gpu/drm/i915/i915_drv.c| 46 ++ drivers/gpu/drm/i915/i915_drv.h| 8 drivers/gpu/drm/i915/i915_gem.c| 61 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 +++ drivers/gpu/drm/i915/i915_irq.c| 32 ++--- drivers/gpu/drm/i915/i915_sysfs.c | 14 +- drivers/gpu/drm/i915/intel_ddi.c | 3 -- drivers/gpu/drm/i915/intel_display.c | 41 +--- drivers/gpu/drm/i915/intel_dp.c| 75 +++--- drivers/gpu/drm/i915/intel_drv.h | 7 +++ drivers/gpu/drm/i915/intel_panel.c | 3 ++ drivers/gpu/drm/i915/intel_pm.c| 70 +++- drivers/gpu/drm/i915/intel_uncore.c| 13 ++ 16 files changed, 366 insertions(+), 70 deletions(-) -- 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 06/19] drm/i915: do adapter power state notification at runtime PM
On Thu, Nov 21, 2013 at 01:47:20PM -0200, Paulo Zanoni wrote: > From: Paulo Zanoni > > Now that we are actually setting the device to the D3 state, we should > issue the notification. Can you please add a snippet to justify the ordering? Is there anything to say what state the callee expects the device to be in when we send the notificiation? -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 v4 06/10] drm/i915: Implement LRI based FBC tracking
On Thu, Nov 21, 2013 at 06:33:21PM +0200, Ville Syrjälä wrote: > On Thu, Nov 21, 2013 at 11:49:47AM +, Chris Wilson wrote: > > On Thu, Nov 21, 2013 at 01:14:10PM +0200, ville.syrj...@linux.intel.com > > wrote: > > > From: Ville Syrjälä > > > > > > As per the SNB and HSW PM guides, we should enable FBC render/blitter > > > tracking only during batches targetting the front buffer. > > > > > > On SNB we must also update the FBC render tracking address whenever it > > > changes. And since the register in question is stored in the context, > > > we need to make sure we reload it with correct data after context > > > switches. > > > > > > On IVB/HSW we use the render nuke mechanism, so no render tracking > > > address updates are needed. Hoever on the blitter side we need to > > > enable the blitter tracking like on SNB, and in addition we need > > > to issue the cache clean messages, which we already did. > > > > > > v2: Introduce intel_fb_obj_has_fbc() > > > Fix crtc locking around crtc->fb access > > > Drop a hunk that was included by accident in v1 > > > Set fbc_address_dirty=false not true after emitting the LRI > > > v3: Now that fbc hangs on to the fb intel_fb_obj_has_fbc() doesn't > > > need to upset lockdep anymore > > > v4: Use |= instead of = to update fbc_address_dirty > > > > Ah, should we do the same for fbc_dirty? In the past we could dispatch a > > batchbuffer but fail to add the request (and so fail to flush the > > rendering/fbc). We currently preallocate the request so that failure > > path is history, but we will more than likely be caught out again in the > > future. > > I guess we could do it for fbc_dirty as well, but I don't think that > should actually change anything. Either we're rendering to the FBC > scanout buffer, or we're not. The scenario I worry about here is the missing flush after the rendering. It has been possible for us to lose it (under memory pressure, other constaints etc) and to issue a catch-up flush on the next batch. Without the or, we'd lose the FBC flush. It is just a potential issue for the future. > I did start pondering if I should actually move the fbc_address to live > under the context once that's where it actually belongs. If we'd track > it per-context we might be able to avoid emitting the LRI for every > context switch. > > > > > Like pc8, I'd like for a device (or crtc if you must) property whether > > or not indirect rendering is preferred. > > > > Other than that and the bikeshed to kill the redundant fbc_obj local > > variable and pack the dirty bits, it looks good to me. > > Actually I just fired it up for real on SNB and it failed. The problem > is that we end up doing the invalidate before the context switch. So > we've not yet forced fbc_address_dirty=true due to the context switch > when we do the invalidate. The most straightforward fix would be to > simply move i915_gem_execbuffer_move_to_gpu() to be called after > i915_switch_context(). I did that on my machine and now it passes my > context related FBC tests. But I do wonder if the order of these > operations was chose for a reason, and whether the reordering might > cause other problems. Ben, opinion on whether the ordering was just convenience? -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 13/19] drm: do not steal the display if we have a master
On Thu, Nov 21, 2013 at 01:47:27PM -0200, Paulo Zanoni wrote: > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 0a19401..199d0c0 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -368,6 +368,12 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper > *fb_helper) > > if (bound < crtcs_bound) > return false; > + > + /* Sometimes user space wants everything disabled, so don't steal the > + * display if there's a master. */ > + if (bound == 0 && dev->primary->master) > + return false; Just do this check first (irrespective of bound). -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 igt 1/2] rendercopy: Pass context to rendercopy functions
From: Ville Syrjälä rendercopy does the batch buffer flush internally, so if we want to use it with multiple contexts, we need to pass the context in from caller. Signed-off-by: Ville Syrjälä --- lib/rendercopy.h| 5 + lib/rendercopy_gen6.c | 12 +++- lib/rendercopy_gen7.c | 12 +++- lib/rendercopy_i830.c | 1 + lib/rendercopy_i915.c | 1 + tests/gem_ctx_basic.c | 2 +- tests/gem_render_copy.c | 2 +- tests/gem_render_linear_blits.c | 6 +++--- tests/gem_render_tiled_blits.c | 6 +++--- tests/gem_ringfill.c| 5 +++-- tests/gem_seqno_wrap.c | 2 +- tests/gem_stress.c | 2 +- tests/kms_flip.c| 2 +- 13 files changed, 35 insertions(+), 23 deletions(-) diff --git a/lib/rendercopy.h b/lib/rendercopy.h index f726df6..5586b1c 100644 --- a/lib/rendercopy.h +++ b/lib/rendercopy.h @@ -62,6 +62,7 @@ static inline unsigned buf_height(struct scratch_buf *buf) } typedef void (*render_copyfunc_t)(struct intel_batchbuffer *batch, + drm_intel_context *context, struct scratch_buf *src, unsigned src_x, unsigned src_y, unsigned width, unsigned height, struct scratch_buf *dst, unsigned dst_x, unsigned dst_y); @@ -73,18 +74,22 @@ void gen8_render_copyfunc(struct intel_batchbuffer *batch, unsigned width, unsigned height, struct scratch_buf *dst, unsigned dst_x, unsigned dst_y); void gen7_render_copyfunc(struct intel_batchbuffer *batch, + drm_intel_context *context, struct scratch_buf *src, unsigned src_x, unsigned src_y, unsigned width, unsigned height, struct scratch_buf *dst, unsigned dst_x, unsigned dst_y); void gen6_render_copyfunc(struct intel_batchbuffer *batch, + drm_intel_context *context, struct scratch_buf *src, unsigned src_x, unsigned src_y, unsigned width, unsigned height, struct scratch_buf *dst, unsigned dst_x, unsigned dst_y); void gen3_render_copyfunc(struct intel_batchbuffer *batch, + drm_intel_context *context, struct scratch_buf *src, unsigned src_x, unsigned src_y, unsigned width, unsigned height, struct scratch_buf *dst, unsigned dst_x, unsigned dst_y); void gen2_render_copyfunc(struct intel_batchbuffer *batch, + drm_intel_context *context, struct scratch_buf *src, unsigned src_x, unsigned src_y, unsigned width, unsigned height, struct scratch_buf *dst, unsigned dst_x, unsigned dst_y); diff --git a/lib/rendercopy_gen6.c b/lib/rendercopy_gen6.c index dafee88..457cb35 100644 --- a/lib/rendercopy_gen6.c +++ b/lib/rendercopy_gen6.c @@ -78,14 +78,15 @@ batch_copy(struct intel_batchbuffer *batch, const void *ptr, uint32_t size, uint } static void -gen6_render_flush(struct intel_batchbuffer *batch, uint32_t batch_end) +gen6_render_flush(struct intel_batchbuffer *batch, + drm_intel_context *context, uint32_t batch_end) { int ret; ret = drm_intel_bo_subdata(batch->bo, 0, 4096, batch->buffer); if (ret == 0) - ret = drm_intel_bo_mrb_exec(batch->bo, batch_end, - NULL, 0, 0, 0); + ret = drm_intel_gem_bo_context_exec(batch->bo, context, + batch_end, 0); assert(ret == 0); } @@ -529,6 +530,7 @@ static uint32_t gen6_emit_primitive(struct intel_batchbuffer *batch) } void gen6_render_copyfunc(struct intel_batchbuffer *batch, + drm_intel_context *context, struct scratch_buf *src, unsigned src_x, unsigned src_y, unsigned width, unsigned height, struct scratch_buf *dst, unsigned dst_x, unsigned dst_y) @@ -537,7 +539,7 @@ void gen6_render_copyfunc(struct intel_batchbuffer *batch, uint32_t cc_vp, cc_blend, offset; uint32_t batch_end; - intel_batchbuffer_flush(batch); + intel_batchbuffer_flush_with_context(batch, context); batch->ptr = batch->buffer + 1024; batch_alloc(batch, 64, 64); @@ -594,6 +596,6 @@ void gen6_render_copyfunc(struct intel_batchbuffer *batch, emit_vertex_normalized(batch, src_x, buf_width(src)); emit_vertex_normalized(batch, src_y, buf_height(src)); - gen6_render_flush(batch, batch_end); + gen6_render_flush(batch, context, batch_end); intel_batchbuffer_reset(batch); } diff --git a/lib
Re: [Intel-gfx] [PATCH v4 06/10] drm/i915: Implement LRI based FBC tracking
On Thu, Nov 21, 2013 at 11:49:47AM +, Chris Wilson wrote: > On Thu, Nov 21, 2013 at 01:14:10PM +0200, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > As per the SNB and HSW PM guides, we should enable FBC render/blitter > > tracking only during batches targetting the front buffer. > > > > On SNB we must also update the FBC render tracking address whenever it > > changes. And since the register in question is stored in the context, > > we need to make sure we reload it with correct data after context > > switches. > > > > On IVB/HSW we use the render nuke mechanism, so no render tracking > > address updates are needed. Hoever on the blitter side we need to > > enable the blitter tracking like on SNB, and in addition we need > > to issue the cache clean messages, which we already did. > > > > v2: Introduce intel_fb_obj_has_fbc() > > Fix crtc locking around crtc->fb access > > Drop a hunk that was included by accident in v1 > > Set fbc_address_dirty=false not true after emitting the LRI > > v3: Now that fbc hangs on to the fb intel_fb_obj_has_fbc() doesn't > > need to upset lockdep anymore > > v4: Use |= instead of = to update fbc_address_dirty > > Ah, should we do the same for fbc_dirty? In the past we could dispatch a > batchbuffer but fail to add the request (and so fail to flush the > rendering/fbc). We currently preallocate the request so that failure > path is history, but we will more than likely be caught out again in the > future. I guess we could do it for fbc_dirty as well, but I don't think that should actually change anything. Either we're rendering to the FBC scanout buffer, or we're not. I did start pondering if I should actually move the fbc_address to live under the context once that's where it actually belongs. If we'd track it per-context we might be able to avoid emitting the LRI for every context switch. > > Like pc8, I'd like for a device (or crtc if you must) property whether > or not indirect rendering is preferred. > > Other than that and the bikeshed to kill the redundant fbc_obj local > variable and pack the dirty bits, it looks good to me. Actually I just fired it up for real on SNB and it failed. The problem is that we end up doing the invalidate before the context switch. So we've not yet forced fbc_address_dirty=true due to the context switch when we do the invalidate. The most straightforward fix would be to simply move i915_gem_execbuffer_move_to_gpu() to be called after i915_switch_context(). I did that on my machine and now it passes my context related FBC tests. But I do wonder if the order of these operations was chose for a reason, and whether the reordering might cause other problems. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 18/19] drm/i915: save some time when waiting the eDP timings
From: Paulo Zanoni The eDP spec defines some points where after you do action A, you have to wait some time before action B. The thing is that in our driver action B does not happen exactly after action A, but we still use msleep() calls directly. What this patch happens is that we record the timestamp of when action A happened, then, just before action B, we look at how much time has passed and only sleep the remaining amount needed. With this change, I am able to save about 5-20ms (out of the total 200ms) of the backlight_off delay and completely skip the 1ms backlight_on delay. The 600ms vdd_off delay doesn't happen during normal usage anymore due to a previous patch. Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_dp.c | 38 +++--- drivers/gpu/drm/i915/intel_drv.h | 3 +++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index b438e76..3a1ca80 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1051,12 +1051,41 @@ static void ironlake_wait_panel_off(struct intel_dp *intel_dp) ironlake_wait_panel_status(intel_dp, IDLE_OFF_MASK, IDLE_OFF_VALUE); } +static void ironlake_wait_jiffies_delay(unsigned long timestamp, + int to_wait_ms) +{ + unsigned long target = timestamp + msecs_to_jiffies(to_wait_ms); + unsigned long diff; + + if (time_after(target, jiffies)) { + diff = (long)target - (long)jiffies; + msleep(diff); + } +} + static void ironlake_wait_panel_power_cycle(struct intel_dp *intel_dp) { DRM_DEBUG_KMS("Wait for panel power cycle\n"); + + /* When we disable the VDD override bit last we have to do the manual +* wait. */ + ironlake_wait_jiffies_delay(intel_dp->last_power_cycle, + intel_dp->panel_power_cycle_delay); + ironlake_wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE); } +static void ironlake_wait_backlight_on(struct intel_dp *intel_dp) +{ + ironlake_wait_jiffies_delay(intel_dp->last_power_on, + intel_dp->backlight_on_delay); +} + +static void ironlake_edp_wait_backlight_off(struct intel_dp *intel_dp) +{ + ironlake_wait_jiffies_delay(intel_dp->last_backlight_off, + intel_dp->backlight_off_delay); +} /* Read the current pp_control value, unlocking the register if it * is locked @@ -1144,7 +1173,7 @@ static void ironlake_panel_vdd_off_sync(struct intel_dp *intel_dp) I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg)); if ((pp & POWER_TARGET_ON) == 0) - msleep(intel_dp->panel_power_cycle_delay); + intel_dp->last_power_cycle = jiffies; intel_runtime_pm_put(dev_priv); } @@ -1219,6 +1248,7 @@ void ironlake_edp_panel_on(struct intel_dp *intel_dp) POSTING_READ(pp_ctrl_reg); ironlake_wait_panel_on(intel_dp); + intel_dp->last_power_on = jiffies; if (IS_GEN5(dev)) { pp |= PANEL_POWER_RESET; /* restore panel reset bit */ @@ -1239,6 +1269,8 @@ void ironlake_edp_panel_off(struct intel_dp *intel_dp) DRM_DEBUG_KMS("Turn eDP power off\n"); + ironlake_edp_wait_backlight_off(intel_dp); + pp = ironlake_get_pp_control(intel_dp); /* We need to switch off panel power _and_ force vdd, for otherwise some * panels get very unhappy and cease to work. */ @@ -1270,7 +1302,7 @@ void ironlake_edp_backlight_on(struct intel_dp *intel_dp) * link. So delay a bit to make sure the image is solid before * allowing it to appear. */ - msleep(intel_dp->backlight_on_delay); + ironlake_wait_backlight_on(intel_dp); pp = ironlake_get_pp_control(intel_dp); pp |= EDP_BLC_ENABLE; @@ -1302,7 +1334,7 @@ void ironlake_edp_backlight_off(struct intel_dp *intel_dp) I915_WRITE(pp_ctrl_reg, pp); POSTING_READ(pp_ctrl_reg); - msleep(intel_dp->backlight_off_delay); + intel_dp->last_backlight_off = jiffies; } static void ironlake_edp_pll_on(struct intel_dp *intel_dp) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 5596498..778a7be 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -484,6 +484,9 @@ struct intel_dp { int backlight_off_delay; struct delayed_work panel_vdd_work; bool want_panel_vdd; + unsigned long last_power_cycle; + unsigned long last_power_on; + unsigned long last_backlight_off; bool psr_setup_done; struct intel_connector *attached_connector; }; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mail
[Intel-gfx] [PATCH 3/9] drm/i915: Don't set DPFC_HT_MODIFY bit on CTG/ILK/SNB
From: Ville Syrjälä The ILK/SNB docs don't really mention the the DPFC_HT_MODIFY bit. CTG docs clearly state that it should be set only when tracking back buffer modification in persistent mode. The bit is supposed to be set by software after the first CPU modification to the back buffer, and it would get automagically cleared by the hardware on the next page flip. Since we only track front buffer modification we don't need to set this bit. GTT modification tracking still appears to work on ILK and SNB with the bit unset. I don't have a CTG to verify how that behaves. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8cea92e..f46bb56 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -152,7 +152,6 @@ static void g4x_enable_fbc(struct drm_crtc *crtc, dpfc_ctl = plane | DPFC_SR_EN | DPFC_CTL_LIMIT_1X; dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg; - I915_WRITE(DPFC_CHICKEN, DPFC_HT_MODIFY); I915_WRITE(DPFC_RECOMP_CTL, DPFC_RECOMP_STALL_EN | (stall_watermark << DPFC_RECOMP_STALL_WM_SHIFT) | @@ -206,7 +205,6 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, dpfc_ctl |= DPFC_CTL_FENCE_EN; if (IS_GEN5(dev)) dpfc_ctl |= obj->fence_reg; - I915_WRITE(ILK_DPFC_CHICKEN, DPFC_HT_MODIFY); I915_WRITE(ILK_DPFC_RECOMP_CTL, DPFC_RECOMP_STALL_EN | (stall_watermark << DPFC_RECOMP_STALL_WM_SHIFT) | -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 9/9] drm/i915: Flush caches for scanout during cpu->gtt move
From: Ville Syrjälä Flush the caches when moving a scanout buffer from CPU to GTT domain. This allows us to move a scanout buffer to CPU write domain, do some writes, and move it back to the GTT read domain. The display will then see the correct data. In addition we still need to do the dirtyfb ioctl to nuke FBC if that's enabled. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 40d9dcf..d6e354d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3428,7 +3428,7 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write) if (ret) return ret; - i915_gem_object_flush_cpu_write_domain(obj, false); + i915_gem_object_flush_cpu_write_domain(obj, obj->pin_display); /* Serialise direct access to this object with the barriers for * coherent writes from the GPU, by effectively invalidating the -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/9] drm/i915: Use LRI based FBC render tracking for ILK
From: Ville Syrjälä ILK should work pretty much the same as SNB, except it doesn't have the blitter, so we only care about render tracking. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_reg.h | 2 +- drivers/gpu/drm/i915/intel_pm.c | 2 -- drivers/gpu/drm/i915/intel_ringbuffer.c | 57 + 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 1777beb..db532cf 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1091,7 +1091,7 @@ #define ILK_DPFC_CHICKEN 0x43224 #define ILK_FBC_RT_BASE0x2128 #define ILK_FBC_RT_VALID (1<<0) -#define SNB_FBC_FRONT_BUFFER (1<<1) +#define ILK_FBC_FRONT_BUFFER (1<<1) #define ILK_DISPLAY_CHICKEN1 0x42000 #define ILK_FBCQ_DIS (1<<22) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index f46bb56..4039aa3 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -210,8 +210,6 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, (stall_watermark << DPFC_RECOMP_STALL_WM_SHIFT) | (interval << DPFC_RECOMP_TIMER_COUNT_SHIFT)); I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->y); - if (IS_GEN5(dev)) - I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID); /* enable it... */ I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 2e62d76..a02d472 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -77,6 +77,32 @@ gen2_render_ring_flush(struct intel_ring_buffer *ring, return 0; } +static int gen5_render_fbc_tracking(struct intel_ring_buffer *ring) +{ + int ret; + + if (!ring->fbc_address_dirty) + return 0; + + ret = intel_ring_begin(ring, 4); + if (ret) + return ret; + + intel_ring_emit(ring, MI_NOOP); + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); + intel_ring_emit(ring, ILK_FBC_RT_BASE); + if (ring->fbc_address != -1) + intel_ring_emit(ring, ring->fbc_address | + ILK_FBC_FRONT_BUFFER | ILK_FBC_RT_VALID); + else + intel_ring_emit(ring, 0); + intel_ring_advance(ring); + + ring->fbc_address_dirty = false; + + return 0; +} + static int gen4_render_ring_flush(struct intel_ring_buffer *ring, u32 invalidate_domains, @@ -132,6 +158,9 @@ gen4_render_ring_flush(struct intel_ring_buffer *ring, intel_ring_emit(ring, MI_NOOP); intel_ring_advance(ring); + if (invalidate_domains && IS_GEN5(dev)) + return gen5_render_fbc_tracking(ring); + return 0; } @@ -232,32 +261,6 @@ static int gen6_blt_fbc_tracking(struct intel_ring_buffer *ring) return 0; } -static int gen6_render_fbc_tracking(struct intel_ring_buffer *ring) -{ - int ret; - - if (!ring->fbc_address_dirty) - return 0; - - ret = intel_ring_begin(ring, 4); - if (ret) - return ret; - - intel_ring_emit(ring, MI_NOOP); - intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); - intel_ring_emit(ring, ILK_FBC_RT_BASE); - if (ring->fbc_address != -1) - intel_ring_emit(ring, ring->fbc_address | - SNB_FBC_FRONT_BUFFER | ILK_FBC_RT_VALID); - else - intel_ring_emit(ring, 0); - intel_ring_advance(ring); - - ring->fbc_address_dirty = false; - - return 0; -} - static int gen6_render_ring_flush(struct intel_ring_buffer *ring, u32 invalidate_domains, u32 flush_domains) @@ -308,7 +311,7 @@ gen6_render_ring_flush(struct intel_ring_buffer *ring, intel_ring_advance(ring); if (invalidate_domains) - return gen6_render_fbc_tracking(ring); + return gen5_render_fbc_tracking(ring); return 0; } -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] i915, debugfs: Fix uninitialized warning
From: Borislav Petkov gcc complains that: drivers/gpu/drm/i915/i915_debugfs.c: In function ‘display_crc_ctl_write’: drivers/gpu/drm/i915/i915_debugfs.c:2393:2: warning: ‘val’ may be used uninitialized in this function [-Wuninitialized] drivers/gpu/drm/i915/i915_debugfs.c:2350:6: note: ‘val’ was declared here but it can't see that we're going to use val only in the success case. So shut it up. Cc: Daniel Vetter Cc: David Airlie Cc: intel-gfx@lists.freedesktop.org Cc: dri-de...@lists.freedesktop.org Signed-off-by: Borislav Petkov --- drivers/gpu/drm/i915/i915_debugfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 6ed45a984230..1191aa47adc9 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2347,7 +2347,7 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, { struct drm_i915_private *dev_priv = dev->dev_private; struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; - u32 val; + u32 val = 0; /* shut up gcc */ int ret; if (pipe_crc->source == source) -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/19] drm/i915: add runtime put/get calls at the basic places
From: Paulo Zanoni If I add code to enable runtime PM on my Haswell machine, start a desktop environment, then enable runtime PM, these functions will complain that they're trying to read/write registers while the graphics card is suspended. Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/i915_gem.c| 53 +++--- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 drivers/gpu/drm/i915/i915_irq.c| 6 3 files changed, 46 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 40d9dcf..94c2a38 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1377,36 +1377,38 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf) drm_i915_private_t *dev_priv = dev->dev_private; pgoff_t page_offset; unsigned long pfn; - int ret = 0; + int rc = 0, ret; bool write = !!(vmf->flags & FAULT_FLAG_WRITE); + intel_runtime_pm_get(dev_priv); + /* We don't use vmf->pgoff since that has the fake offset */ page_offset = ((unsigned long)vmf->virtual_address - vma->vm_start) >> PAGE_SHIFT; - ret = i915_mutex_lock_interruptible(dev); - if (ret) + rc = i915_mutex_lock_interruptible(dev); + if (rc) goto out; trace_i915_gem_object_fault(obj, page_offset, true, write); /* Access to snoopable pages through the GTT is incoherent. */ if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(dev)) { - ret = -EINVAL; + rc = -EINVAL; goto unlock; } /* Now bind it into the GTT if needed */ - ret = i915_gem_obj_ggtt_pin(obj, 0, true, false); - if (ret) + rc = i915_gem_obj_ggtt_pin(obj, 0, true, false); + if (rc) goto unlock; - ret = i915_gem_object_set_to_gtt_domain(obj, write); - if (ret) + rc = i915_gem_object_set_to_gtt_domain(obj, write); + if (rc) goto unpin; - ret = i915_gem_object_get_fence(obj); - if (ret) + rc = i915_gem_object_get_fence(obj); + if (rc) goto unpin; obj->fault_mappable = true; @@ -1416,19 +1418,21 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf) pfn += page_offset; /* Finally, remap it using the new GTT offset */ - ret = vm_insert_pfn(vma, (unsigned long)vmf->virtual_address, pfn); + rc = vm_insert_pfn(vma, (unsigned long)vmf->virtual_address, pfn); unpin: i915_gem_object_unpin(obj); unlock: mutex_unlock(&dev->struct_mutex); out: - switch (ret) { + switch (rc) { case -EIO: /* If this -EIO is due to a gpu hang, give the reset code a * chance to clean up the mess. Otherwise return the proper * SIGBUS. */ - if (i915_terminally_wedged(&dev_priv->gpu_error)) - return VM_FAULT_SIGBUS; + if (i915_terminally_wedged(&dev_priv->gpu_error)) { + ret = VM_FAULT_SIGBUS; + break; + } case -EAGAIN: /* * EAGAIN means the gpu is hung and we'll wait for the error @@ -1443,15 +1447,22 @@ out: * EBUSY is ok: this just means that another thread * already did the job. */ - return VM_FAULT_NOPAGE; + ret = VM_FAULT_NOPAGE; + break; case -ENOMEM: - return VM_FAULT_OOM; + ret = VM_FAULT_OOM; + break; case -ENOSPC: - return VM_FAULT_SIGBUS; + ret = VM_FAULT_SIGBUS; + break; default: - WARN_ONCE(ret, "unhandled error in i915_gem_fault: %i\n", ret); - return VM_FAULT_SIGBUS; + WARN_ONCE(rc, "unhandled error in i915_gem_fault: %i\n", rc); + ret = VM_FAULT_SIGBUS; + break; } + + intel_runtime_pm_put(dev_priv); + return ret; } /** @@ -4165,6 +4176,8 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) drm_i915_private_t *dev_priv = dev->dev_private; struct i915_vma *vma, *next; + intel_runtime_pm_get(dev_priv); + trace_i915_gem_object_destroy(obj); if (obj->phys_obj) @@ -4209,6 +4222,8 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) kfree(obj->bit_17); i915_gem_object_free(obj); + + intel_runtime_pm_put(dev_priv); } struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj, diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 885d595..2c35a9d 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/d
Re: [Intel-gfx] [PATCH 12/19] drm/i915: release the GTT mmaps when going into D3
2013/11/21 Chris Wilson : > On Thu, Nov 21, 2013 at 01:47:26PM -0200, Paulo Zanoni wrote: >> From: Paulo Zanoni >> >> So we'll get a fault when someone tries to access the mmap, then we'll >> wake up from D3. > > Harsh. Very harsh. Is the GTT completely off-limits under pc8 Under D3, not PC8. > , or is it > only the GTT access to the display engine? i.e. could we get away with > only killing some of the mmaps? I don't have any documentation describing what should work and what shouldn't: all I know comes from our test suite, function gem_mmap_subtest: http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/tests/pm_pc8.c#n927 . On that test, we don't use the buffers on the display engine. We just do gem_create(), then gem_mmap__gtt(), write values to the buff, put the device in D3, then try to read the buffer contents and see if it is what we wrote. So I imagine the answer to your question is probably "no". If you have more ideas of tests I could write that would help answering your questions, please tell me:) > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] i915, debugfs: Fix uninitialized warning
On Thu, Nov 21, 2013 at 04:49:46PM +0100, Borislav Petkov wrote: > From: Borislav Petkov > > gcc complains that: > > drivers/gpu/drm/i915/i915_debugfs.c: In function ‘display_crc_ctl_write’: > drivers/gpu/drm/i915/i915_debugfs.c:2393:2: warning: ‘val’ may be used > uninitialized in this function [-Wuninitialized] > drivers/gpu/drm/i915/i915_debugfs.c:2350:6: note: ‘val’ was declared here > > but it can't see that we're going to use val only in the success case. > So shut it up. > > Cc: Daniel Vetter > Cc: David Airlie > Cc: intel-gfx@lists.freedesktop.org > Cc: dri-de...@lists.freedesktop.org > Signed-off-by: Borislav Petkov Queued for -next, thanks for the patch. -Daniel > --- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index 6ed45a984230..1191aa47adc9 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -2347,7 +2347,7 @@ static int pipe_crc_set_source(struct drm_device *dev, > enum pipe pipe, > { > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; > - u32 val; > + u32 val = 0; /* shut up gcc */ > int ret; > > if (pipe_crc->source == source) > -- > 1.8.4 > -- 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 08/19] drm/i915: add some runtime PM get/put calls
From: Paulo Zanoni These are needed when we cat the debugfs and sysfs files. V2: - Rebase V3: - Rebase Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/i915_debugfs.c | 45 ++--- drivers/gpu/drm/i915/i915_sysfs.c | 14 ++-- drivers/gpu/drm/i915/intel_dp.c | 11 +++-- drivers/gpu/drm/i915/intel_panel.c | 3 +++ drivers/gpu/drm/i915/intel_uncore.c | 4 5 files changed, 70 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index d1491f8..a1cb6fa 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -564,10 +564,12 @@ static int i915_gem_seqno_info(struct seq_file *m, void *data) ret = mutex_lock_interruptible(&dev->struct_mutex); if (ret) return ret; + intel_runtime_pm_get(dev_priv); for_each_ring(ring, dev_priv, i) i915_ring_seqno_info(m, ring); + intel_runtime_pm_put(dev_priv); mutex_unlock(&dev->struct_mutex); return 0; @@ -585,6 +587,7 @@ static int i915_interrupt_info(struct seq_file *m, void *data) ret = mutex_lock_interruptible(&dev->struct_mutex); if (ret) return ret; + intel_runtime_pm_get(dev_priv); if (INTEL_INFO(dev)->gen >= 8) { int i; @@ -711,6 +714,7 @@ static int i915_interrupt_info(struct seq_file *m, void *data) } i915_ring_seqno_info(m, ring); } + intel_runtime_pm_put(dev_priv); mutex_unlock(&dev->struct_mutex); return 0; @@ -904,9 +908,11 @@ static int i915_rstdby_delays(struct seq_file *m, void *unused) ret = mutex_lock_interruptible(&dev->struct_mutex); if (ret) return ret; + intel_runtime_pm_get(dev_priv); crstanddelay = I915_READ16(CRSTANDVID); + intel_runtime_pm_put(dev_priv); mutex_unlock(&dev->struct_mutex); seq_printf(m, "w/ctx: %d, w/o ctx: %d\n", (crstanddelay >> 8) & 0x3f, (crstanddelay & 0x3f)); @@ -919,7 +925,9 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused) struct drm_info_node *node = (struct drm_info_node *) m->private; struct drm_device *dev = node->minor->dev; drm_i915_private_t *dev_priv = dev->dev_private; - int ret; + int ret = 0; + + intel_runtime_pm_get(dev_priv); flush_delayed_work(&dev_priv->rps.delayed_resume_work); @@ -945,7 +953,7 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused) /* RPSTAT1 is in the GT power well */ ret = mutex_lock_interruptible(&dev->struct_mutex); if (ret) - return ret; + goto out; gen6_gt_force_wake_get(dev_priv); @@ -1033,7 +1041,9 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused) seq_puts(m, "no P-state info available\n"); } - return 0; +out: + intel_runtime_pm_put(dev_priv); + return ret; } static int i915_delayfreq_table(struct seq_file *m, void *unused) @@ -1047,6 +1057,7 @@ static int i915_delayfreq_table(struct seq_file *m, void *unused) ret = mutex_lock_interruptible(&dev->struct_mutex); if (ret) return ret; + intel_runtime_pm_get(dev_priv); for (i = 0; i < 16; i++) { delayfreq = I915_READ(PXVFREQ_BASE + i * 4); @@ -1054,6 +1065,8 @@ static int i915_delayfreq_table(struct seq_file *m, void *unused) (delayfreq & PXVFREQ_PX_MASK) >> PXVFREQ_PX_SHIFT); } + intel_runtime_pm_put(dev_priv); + mutex_unlock(&dev->struct_mutex); return 0; @@ -1075,12 +1088,14 @@ static int i915_inttoext_table(struct seq_file *m, void *unused) ret = mutex_lock_interruptible(&dev->struct_mutex); if (ret) return ret; + intel_runtime_pm_get(dev_priv); for (i = 1; i <= 32; i++) { inttoext = I915_READ(INTTOEXT_BASE_ILK + i * 4); seq_printf(m, "INTTOEXT%02d: 0x%08x\n", i, inttoext); } + intel_runtime_pm_put(dev_priv); mutex_unlock(&dev->struct_mutex); return 0; @@ -1098,11 +1113,13 @@ static int ironlake_drpc_info(struct seq_file *m) ret = mutex_lock_interruptible(&dev->struct_mutex); if (ret) return ret; + intel_runtime_pm_get(dev_priv); rgvmodectl = I915_READ(MEMMODECTL); rstdbyctl = I915_READ(RSTDBYCTL); crstandvid = I915_READ16(CRSTANDVID); + intel_runtime_pm_put(dev_priv); mutex_unlock(&dev->struct_mutex); seq_printf(m, "HD boost: %s\n", (rgvmodectl & MEMMODE_BOOST_EN) ? @@ -1166,6 +1183,7 @@ static int gen6_drpc_info(struct seq_file *m) ret = mutex_lock_interruptible(&dev->struct_mutex); i
[Intel-gfx] [PATCH 2/9] drm/i915: Don't set persistent FBC mode on ILK/SNB
From: Ville Syrjälä The ILK/SNB docs are a bit unclear what the persistent mode does, but the CTG docs clearly state that it was meant to be used when we're tracking back buffer modifications. We never do that, so leave it in non-persistent mode. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c9d1b0c..8cea92e 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -203,8 +203,6 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, dpfc_ctl = I915_READ(ILK_DPFC_CONTROL); dpfc_ctl &= DPFC_RESERVED; dpfc_ctl |= (plane | DPFC_CTL_LIMIT_1X); - /* Set persistent mode for front-buffer rendering, ala X. */ - dpfc_ctl |= DPFC_CTL_PERSISTENT_MODE; dpfc_ctl |= DPFC_CTL_FENCE_EN; if (IS_GEN5(dev)) dpfc_ctl |= obj->fence_reg; -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/19] drm/i915: disable interrupts when enabling PC8
From: Paulo Zanoni The plan is to merge PC8 and D3 into a single feature, and when we're in D3 we won't get any hotplug interrupt anyway, so leaving them enable doesn't make sense, and it also brings us a problem. The problem is that we get a hotplug interrupt right when we we wake up from D3, when we're still waking up everything. If we fully disable interrupts we won't get this hotplug interrupt, so we won't have problems. Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/i915_irq.c | 26 +- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 70c4cef..d0f4e61 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3902,8 +3902,8 @@ void hsw_pc8_disable_interrupts(struct drm_device *dev) dev_priv->pc8.regsave.gtier = I915_READ(GTIER); dev_priv->pc8.regsave.gen6_pmimr = I915_READ(GEN6_PMIMR); - ironlake_disable_display_irq(dev_priv, ~DE_PCH_EVENT_IVB); - ibx_disable_display_interrupt(dev_priv, ~SDE_HOTPLUG_MASK_CPT); + ironlake_disable_display_irq(dev_priv, 0x); + ibx_disable_display_interrupt(dev_priv, 0x); ilk_disable_gt_irq(dev_priv, 0x); snb_disable_pm_irq(dev_priv, 0x); @@ -3917,34 +3917,26 @@ void hsw_pc8_restore_interrupts(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; unsigned long irqflags; - uint32_t val, expected; + uint32_t val; spin_lock_irqsave(&dev_priv->irq_lock, irqflags); val = I915_READ(DEIMR); - expected = ~DE_PCH_EVENT_IVB; - WARN(val != expected, "DEIMR is 0x%08x, not 0x%08x\n", val, expected); + WARN(val != 0x, "DEIMR is 0x%08x\n", val); - val = I915_READ(SDEIMR) & ~SDE_HOTPLUG_MASK_CPT; - expected = ~SDE_HOTPLUG_MASK_CPT; - WARN(val != expected, "SDEIMR non-HPD bits are 0x%08x, not 0x%08x\n", -val, expected); + val = I915_READ(SDEIMR); + WARN(val != 0x, "SDEIMR is 0x%08x\n", val); val = I915_READ(GTIMR); - expected = 0x; - WARN(val != expected, "GTIMR is 0x%08x, not 0x%08x\n", val, expected); + WARN(val != 0x, "GTIMR is 0x%08x\n", val); val = I915_READ(GEN6_PMIMR); - expected = 0x; - WARN(val != expected, "GEN6_PMIMR is 0x%08x, not 0x%08x\n", val, -expected); + WARN(val != 0x, "GEN6_PMIMR is 0x%08x\n", val); dev_priv->pc8.irqs_disabled = false; ironlake_enable_display_irq(dev_priv, ~dev_priv->pc8.regsave.deimr); - ibx_enable_display_interrupt(dev_priv, -~dev_priv->pc8.regsave.sdeimr & -~SDE_HOTPLUG_MASK_CPT); + ibx_enable_display_interrupt(dev_priv, ~dev_priv->pc8.regsave.sdeimr); ilk_enable_gt_irq(dev_priv, ~dev_priv->pc8.regsave.gtimr); snb_enable_pm_irq(dev_priv, ~dev_priv->pc8.regsave.gen6_pmimr); I915_WRITE(GTIER, dev_priv->pc8.regsave.gtier); -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 14/19] drm/i915: add runtime PM support on Haswell
From: Paulo Zanoni The code to enable/disable PC8 already takes care of saving and restoring all the registers we need to save/restore, so do a put() call when we enable PC8 and a get() call when we disable it. Ideally, in order to make it easier to add runtime PM support to other platforms, we should move some things from the PC8 code to the runtime PM code, but let's do this later, since we can make Haswell work right now. V2: - Rebase Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/intel_display.c | 4 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3702746..002b99d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1836,7 +1836,7 @@ struct drm_i915_file_private { #define HAS_FPGA_DBG_UNCLAIMED(dev)(INTEL_INFO(dev)->has_fpga_dbg) #define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev)) #define HAS_PC8(dev) (IS_HASWELL(dev)) /* XXX HSW:ULX */ -#define HAS_RUNTIME_PM(dev)false +#define HAS_RUNTIME_PM(dev)(IS_HASWELL(dev)) #define INTEL_PCH_DEVICE_ID_MASK 0xff00 #define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 95e8831..820013a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6641,6 +6641,8 @@ void hsw_enable_pc8_work(struct work_struct *__work) lpt_disable_clkout_dp(dev); hsw_pc8_disable_interrupts(dev); hsw_disable_lcpll(dev_priv, true, true); + + intel_runtime_pm_put(dev_priv); } static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv) @@ -6678,6 +6680,8 @@ static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv) DRM_DEBUG_KMS("Disabling package C8+\n"); + intel_runtime_pm_get(dev_priv); + hsw_restore_lcpll(dev_priv); hsw_pc8_restore_interrupts(dev); lpt_init_pch_refclk(dev); -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/19] drm/i915: get a PC8 reference when enabling the power well
From: Paulo Zanoni In the current code, at haswell_modeset_global_resources, first we decide if we want to enable/disable the power well, then we decide if we want to enable/disable PC8. On the case where we're enabling PC8 this works fine, but on the case where we disable PC8 due to a non-eDP monitor being enabled, we first enable the power well and then disable PC8. Although wrong, this doesn't seem to be causing any problems now, and we don't even see anything in dmesg. But the patches for runtime D3 turn this problem into a real bug, so we need to fix it. This fixes the "modeset-non-lpsp" test from both "pc8" and "runtime_pm" tests from intel-gpu-tools. v2: - Rebase (i915_disable_power_well). Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_pm.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 04e9863..6bd0fc6 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5652,6 +5652,8 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable) unsigned long irqflags; uint32_t tmp; + WARN_ON(dev_priv->pc8.enabled); + tmp = I915_READ(HSW_PWR_WELL_DRIVER); is_enabled = tmp & HSW_PWR_WELL_STATE_ENABLED; enable_requested = tmp & HSW_PWR_WELL_ENABLE_REQUEST; @@ -5711,16 +5713,24 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable) static void __intel_power_well_get(struct drm_device *dev, struct i915_power_well *power_well) { - if (!power_well->count++) + struct drm_i915_private *dev_priv = dev->dev_private; + + if (!power_well->count++) { + hsw_disable_package_c8(dev_priv); __intel_set_power_well(dev, true); + } } static void __intel_power_well_put(struct drm_device *dev, struct i915_power_well *power_well) { + struct drm_i915_private *dev_priv = dev->dev_private; + WARN_ON(!power_well->count); - if (!--power_well->count && i915_disable_power_well) + if (!--power_well->count && i915_disable_power_well) { __intel_set_power_well(dev, false); + hsw_enable_package_c8(dev_priv); + } } void intel_display_power_get(struct drm_device *dev, -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: vlv: W/a for hotplug/manual VGA detection
At least on my VLV stepping VGA detection doesn't work in certain cases. One such case is when all pipes are off and VGA is plugged in. Another case reported by Joonas Lahtinen (also on the same stepping) is booting with VGA disconnected where we incorrectly report that VGA is connected. At least in the first case writing the FORCE bit in the ADPA reg will get stuck, i.e. the detection never completes. Both cases seem to be solved by disabling DPIO clock gating based on the PSR state. As I haven't found any trace that this would be a known issue, I can only speculate that both the DPIO HW block and the HW block responsible for VGA detection uses the same clock source which gets gated even though PSR is inactive. I haven't measured if and how this change affects our power savings. Reported-by: Joonas Lahtinen Signed-off-by: Imre Deak --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 5 + 2 files changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 849e595..9981d21 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1447,6 +1447,9 @@ # define I965_FT_CLOCK_GATE_DISABLE(1 << 1) # define I965_DM_CLOCK_GATE_DISABLE(1 << 0) +#define DPPSR_CGDIS_VLV(dev_priv->info->display_mmio_offset + 0x6204) +# define DPIOUNIT_PSR_CLOCK_GATING_DISABLE (1 << 6) + #define RENCLK_GATE_D2 0x6208 #define VF_UNIT_CLOCK_GATE_DISABLE (1 << 9) #define GS_UNIT_CLOCK_GATE_DISABLE (1 << 7) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 365545f..7f373e9 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5430,6 +5430,11 @@ static void valleyview_init_clock_gating(struct drm_device *dev) I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE); + /* Wa to make VGA hotplug and manual detection work. */ + val = I915_READ(DPPSR_CGDIS_VLV); + val |= DPIOUNIT_PSR_CLOCK_GATING_DISABLE; + I915_WRITE(DPIOUNIT_PSR_CLOCK_GATING_DISABLE, val); + /* WaDisableEarlyCull:vlv */ I915_WRITE(_3D_CHICKEN3, _MASKED_BIT_ENABLE(_3D_CHICKEN_SF_DISABLE_OBJEND_CULL)); -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/19] drm/i915: add runtime put/get calls at the basic places
On Thu, Nov 21, 2013 at 01:47:21PM -0200, Paulo Zanoni wrote: > From: Paulo Zanoni > > If I add code to enable runtime PM on my Haswell machine, start a > desktop environment, then enable runtime PM, these functions will > complain that they're trying to read/write registers while the > graphics card is suspended. > > Signed-off-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/i915_gem.c| 53 > +++--- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 > drivers/gpu/drm/i915/i915_irq.c| 6 > 3 files changed, 46 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 40d9dcf..94c2a38 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1377,36 +1377,38 @@ int i915_gem_fault(struct vm_area_struct *vma, struct > vm_fault *vmf) > drm_i915_private_t *dev_priv = dev->dev_private; > pgoff_t page_offset; > unsigned long pfn; > - int ret = 0; > + int rc = 0, ret; Ugh. Just keep ret and don't add rc. -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 12/19] drm/i915: release the GTT mmaps when going into D3
From: Paulo Zanoni So we'll get a fault when someone tries to access the mmap, then we'll wake up from D3. This fixes the gem-mmap-gtt subtest from pm_pc8 from intel-gpu-tools. Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/i915_drv.c | 2 ++ drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 8 3 files changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index b133836..6510483 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -912,6 +912,8 @@ static int i915_runtime_suspend(struct device *device) DRM_DEBUG_KMS("Suspending device\n"); + i915_gem_release_all_mmaps(dev_priv); + dev_priv->pm.suspended = true; intel_opregion_notify_adapter(dev, PCI_D3cold); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3877a68..3702746 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2003,6 +2003,7 @@ void i915_gem_object_unpin(struct drm_i915_gem_object *obj); int __must_check i915_vma_unbind(struct i915_vma *vma); int __must_check i915_gem_object_ggtt_unbind(struct drm_i915_gem_object *obj); int i915_gem_object_put_pages(struct drm_i915_gem_object *obj); +void i915_gem_release_all_mmaps(struct drm_i915_private *dev_priv); void i915_gem_release_mmap(struct drm_i915_gem_object *obj); void i915_gem_lastclose(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 94c2a38..68fa7c7 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1465,6 +1465,14 @@ out: return ret; } +void i915_gem_release_all_mmaps(struct drm_i915_private *dev_priv) +{ + struct drm_i915_gem_object *obj; + + list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) + i915_gem_release_mmap(obj); +} + /** * i915_gem_release_mmap - remove physical page mappings * @obj: obj in question -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 8/9] drm/i915: Hook up dirtyfb ioctl for FBC nuke
From: Ville Syrjälä FBC host modification tracking only works through GTT mmaps, so any direct CPU access needs to manually nuke the compressed framebuffer on modifications. Hook up the dirtyfb ioctl to do just that. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 19 +++ drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 30 ++ 3 files changed, 50 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index af0eafe..e5de929 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10385,8 +10385,27 @@ static int intel_user_framebuffer_create_handle(struct drm_framebuffer *fb, return drm_gem_handle_create(file, &obj->base, handle); } +static int intel_user_framebuffer_dirty(struct drm_framebuffer *fb, + struct drm_file *file_priv, + unsigned flags, unsigned color, + struct drm_clip_rect *clips, + unsigned num_clips) +{ + struct drm_device *dev = fb->dev; + + if (!I915_HAS_FBC(dev)) + return 0; + + mutex_lock(&dev->struct_mutex); + intel_fbc_nuke(fb); + mutex_unlock(&dev->struct_mutex); + + return 0; +} + static const struct drm_framebuffer_funcs intel_fb_funcs = { .destroy = intel_user_framebuffer_destroy, + .dirty = intel_user_framebuffer_dirty, .create_handle = intel_user_framebuffer_create_handle, }; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index f9e9ca0..c11ef8a 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -840,6 +840,7 @@ void intel_fbc_flip_start(struct drm_crtc *crtc, struct drm_framebuffer *fb); void intel_fbc_flip_end(struct drm_crtc *crtc, struct drm_framebuffer *fb); +void intel_fbc_nuke(struct drm_framebuffer *fb); void intel_update_fbc(struct drm_device *dev); void intel_gpu_ips_init(struct drm_i915_private *dev_priv); void intel_gpu_ips_teardown(void); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2a613ac..d84630a 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -508,6 +508,36 @@ void intel_fbc_flip_end(struct drm_crtc *crtc, intel_update_fbc_fb(crtc, fb); } +void intel_fbc_nuke(struct drm_framebuffer *fb) +{ + struct drm_device *dev = fb->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_crtc *crtc; + + if (dev_priv->fbc.fbc_work) { + if (dev_priv->fbc.fbc_work->fb != fb) + return; + + crtc = dev_priv->fbc.fbc_work->crtc; + } else { + if (dev_priv->fbc.fb != fb) + return; + + if (WARN_ON(dev_priv->fbc.plane < 0)) + return; + + crtc = dev_priv->plane_to_crtc_mapping[dev_priv->fbc.plane]; + } + + intel_disable_fbc(dev); + + /* +* Must wait until the next vblank before re-enabling +* otherwise the nuking won't actually happen. +*/ + intel_enable_fbc(crtc, 500); +} + static bool set_no_fbc_reason(struct drm_i915_private *dev_priv, enum no_fbc_reason reason) { -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/9] drm/i915: Improve page flip vs. FBC interaction
From: Ville Syrjälä On FBC2 (CTG+) page flips will automagically nuke FBC, so the only thing we need to do on page flip is update the CPU fence information. On FBC1 we need to to disable+re-enable FBC around page flips. Previously we just called intel_disable_fbc() + intel_update_fbc() to do that. However intel_update_fbc() would actually need to grab all the modeset locks, which we can't do from the driver workqueue. Make things a bit more straightforward by simply disabling FBC only if it's active/scheduled to be active on the current CRTC, and restart it on the same CRTC after the flip. FIXME: Several FBC1 docs actually seem to indicate that sync flips will also cause a nuke on FBC1. Need to verify this. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 5 +- drivers/gpu/drm/i915/intel_drv.h | 4 ++ drivers/gpu/drm/i915/intel_pm.c | 96 +++- 3 files changed, 102 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4155814..af0eafe 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8222,7 +8222,7 @@ static void intel_unpin_work_fn(struct work_struct *__work) drm_gem_object_unreference(&work->pending_flip_obj->base); drm_gem_object_unreference(&work->old_fb_obj->base); - intel_update_fbc(dev); + intel_fbc_flip_end(work->crtc, work->crtc->fb); mutex_unlock(&dev->struct_mutex); BUG_ON(atomic_read(&to_intel_crtc(work->crtc)->unpin_work_count) == 0); @@ -8661,7 +8661,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, if (ret) goto cleanup_pending; - intel_disable_fbc(dev); + intel_fbc_flip_start(crtc, fb); + intel_mark_fb_busy(obj, NULL); mutex_unlock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 119bb95..f9e9ca0 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -836,6 +836,10 @@ void intel_update_sprite_watermarks(struct drm_plane *plane, bool enabled, bool scaled); void intel_init_pm(struct drm_device *dev); bool intel_fbc_enabled(struct drm_device *dev); +void intel_fbc_flip_start(struct drm_crtc *crtc, + struct drm_framebuffer *fb); +void intel_fbc_flip_end(struct drm_crtc *crtc, + struct drm_framebuffer *fb); void intel_update_fbc(struct drm_device *dev); void intel_gpu_ips_init(struct drm_i915_private *dev_priv); void intel_gpu_ips_teardown(void); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 4039aa3..2a613ac 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -363,6 +363,8 @@ static void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval) if (!dev_priv->display.enable_fbc) return; + WARN_ON(dev_priv->fbc.fbc_work && dev_priv->fbc.plane != -1); + intel_cancel_fbc_work(dev_priv); work = kzalloc(sizeof(*work), GFP_KERNEL); @@ -401,6 +403,8 @@ void intel_disable_fbc(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; + WARN_ON(dev_priv->fbc.fbc_work && dev_priv->fbc.plane != -1); + intel_cancel_fbc_work(dev_priv); if (!dev_priv->display.disable_fbc) @@ -414,6 +418,96 @@ void intel_disable_fbc(struct drm_device *dev) } } +static void intel_update_fbc_fb(struct drm_crtc *crtc, + struct drm_framebuffer *fb) +{ + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_fbc_work *work = dev_priv->fbc.fbc_work; + + /* +* If we've just scheduled an fbc work, +* simply update the fb for the work. +*/ + if (work) { + WARN_ON(dev_priv->fbc.plane != -1); + + if (work->crtc != crtc) + return; + + drm_framebuffer_reference(fb); + drm_framebuffer_unreference(work->fb); + work->fb = fb; + } + + if (dev_priv->fbc.plane == to_intel_crtc(crtc)->plane) { + WARN_ON(dev_priv->fbc.fbc_work); + + if (dev_priv->fbc.fb == fb && + dev_priv->fbc.y == crtc->y) + return; + + intel_setup_fbc(crtc, fb, 500); + } +} + +void intel_fbc_flip_start(struct drm_crtc *crtc, + struct drm_framebuffer *fb) +{ + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_fbc_work *work = dev_priv->fbc.fbc_work; + + if (!I915_HAS_FBC(dev)) + return; + + WARN_ON(work && dev_priv->fbc.plane != -1); + + /* just need to upda
[Intel-gfx] [PATCH 1/9] drm/i915: Don't set the fence number in DPFC_CTL on SNB
From: Ville Syrjälä SNB has another register where the actual FBC CPU fence number is stored. The documenation explicitly states that the fence number in DPFC_CTL must be 0 on SNB. And in fact when it's not zero, the GTT tracking simply doesn't work. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 53ba1a2..c9d1b0c 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -205,7 +205,9 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, dpfc_ctl |= (plane | DPFC_CTL_LIMIT_1X); /* Set persistent mode for front-buffer rendering, ala X. */ dpfc_ctl |= DPFC_CTL_PERSISTENT_MODE; - dpfc_ctl |= (DPFC_CTL_FENCE_EN | obj->fence_reg); + dpfc_ctl |= DPFC_CTL_FENCE_EN; + if (IS_GEN5(dev)) + dpfc_ctl |= obj->fence_reg; I915_WRITE(ILK_DPFC_CHICKEN, DPFC_HT_MODIFY); I915_WRITE(ILK_DPFC_RECOMP_CTL, DPFC_RECOMP_STALL_EN | -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] i915, debugfs: Fix uninitialized warning
On Thu, Nov 21, 2013 at 05:10:30PM +0100, Richard Weinberger wrote: > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > b/drivers/gpu/drm/i915/i915_debugfs.c > > index 6ed45a984230..1191aa47adc9 100644 > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > @@ -2347,7 +2347,7 @@ static int pipe_crc_set_source(struct drm_device > > *dev, enum pipe pipe, > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; > > - u32 val; > > + u32 val = 0; /* shut up gcc */ > > Wouldn't it be better to use uninitialized_var() here? I remember Linus' rant about this macro so that's why I don't use it anymore. In this specific case, it doesn't matter whichever we do so I'll let the maintainer make a wish :) -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] AVI infoframes: default aspect ratio/VIC for CEA modes
On Thu, Nov 21, 2013 at 07:19:45PM +0200, Ville Syrjälä wrote: > > > It seems natural to extend those flags to describe the picture aspect > > > ratio (that > > > why dri-devel is in Cc., touching core DRM). > > > > To start with we can use a single bit in drm_display_mode->flags to > > distinguish 16:9 vs 4:3. > > I must admit I haven't really looked into the matter, but do we actually > need any mode bits? Would it be enoguh to just have a property that > allows the user to specify the picture aspect ratio? Well, that's a good question! the picture aspect ratio is attached to modes in CEA-861, but it could be a good idea to have an orthogonal picture a-r property on the crtc. Not quite sure at this point, needs a bit more understanding of the problem. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/19] drm/i915: WARN if !HAS_PC8 when enabling/disabling PC8
From: Paulo Zanoni We already have some checks and shouldn't be reaching these places on !HAS_PC8 platforms, but add a WARN, just in case. Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_display.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e85d838..5566de5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6623,6 +6623,8 @@ void hsw_enable_pc8_work(struct work_struct *__work) struct drm_device *dev = dev_priv->dev; uint32_t val; + WARN_ON(!HAS_PC8(dev)); + if (dev_priv->pc8.enabled) return; @@ -6668,6 +6670,8 @@ static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv) if (dev_priv->pc8.disable_count != 1) return; + WARN_ON(!HAS_PC8(dev)); + cancel_delayed_work_sync(&dev_priv->pc8.enable_work); if (!dev_priv->pc8.enabled) return; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/9] drm: Push dirtyfb ioctl kms locking down to drivers
From: Ville Syrjälä Not all drivers will need take all the modeset locks for dirtyfb, so push the locking down to the drivers. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_crtc.c | 2 -- drivers/gpu/drm/omapdrm/omap_fb.c | 4 drivers/gpu/drm/qxl/qxl_display.c | 9 - drivers/gpu/drm/udl/udl_fb.c| 12 +--- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 18 -- 5 files changed, 37 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d6cf77c..266a01d 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2767,10 +2767,8 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev, } if (fb->funcs->dirty) { - drm_modeset_lock_all(dev); ret = fb->funcs->dirty(fb, file_priv, flags, r->color, clips, num_clips); - drm_modeset_unlock_all(dev); } else { ret = -ENOSYS; } diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c index f2b8f06..f466c4a 100644 --- a/drivers/gpu/drm/omapdrm/omap_fb.c +++ b/drivers/gpu/drm/omapdrm/omap_fb.c @@ -123,12 +123,16 @@ static int omap_framebuffer_dirty(struct drm_framebuffer *fb, { int i; + drm_modeset_lock_all(fb->dev); + for (i = 0; i < num_clips; i++) { omap_framebuffer_flush(fb, clips[i].x1, clips[i].y1, clips[i].x2 - clips[i].x1, clips[i].y2 - clips[i].y1); } + drm_modeset_unlock_all(fb->dev); + return 0; } diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index 5e827c2..b8f3bc7 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -399,10 +399,14 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb, struct qxl_bo *qobj; int inc = 1; + drm_modeset_lock_all(fb->dev); + qobj = gem_to_qxl_bo(qxl_fb->obj); /* if we aren't primary surface ignore this */ - if (!qobj->is_primary) + if (!qobj->is_primary) { + drm_modeset_unlock_all(fb->dev); return 0; + } if (!num_clips) { num_clips = 1; @@ -417,6 +421,9 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb, qxl_draw_dirty_fb(qdev, qxl_fb, qobj, flags, color, clips, num_clips, inc); + + drm_modeset_unlock_all(fb->dev); + return 0; } diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c index 97e9d61..dbadd49 100644 --- a/drivers/gpu/drm/udl/udl_fb.c +++ b/drivers/gpu/drm/udl/udl_fb.c @@ -403,15 +403,17 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb, int i; int ret = 0; + drm_modeset_lock_all(fb->dev); + if (!ufb->active_16) - return 0; + goto unlock; if (ufb->obj->base.import_attach) { ret = dma_buf_begin_cpu_access(ufb->obj->base.import_attach->dmabuf, 0, ufb->obj->base.size, DMA_FROM_DEVICE); if (ret) - return ret; + goto unlock; } for (i = 0; i < num_clips; i++) { @@ -419,7 +421,7 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb, clips[i].x2 - clips[i].x1, clips[i].y2 - clips[i].y1); if (ret) - break; + goto unlock; } if (ufb->obj->base.import_attach) { @@ -427,6 +429,10 @@ static int udl_user_framebuffer_dirty(struct drm_framebuffer *fb, 0, ufb->obj->base.size, DMA_FROM_DEVICE); } + + unlock: + drm_modeset_unlock_all(fb->dev); + return ret; } diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index ecb3d86..ab0b88f 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -608,9 +608,13 @@ int vmw_framebuffer_surface_dirty(struct drm_framebuffer *framebuffer, if (!dev_priv->sou_priv) return -EINVAL; + drm_modeset_lock_all(dev_priv->dev); + ret = ttm_read_lock(&vmaster->lock, true); - if (unlikely(ret != 0)) + if (unlikely(ret != 0)) { + drm_modeset_unlock_all(dev_priv->dev); return ret; + } if (!num_clips) { num_clips = 1; @@ -628,6 +632,9 @@ int vmw_framebuffer_surface_dirty(struct drm_framebuffer *framebuffer, clips, num_clips, inc, NULL);
[Intel-gfx] [PATCH 04/19] drm/i915: get/put PC8 when we get/put a CRTC
From: Paulo Zanoni Currently, PC8 is enabled at modeset_global_resources, which is called after intel_modeset_update_state. Due to this, there's a small race condition on the case where we start enabling PC8, then do a modeset while PC8 is still being enabled. The racing condition triggers a WARN because intel_modeset_update_state will mark the CRTC as enabled, then the thread that's still enabling PC8 might look at the data structure and think that PC8 is being enabled while a pipe is enabled. Despite the WARN, this is not really a bug since we'll wait for the PC8-enabling thread to finish when we call modeset_global_resources. So this patch makes sure we get/put PC8 before we update drm_crtc->enabled, because if a get() call triggers a PC8 disable, we'll call cancel_delayed_work_sync(), which will wait for the thread that's enabling PC8, then, after this, we'll disable PC8. The side-effect benefit of this patch is that we have a nice place to track enabled/disabled CRTCs, so we may want to move some code from modeset_global_resources to intel_crtc_set_state in the future. The problem fixed by this patch can be reproduced by the modeset-lpsp-stress-no-wait subtest from the pc8 test of intel-gpu-tools. v2: - No need for pc8.lock since we already have cancel_delayed_work_sync(). Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_display.c | 27 +++ 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5ea32a8..846f2de 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9124,6 +9124,24 @@ static bool intel_crtc_in_use(struct drm_crtc *crtc) return false; } +/* Sets crtc->base.enabled and gets/puts whatever resources are needed by the + * CRTC. */ +static void intel_crtc_set_state(struct intel_crtc *crtc, bool enabled) +{ + struct drm_device *dev = crtc->base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + + if (enabled == crtc->base.enabled) + return; + + if (enabled) + hsw_disable_package_c8(dev_priv); + else + hsw_enable_package_c8(dev_priv); + + crtc->base.enabled = enabled; +} + static void intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes) { @@ -9147,7 +9165,8 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes) /* Update computed state. */ list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list, base.head) { - intel_crtc->base.enabled = intel_crtc_in_use(&intel_crtc->base); + intel_crtc_set_state(intel_crtc, +intel_crtc_in_use(&intel_crtc->base)); } list_for_each_entry(connector, &dev->mode_config.connector_list, head) { @@ -10956,7 +10975,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) } WARN_ON(crtc->active); - crtc->base.enabled = false; + intel_crtc_set_state(crtc, false); } if (dev_priv->quirks & QUIRK_PIPEA_FORCE && @@ -10983,7 +11002,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) crtc->base.enabled ? "enabled" : "disabled", crtc->active ? "enabled" : "disabled"); - crtc->base.enabled = crtc->active; + intel_crtc_set_state(crtc, crtc->active); /* Because we only establish the connector -> encoder -> * crtc links if something is active, this means the @@ -11080,7 +11099,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) crtc->active = dev_priv->display.get_pipe_config(crtc, &crtc->config); - crtc->base.enabled = crtc->active; + intel_crtc_set_state(crtc, crtc->active); crtc->primary_enabled = crtc->active; DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n", -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/9] drm/i915: Some more FBC stuff
Another set of FBC patches, which should fit on top of the previous set: "[PATCH 00/10] drm/i915: FBC fixes v2" The persistent mode and HT tracking bit stuff is a bit unclear in the docs, but I can remove it all, and everything still seems to work fine. The page flip and dirtyfb stuff is maybe a bit raw, but I'll post anyway now since it seems to work for me. I'll post my igt test case that tries to stress all this shortly. It passes for me on ILK, SNB and IVB. On ILK it's a bit limited since there are no contexts (didn't try the ILK context patches w/ this) and we're missing a gen5 rendercopy, so I couldn't test the render tracking using igt. But I don't get any screen corruption w/ FBC enabled, so it must be working. The only FBC1 capable hardware on my desk is a MGM, but someone was a bit too conservative when they implemented FBC1 support and enabled it only for CL. I was too lazy to read through the code to see if it should work for MGM. Ville Syrjälä (9): drm/i915: Don't set the fence number in DPFC_CTL on SNB drm/i915: Don't set persistent FBC mode on ILK/SNB drm/i915: Don't set DPFC_HT_MODIFY bit on CTG/ILK/SNB drm/i915: Use LRI based FBC render tracking for ILK drm/i915: Reorder i915_gem_execbuffer_move_to_gpu() and i915_switch_context() drm/i915: Improve page flip vs. FBC interaction drm: Push dirtyfb ioctl kms locking down to drivers drm/i915: Hook up dirtyfb ioctl for FBC nuke drm/i915: Flush caches for scanout during cpu->gtt move drivers/gpu/drm/drm_crtc.c | 2 - drivers/gpu/drm/i915/i915_gem.c| 2 +- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 8 +- drivers/gpu/drm/i915/i915_reg.h| 2 +- drivers/gpu/drm/i915/intel_display.c | 24 - drivers/gpu/drm/i915/intel_drv.h | 5 ++ drivers/gpu/drm/i915/intel_pm.c| 136 +++-- drivers/gpu/drm/i915/intel_ringbuffer.c| 57 ++-- drivers/gpu/drm/omapdrm/omap_fb.c | 4 + drivers/gpu/drm/qxl/qxl_display.c | 9 +- drivers/gpu/drm/udl/udl_fb.c | 12 ++- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c| 18 +++- 12 files changed, 228 insertions(+), 51 deletions(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/19] drm/i915: get/put PC8 when we get/put a CRTC
On Thu, Nov 21, 2013 at 01:47:18PM -0200, Paulo Zanoni wrote: > From: Paulo Zanoni > > Currently, PC8 is enabled at modeset_global_resources, which is called > after intel_modeset_update_state. Due to this, there's a small race > condition on the case where we start enabling PC8, then do a modeset > while PC8 is still being enabled. The racing condition triggers a WARN > because intel_modeset_update_state will mark the CRTC as enabled, then > the thread that's still enabling PC8 might look at the data structure > and think that PC8 is being enabled while a pipe is enabled. Despite > the WARN, this is not really a bug since we'll wait for the > PC8-enabling thread to finish when we call modeset_global_resources. > > So this patch makes sure we get/put PC8 before we update > drm_crtc->enabled, because if a get() call triggers a PC8 disable, > we'll call cancel_delayed_work_sync(), which will wait for the thread > that's enabling PC8, then, after this, we'll disable PC8. > > The side-effect benefit of this patch is that we have a nice place to > track enabled/disabled CRTCs, so we may want to move some code from > modeset_global_resources to intel_crtc_set_state in the future. > > The problem fixed by this patch can be reproduced by the > modeset-lpsp-stress-no-wait subtest from the pc8 test of > intel-gpu-tools. > > v2: - No need for pc8.lock since we already have > cancel_delayed_work_sync(). > > Signed-off-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/intel_display.c | 27 +++ > 1 file changed, 23 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 5ea32a8..846f2de 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9124,6 +9124,24 @@ static bool intel_crtc_in_use(struct drm_crtc *crtc) > return false; > } > > +/* Sets crtc->base.enabled and gets/puts whatever resources are needed by the > + * CRTC. */ > +static void intel_crtc_set_state(struct intel_crtc *crtc, bool enabled) set_state is too generic a term, intel_crtc_set_enabled() > +{ > + struct drm_device *dev = crtc->base.dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + if (enabled == crtc->base.enabled) > + return; > + > + if (enabled) > + hsw_disable_package_c8(dev_priv); > + else > + hsw_enable_package_c8(dev_priv); We can reduce the code slightly if this was also hsw_package_c8_set_enabled(). -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 06/19] drm/i915: do adapter power state notification at runtime PM
From: Paulo Zanoni Now that we are actually setting the device to the D3 state, we should issue the notification. Jani originally wrote a similar patch for PC8, but then we discovered that we were not really changing the PCI D states when enabling/disabling PC8, so we had to postpone his patch. Cc: Jani Nikula Credits-to: Jani Nikula Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/i915_drv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index d7ff095..b133836 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -914,6 +914,7 @@ static int i915_runtime_suspend(struct device *device) dev_priv->pm.suspended = true; + intel_opregion_notify_adapter(dev, PCI_D3cold); pci_save_state(pdev); pci_set_power_state(pdev, PCI_D3cold); @@ -932,6 +933,7 @@ static int i915_runtime_resume(struct device *device) pci_set_power_state(pdev, PCI_D0); pci_restore_state(pdev); + intel_opregion_notify_adapter(dev, PCI_D0); dev_priv->pm.suspended = false; -- 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] i915, fbdev: Fix Kconfig typo
On Thu, Nov 21, 2013 at 03:29:55PM +0100, Borislav Petkov wrote: > From: Borislav Petkov > > Too many t's. > > Cc: Daniel Vetter > Cc: intel-gfx@lists.freedesktop.org > Cc: dri-de...@lists.freedesktop.org > Signed-off-by: Borislav Petkov Queued for -next, thanks for the patch. -Daniel > --- > drivers/gpu/drm/i915/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig > index 6199d0b5b958..7ca35ecfd952 100644 > --- a/drivers/gpu/drm/i915/Kconfig > +++ b/drivers/gpu/drm/i915/Kconfig > @@ -43,7 +43,7 @@ config DRM_I915_KMS > like intelfb. > > config DRM_I915_FBDEV > - bool "Enable legacy fbdev support for the modesettting intel driver" > + bool "Enable legacy fbdev support for the modesetting intel driver" > depends on DRM_I915 > select DRM_KMS_FB_HELPER > select FB_CFB_FILLRECT > -- > 1.8.4 > -- 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 v4 06/10] drm/i915: Implement LRI based FBC tracking
On Thu, Nov 21, 2013 at 04:39:43PM +, Chris Wilson wrote: > On Thu, Nov 21, 2013 at 06:33:21PM +0200, Ville Syrjälä wrote: > > On Thu, Nov 21, 2013 at 11:49:47AM +, Chris Wilson wrote: > > > On Thu, Nov 21, 2013 at 01:14:10PM +0200, ville.syrj...@linux.intel.com > > > wrote: > > > > From: Ville Syrjälä > > > > > > > > As per the SNB and HSW PM guides, we should enable FBC render/blitter > > > > tracking only during batches targetting the front buffer. > > > > > > > > On SNB we must also update the FBC render tracking address whenever it > > > > changes. And since the register in question is stored in the context, > > > > we need to make sure we reload it with correct data after context > > > > switches. > > > > > > > > On IVB/HSW we use the render nuke mechanism, so no render tracking > > > > address updates are needed. Hoever on the blitter side we need to > > > > enable the blitter tracking like on SNB, and in addition we need > > > > to issue the cache clean messages, which we already did. > > > > > > > > v2: Introduce intel_fb_obj_has_fbc() > > > > Fix crtc locking around crtc->fb access > > > > Drop a hunk that was included by accident in v1 > > > > Set fbc_address_dirty=false not true after emitting the LRI > > > > v3: Now that fbc hangs on to the fb intel_fb_obj_has_fbc() doesn't > > > > need to upset lockdep anymore > > > > v4: Use |= instead of = to update fbc_address_dirty > > > > > > Ah, should we do the same for fbc_dirty? In the past we could dispatch a > > > batchbuffer but fail to add the request (and so fail to flush the > > > rendering/fbc). We currently preallocate the request so that failure > > > path is history, but we will more than likely be caught out again in the > > > future. > > > > I guess we could do it for fbc_dirty as well, but I don't think that > > should actually change anything. Either we're rendering to the FBC > > scanout buffer, or we're not. > > The scenario I worry about here is the missing flush after the > rendering. It has been possible for us to lose it (under memory > pressure, other constaints etc) and to issue a catch-up flush on the > next batch. Without the or, we'd lose the FBC flush. It is just a > potential issue for the future. > > > I did start pondering if I should actually move the fbc_address to live > > under the context once that's where it actually belongs. If we'd track > > it per-context we might be able to avoid emitting the LRI for every > > context switch. > > > > > > > > Like pc8, I'd like for a device (or crtc if you must) property whether > > > or not indirect rendering is preferred. > > > > > > Other than that and the bikeshed to kill the redundant fbc_obj local > > > variable and pack the dirty bits, it looks good to me. > > > > Actually I just fired it up for real on SNB and it failed. The problem > > is that we end up doing the invalidate before the context switch. So > > we've not yet forced fbc_address_dirty=true due to the context switch > > when we do the invalidate. The most straightforward fix would be to > > simply move i915_gem_execbuffer_move_to_gpu() to be called after > > i915_switch_context(). I did that on my machine and now it passes my > > context related FBC tests. But I do wonder if the order of these > > operations was chose for a reason, and whether the reordering might > > cause other problems. > > Ben, opinion on whether the ordering was just convenience? > -Chris If it was anything but convenience, I've long since forgotten. I guess as long as we do it in two patches we can always bisect, and it's all good. > > -- > Chris Wilson, Intel Open Source Technology Centre -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx