From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> We shouldn't disable the trickle feed bits on Haswell. Our documentation explicitly says the trickle feed bits of PRI_CTL and CUR_CTL should not be programmed to 1, and the hardware engineer also asked us to not program the SPR_CTL field to 1. Leaving the bits as 1 could cause underflows. Reported-by: Arthur Runyan <arthur.j.runyan@xxxxxxxxx> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_display.c | 10 +++++++--- drivers/gpu/drm/i915/intel_pm.c | 2 -- drivers/gpu/drm/i915/intel_sprite.c | 7 +++++-- 4 files changed, 13 insertions(+), 7 deletions(-) Some side discussions: 1 - It seems we always do read-modify-write with the PRI/SPR/CUR registers. I think this is dangerous since we may forget to disable something and keep our code in a bugged state forever. Shouldn't we try to completely set the full state of the bits from scratch every time we enable PRI/SPR/CUR? 2 - We also have code to enable the trickle feed bits at init_clock_gating. Isn't this dangerous? Trickle fee changes the memory is read, my first guess would be that it's probably not safe to change this bit when things are enabled. Also, we lose the state of these bits when the power well is disabled, so the code in init_clock_gating really makes no sense on Haswell. diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index bbbc177..e92bb47 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3312,6 +3312,7 @@ #define MCURSOR_PIPE_A 0x00 #define MCURSOR_PIPE_B (1 << 28) #define MCURSOR_GAMMA_ENABLE (1 << 26) +#define CURSOR_TRICKLE_FEED_DISABLE (1 << 14) #define _CURABASE (dev_priv->info->display_mmio_offset + 0x70084) #define _CURAPOS (dev_priv->info->display_mmio_offset + 0x70088) #define CURSOR_POS_MASK 0x007FF diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c64631d..d5f038e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2077,8 +2077,10 @@ static int ironlake_update_plane(struct drm_crtc *crtc, else dspcntr &= ~DISPPLANE_TILED; - /* must disable */ - dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE; + if (IS_HASWELL(dev)) + dspcntr &= ~DISPPLANE_TRICKLE_FEED_DISABLE; + else + dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE; I915_WRITE(reg, dspcntr); @@ -6768,8 +6770,10 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base) cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE); cntl |= CURSOR_MODE_DISABLE; } - if (IS_HASWELL(dev)) + if (IS_HASWELL(dev)) { cntl |= CURSOR_PIPE_CSC_ENABLE; + cntl &= ~CURSOR_TRICKLE_FEED_DISABLE; + } I915_WRITE(CURCNTR_IVB(pipe), cntl); intel_crtc->cursor_visible = visible; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 4605682..3a5c0bb 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4950,8 +4950,6 @@ static void haswell_init_clock_gating(struct drm_device *dev) I915_READ(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG) | GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB); - g4x_disable_trickle_feed(dev); - /* WaVSRefCountFullforceMissDisable:hsw */ gen7_setup_fixed_func_scheduler(dev_priv); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 78b621c..ad6ec4b 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -260,8 +260,11 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, if (obj->tiling_mode != I915_TILING_NONE) sprctl |= SPRITE_TILED; - /* must disable */ - sprctl |= SPRITE_TRICKLE_FEED_DISABLE; + if (IS_HASWELL(dev)) + sprctl &= ~SPRITE_TRICKLE_FEED_DISABLE; + else + sprctl |= SPRITE_TRICKLE_FEED_DISABLE; + sprctl |= SPRITE_ENABLE; if (IS_HASWELL(dev)) -- 1.8.1.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx