| Message ID | 1350553794-5534-2-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 18 Oct 2012 11:49:50 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > Or at least our best understanding of it. > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Looks legit. Just a minor alteration to include the w/a name inside ilk_dummy_write() as well. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Thu, 18 Oct 2012 11:49:50 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > #define __i915_read(x, y) \ > u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \ > u##x val = 0; \ > + if (IS_GEN5(dev_priv->dev)) \ > + ilk_dummy_write(dev_priv); \ IS_GEN5(dev_priv->dev) just makes me want to puke. At some point we must go through and add an __INTEL_INFO(dev_priv) and so #define __IS_GEN5(dev_priv__) ((dev_priv__)->info->->gen == 5) #define IS_GEN5(dev) __IS_GEN5(to_drm_i915_private(dev)) -Chris
On Thu, Oct 18, 2012 at 1:17 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Thu, 18 Oct 2012 11:49:50 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: >> #define __i915_read(x, y) \ >> u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \ >> u##x val = 0; \ >> + if (IS_GEN5(dev_priv->dev)) \ >> + ilk_dummy_write(dev_priv); \ > > IS_GEN5(dev_priv->dev) just makes me want to puke. At some point we must > go through and add an __INTEL_INFO(dev_priv) and so > #define __IS_GEN5(dev_priv__) ((dev_priv__)->info->->gen == 5) > #define IS_GEN5(dev) __IS_GEN5(to_drm_i915_private(dev)) We could also teach the drm setup and teardown code manners and embed struct drm_device into our own device struct and call it a day. Would kill all that pointless circular pointer chasing. Been on my todo since ages, will probably stay there for a while ... -Daniel
On Thu, Oct 18, 2012 at 10:49 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > +static void > +ilk_dummy_write(struct drm_i915_private *dev_priv) > +{ > + /* Ilk w/a: Issue a dummy write to wake up the chip from rc6 before > + * touching it for real. MI_MODE is masked, hence harmless to write 0 > + * into. */ I'd document the wa implemented here. The summary line is missing an 'e' in Wake, it makes it harder to look for it.
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index a7837e5..c3f4f04 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1131,9 +1131,20 @@ static bool IS_DISPLAYREG(u32 reg) return true; } +static void +ilk_dummy_write(struct drm_i915_private *dev_priv) +{ + /* Ilk w/a: Issue a dummy write to wake up the chip from rc6 before + * touching it for real. MI_MODE is masked, hence harmless to write 0 + * into. */ + I915_WRITE_NOTRACE(MI_MODE, 0); +} + #define __i915_read(x, y) \ u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \ u##x val = 0; \ + if (IS_GEN5(dev_priv->dev)) \ + ilk_dummy_write(dev_priv); \ if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \ unsigned long irqflags; \ spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \ @@ -1165,6 +1176,8 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \ if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \ __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \ } \ + if (IS_GEN5(dev_priv->dev)) \ + ilk_dummy_write(dev_priv); \ if (IS_VALLEYVIEW(dev_priv->dev) && IS_DISPLAYREG(reg)) { \ write##y(val, dev_priv->regs + reg + 0x180000); \ } else { \
Or at least our best understanding of it. Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_drv.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)