From 060810d7abaabcab282e062c595871d661561400 Mon Sep 17 00:00:00 2001 From: Ben Skeggs Date: Mon, 8 Jul 2013 14:15:51 +1000 Subject: drm/nouveau: fix locking issues in page flipping paths b580c9e2b7ba5030a795aa2fb73b796523d65a78 introduced additional problems while trying to solve issues that became apparent while porting to the new reservation stuff. The major problem was that the the previously mentioned patch took the client mutex earlier than previously, but the pinning of new_bo can can potentially cause a buffer move, which would result in attempting to acquire the same mutex again. This commit attempts to fix that "fix". Thanks to Maarten for the tips on keeping lockdep happy and cooking :) Reported-by: Maarten Lankhorst Signed-off-by: Ben Skeggs Acked-by: Maarten Lankhorst diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 4b1afb1..85fed10 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -973,7 +973,7 @@ nouveau_bo_move_m2mf(struct ttm_buffer_object *bo, int evict, bool intr, struct ttm_mem_reg *old_mem = &bo->mem; int ret; - mutex_lock(&chan->cli->mutex); + mutex_lock_nested(&chan->cli->mutex, SINGLE_DEPTH_NESTING); /* create temporary vmas for the transfer and attach them to the * old nouveau_mem node, these will get cleaned up after ttm has diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 708b2d1..61fdef8 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -524,9 +524,12 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, struct nouveau_page_flip_state *s; struct nouveau_channel *chan = NULL; struct nouveau_fence *fence; - struct list_head res; - struct ttm_validate_buffer res_val[2]; + struct ttm_validate_buffer resv[2] = { + { .bo = &old_bo->bo }, + { .bo = &new_bo->bo }, + }; struct ww_acquire_ctx ticket; + LIST_HEAD(res); int ret; if (!drm->channel) @@ -545,27 +548,19 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, chan = drm->channel; spin_unlock(&old_bo->bo.bdev->fence_lock); - mutex_lock(&chan->cli->mutex); - if (new_bo != old_bo) { ret = nouveau_bo_pin(new_bo, TTM_PL_FLAG_VRAM); - if (likely(!ret)) { - res_val[0].bo = &old_bo->bo; - res_val[1].bo = &new_bo->bo; - INIT_LIST_HEAD(&res); - list_add_tail(&res_val[0].head, &res); - list_add_tail(&res_val[1].head, &res); - ret = ttm_eu_reserve_buffers(&ticket, &res); - if (ret) - nouveau_bo_unpin(new_bo); - } - } else - ret = ttm_bo_reserve(&new_bo->bo, false, false, false, 0); + if (ret) + goto fail_free; - if (ret) { - mutex_unlock(&chan->cli->mutex); - goto fail_free; + list_add(&resv[1].head, &res); } + list_add(&resv[0].head, &res); + + mutex_lock(&chan->cli->mutex); + ret = ttm_eu_reserve_buffers(&ticket, &res); + if (ret) + goto fail_unpin; /* Initialize a page flip struct */ *s = (struct nouveau_page_flip_state) @@ -576,10 +571,8 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, /* Emit a page flip */ if (nv_device(drm->device)->card_type >= NV_50) { ret = nv50_display_flip_next(crtc, fb, chan, 0); - if (ret) { - mutex_unlock(&chan->cli->mutex); + if (ret) goto fail_unreserve; - } } ret = nouveau_page_flip_emit(chan, old_bo, new_bo, s, &fence); @@ -590,22 +583,18 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, /* Update the crtc struct and cleanup */ crtc->fb = fb; - if (old_bo != new_bo) { - ttm_eu_fence_buffer_objects(&ticket, &res, fence); + ttm_eu_fence_buffer_objects(&ticket, &res, fence); + if (old_bo != new_bo) nouveau_bo_unpin(old_bo); - } else { - nouveau_bo_fence(new_bo, fence); - ttm_bo_unreserve(&new_bo->bo); - } nouveau_fence_unref(&fence); return 0; fail_unreserve: - if (old_bo != new_bo) { - ttm_eu_backoff_reservation(&ticket, &res); + ttm_eu_backoff_reservation(&ticket, &res); +fail_unpin: + mutex_unlock(&chan->cli->mutex); + if (old_bo != new_bo) nouveau_bo_unpin(new_bo); - } else - ttm_bo_unreserve(&new_bo->bo); fail_free: kfree(s); return ret; diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index 218a4b5..4eca52b 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -284,8 +284,6 @@ static int nouveau_drm_probe(struct pci_dev *pdev, return 0; } -static struct lock_class_key drm_client_lock_class_key; - static int nouveau_drm_load(struct drm_device *dev, unsigned long flags) { @@ -297,7 +295,6 @@ nouveau_drm_load(struct drm_device *dev, unsigned long flags) ret = nouveau_cli_create(pdev, "DRM", sizeof(*drm), (void**)&drm); if (ret) return ret; - lockdep_set_class(&drm->client.mutex, &drm_client_lock_class_key); dev->dev_private = drm; drm->dev = dev; -- cgit v0.10.2 From fdfb8332651db7a280851dfccfc4f0cff4bcd052 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 8 Jul 2013 14:50:54 +1000 Subject: drm/nouveau: fix some error-path leaks in fbcon handling code Signed-off-by: Maarten Lankhorst Signed-off-by: Ben Skeggs diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 61fdef8..907d20e 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -138,7 +138,7 @@ nouveau_user_framebuffer_create(struct drm_device *dev, { struct nouveau_framebuffer *nouveau_fb; struct drm_gem_object *gem; - int ret; + int ret = -ENOMEM; gem = drm_gem_object_lookup(dev, file_priv, mode_cmd->handles[0]); if (!gem) @@ -146,15 +146,19 @@ nouveau_user_framebuffer_create(struct drm_device *dev, nouveau_fb = kzalloc(sizeof(struct nouveau_framebuffer), GFP_KERNEL); if (!nouveau_fb) - return ERR_PTR(-ENOMEM); + goto err_unref; ret = nouveau_framebuffer_init(dev, nouveau_fb, mode_cmd, nouveau_gem_object(gem)); - if (ret) { - drm_gem_object_unreference(gem); - return ERR_PTR(ret); - } + if (ret) + goto err; return &nouveau_fb->base; + +err: + kfree(nouveau_fb); +err_unref: + drm_gem_object_unreference(gem); + return ERR_PTR(ret); } static const struct drm_mode_config_funcs nouveau_mode_config_funcs = { diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index 9352010..4c1bc06 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -385,6 +385,7 @@ out_unlock: mutex_unlock(&dev->struct_mutex); if (chan) nouveau_bo_vma_del(nvbo, &fbcon->nouveau_fb.vma); + nouveau_bo_unmap(nvbo); out_unpin: nouveau_bo_unpin(nvbo); out_unref: -- cgit v0.10.2 From 8dda53fca24789acf891ef31a3c5e03332b43cce Mon Sep 17 00:00:00 2001 From: Ben Skeggs Date: Tue, 9 Jul 2013 12:35:55 +1000 Subject: drm/nv50/kms: fix pin refcnt leaks Weren't critical previously, the buffers would go away anyway. But with recent changes to core drm/ttm lockdep will get pissed off now, so let's fix it. Reported-by: Maarten Lankhorst Signed-off-by: Ben Skeggs Acked-by: Maarten Lankhorst diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c index 54dc635..8b40a36 100644 --- a/drivers/gpu/drm/nouveau/nv50_display.c +++ b/drivers/gpu/drm/nouveau/nv50_display.c @@ -355,6 +355,7 @@ struct nv50_oimm { struct nv50_head { struct nouveau_crtc base; + struct nouveau_bo *image; struct nv50_curs curs; struct nv50_sync sync; struct nv50_ovly ovly; @@ -517,9 +518,10 @@ nv50_display_flip_next(struct drm_crtc *crtc, struct drm_framebuffer *fb, { struct nouveau_framebuffer *nv_fb = nouveau_framebuffer(fb); struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc); + struct nv50_head *head = nv50_head(crtc); struct nv50_sync *sync = nv50_sync(crtc); - int head = nv_crtc->index, ret; u32 *push; + int ret; swap_interval <<= 4; if (swap_interval == 0) @@ -537,7 +539,7 @@ nv50_display_flip_next(struct drm_crtc *crtc, struct drm_framebuffer *fb, return ret; BEGIN_NV04(chan, 0, NV11_SUBCHAN_DMA_SEMAPHORE, 2); - OUT_RING (chan, NvEvoSema0 + head); + OUT_RING (chan, NvEvoSema0 + nv_crtc->index); OUT_RING (chan, sync->addr ^ 0x10); BEGIN_NV04(chan, 0, NV11_SUBCHAN_SEMAPHORE_RELEASE, 1); OUT_RING (chan, sync->data + 1); @@ -546,7 +548,7 @@ nv50_display_flip_next(struct drm_crtc *crtc, struct drm_framebuffer *fb, OUT_RING (chan, sync->data); } else if (chan && nv_mclass(chan->object) < NVC0_CHANNEL_IND_CLASS) { - u64 addr = nv84_fence_crtc(chan, head) + sync->addr; + u64 addr = nv84_fence_crtc(chan, nv_crtc->index) + sync->addr; ret = RING_SPACE(chan, 12); if (ret) return ret; @@ -565,7 +567,7 @@ nv50_display_flip_next(struct drm_crtc *crtc, struct drm_framebuffer *fb, OUT_RING (chan, NV84_SUBCHAN_SEMAPHORE_TRIGGER_ACQUIRE_EQUAL); } else if (chan) { - u64 addr = nv84_fence_crtc(chan, head) + sync->addr; + u64 addr = nv84_fence_crtc(chan, nv_crtc->index) + sync->addr; ret = RING_SPACE(chan, 10); if (ret) return ret; @@ -630,6 +632,8 @@ nv50_display_flip_next(struct drm_crtc *crtc, struct drm_framebuffer *fb, evo_mthd(push, 0x0080, 1); evo_data(push, 0x00000000); evo_kick(push, sync); + + nouveau_bo_ref(nv_fb->nvbo, &head->image); return 0; } @@ -1038,18 +1042,17 @@ static int nv50_crtc_swap_fbs(struct drm_crtc *crtc, struct drm_framebuffer *old_fb) { struct nouveau_framebuffer *nvfb = nouveau_framebuffer(crtc->fb); + struct nv50_head *head = nv50_head(crtc); int ret; ret = nouveau_bo_pin(nvfb->nvbo, TTM_PL_FLAG_VRAM); - if (ret) - return ret; - - if (old_fb) { - nvfb = nouveau_framebuffer(old_fb); - nouveau_bo_unpin(nvfb->nvbo); + if (ret == 0) { + if (head->image) + nouveau_bo_unpin(head->image); + nouveau_bo_ref(nvfb->nvbo, &head->image); } - return 0; + return ret; } static int @@ -1198,6 +1201,15 @@ nv50_crtc_lut_load(struct drm_crtc *crtc) } } +static void +nv50_crtc_disable(struct drm_crtc *crtc) +{ + struct nv50_head *head = nv50_head(crtc); + if (head->image) + nouveau_bo_unpin(head->image); + nouveau_bo_ref(NULL, &head->image); +} + static int nv50_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv, uint32_t handle, uint32_t width, uint32_t height) @@ -1271,18 +1283,29 @@ nv50_crtc_destroy(struct drm_crtc *crtc) struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc); struct nv50_disp *disp = nv50_disp(crtc->dev); struct nv50_head *head = nv50_head(crtc); + nv50_dmac_destroy(disp->core, &head->ovly.base); nv50_pioc_destroy(disp->core, &head->oimm.base); nv50_dmac_destroy(disp->core, &head->sync.base); nv50_pioc_destroy(disp->core, &head->curs.base); + + /*XXX: this shouldn't be necessary, but the core doesn't call + * disconnect() during the cleanup paths + */ + if (head->image) + nouveau_bo_unpin(head->image); + nouveau_bo_ref(NULL, &head->image); + nouveau_bo_unmap(nv_crtc->cursor.nvbo); if (nv_crtc->cursor.nvbo) nouveau_bo_unpin(nv_crtc->cursor.nvbo); nouveau_bo_ref(NULL, &nv_crtc->cursor.nvbo); + nouveau_bo_unmap(nv_crtc->lut.nvbo); if (nv_crtc->lut.nvbo) nouveau_bo_unpin(nv_crtc->lut.nvbo); nouveau_bo_ref(NULL, &nv_crtc->lut.nvbo); + drm_crtc_cleanup(crtc); kfree(crtc); } @@ -1296,6 +1319,7 @@ static const struct drm_crtc_helper_funcs nv50_crtc_hfunc = { .mode_set_base = nv50_crtc_mode_set_base, .mode_set_base_atomic = nv50_crtc_mode_set_base_atomic, .load_lut = nv50_crtc_lut_load, + .disable = nv50_crtc_disable, }; static const struct drm_crtc_funcs nv50_crtc_func = { -- cgit v0.10.2 From 4f3855997c5d2bf6443853800eb1a03f61ad9140 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Sun, 7 Jul 2013 10:37:35 +0200 Subject: drm/nouveau: do not unpin in nouveau_gem_object_del This should no longer be required, and is harmful for framebuffer pinning. Also add a warning if unpin causes the pin count to drop below 0. Signed-off-by: Maarten Lankhorst Signed-off-by: Ben Skeggs diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 85fed10..824a988 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -148,6 +148,7 @@ nouveau_bo_del_ttm(struct ttm_buffer_object *bo) if (unlikely(nvbo->gem)) DRM_ERROR("bo %p still attached to GEM object\n", bo); + WARN_ON(nvbo->pin_refcnt > 0); nv10_bo_put_tile_region(dev, nvbo->tile, NULL); kfree(nvbo); } @@ -340,13 +341,15 @@ nouveau_bo_unpin(struct nouveau_bo *nvbo) { struct nouveau_drm *drm = nouveau_bdev(nvbo->bo.bdev); struct ttm_buffer_object *bo = &nvbo->bo; - int ret; + int ret, ref; ret = ttm_bo_reserve(bo, false, false, false, 0); if (ret) return ret; - if (--nvbo->pin_refcnt) + ref = --nvbo->pin_refcnt; + WARN_ON_ONCE(ref < 0); + if (ref) goto out; nouveau_bo_placement_set(nvbo, bo->mem.placement, 0); diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c index e72d09c..830cb7b 100644 --- a/drivers/gpu/drm/nouveau/nouveau_gem.c +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c @@ -50,12 +50,6 @@ nouveau_gem_object_del(struct drm_gem_object *gem) return; nvbo->gem = NULL; - /* Lockdep hates you for doing reserve with gem object lock held */ - if (WARN_ON_ONCE(nvbo->pin_refcnt)) { - nvbo->pin_refcnt = 1; - nouveau_bo_unpin(nvbo); - } - if (gem->import_attach) drm_prime_gem_destroy(gem, nvbo->bo.sg); -- cgit v0.10.2 From 06b237ef3903a0a5e33785105ea5203153323dd8 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Sun, 7 Jul 2013 10:53:37 +0200 Subject: drm/nouveau: bump fence timeout to 15 seconds calim didn't like 150 seconds timeout, so lower the timeout for him. 15 seconds should still be plenty. Signed-off-by: Maarten Lankhorst Signed-off-by: Ben Skeggs diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c index 1680d91..be31499 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fence.c +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c @@ -143,7 +143,7 @@ nouveau_fence_emit(struct nouveau_fence *fence, struct nouveau_channel *chan) int ret; fence->channel = chan; - fence->timeout = jiffies + (3 * DRM_HZ); + fence->timeout = jiffies + (15 * DRM_HZ); fence->sequence = ++fctx->sequence; ret = fctx->emit(fence); -- cgit v0.10.2 From 00fc6f6f731efb7b76b839598e494b01890d901d Mon Sep 17 00:00:00 2001 From: Ben Skeggs Date: Tue, 9 Jul 2013 14:20:15 +1000 Subject: drm/nouveau: use dedicated channel for async moves on GT/GF chipsets. The moves themselves were generally async to graphics previously, with the exception that if the "main" channel is used to synchronise a page flip at the same time, it can end up blocked for a noticable amount of time for large buffer moves. Not really critical, and there's better ways of handling this, but they are all rather invasive, so this is fine for now. Based on a patch by Maarten Lankhorst addressing the same issue. Reported-by: Maarten Lankhorst Signed-off-by: Ben Skeggs Acked-by: Maarten Lankhorst diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 824a988..459a445 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -581,7 +581,7 @@ nve0_bo_move_init(struct nouveau_channel *chan, u32 handle) int ret = RING_SPACE(chan, 2); if (ret == 0) { BEGIN_NVC0(chan, NvSubCopy, 0x0000, 1); - OUT_RING (chan, handle); + OUT_RING (chan, handle & 0x0000ffff); FIRE_RING (chan); } return ret; @@ -1017,7 +1017,7 @@ nouveau_bo_move_init(struct nouveau_drm *drm) struct ttm_mem_reg *, struct ttm_mem_reg *); int (*init)(struct nouveau_channel *, u32 handle); } _methods[] = { - { "COPY", 0, 0xa0b5, nve0_bo_move_copy, nve0_bo_move_init }, + { "COPY", 4, 0xa0b5, nve0_bo_move_copy, nve0_bo_move_init }, { "GRCE", 0, 0xa0b5, nve0_bo_move_copy, nvc0_bo_move_init }, { "COPY1", 5, 0x90b8, nvc0_bo_move_copy, nvc0_bo_move_init }, { "COPY0", 4, 0x90b5, nvc0_bo_move_copy, nvc0_bo_move_init }, @@ -1037,7 +1037,7 @@ nouveau_bo_move_init(struct nouveau_drm *drm) struct nouveau_channel *chan; u32 handle = (mthd->engine << 16) | mthd->oclass; - if (mthd->init == nve0_bo_move_init) + if (mthd->engine) chan = drm->cechan; else chan = drm->channel; diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index 4eca52b..6197266 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -192,6 +192,18 @@ nouveau_accel_init(struct nouveau_drm *drm) arg0 = NVE0_CHANNEL_IND_ENGINE_GR; arg1 = 1; + } else + if (device->chipset >= 0xa3 && + device->chipset != 0xaa && + device->chipset != 0xac) { + ret = nouveau_channel_new(drm, &drm->client, NVDRM_DEVICE, + NVDRM_CHAN + 1, NvDmaFB, NvDmaTT, + &drm->cechan); + if (ret) + NV_ERROR(drm, "failed to create ce channel, %d\n", ret); + + arg0 = NvDmaFB; + arg1 = NvDmaTT; } else { arg0 = NvDmaFB; arg1 = NvDmaTT; -- cgit v0.10.2 From 9b234db37825860b833ba110673301e2009f0546 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Sun, 7 Jul 2013 10:45:43 +0200 Subject: drm/nouveau: add falcon interrupt handler This prevents 100% cpu usage on fermi cards when the exit interrupt from the secret scrubber is not acked. Signed-off-by: Maarten Lankhorst Signed-off-by: Ben Skeggs diff --git a/drivers/gpu/drm/nouveau/core/engine/bsp/nvc0.c b/drivers/gpu/drm/nouveau/core/engine/bsp/nvc0.c index 262c9f5..ce860de 100644 --- a/drivers/gpu/drm/nouveau/core/engine/bsp/nvc0.c +++ b/drivers/gpu/drm/nouveau/core/engine/bsp/nvc0.c @@ -90,6 +90,7 @@ nvc0_bsp_ctor(struct nouveau_object *parent, struct nouveau_object *engine, return ret; nv_subdev(priv)->unit = 0x00008000; + nv_subdev(priv)->intr = nouveau_falcon_intr; nv_engine(priv)->cclass = &nvc0_bsp_cclass; nv_engine(priv)->sclass = nvc0_bsp_sclass; return 0; diff --git a/drivers/gpu/drm/nouveau/core/engine/bsp/nve0.c b/drivers/gpu/drm/nouveau/core/engine/bsp/nve0.c index c46882c..ba6aeca 100644 --- a/drivers/gpu/drm/nouveau/core/engine/bsp/nve0.c +++ b/drivers/gpu/drm/nouveau/core/engine/bsp/nve0.c @@ -90,6 +90,7 @@ nve0_bsp_ctor(struct nouveau_object *parent, struct nouveau_object *engine, return ret; nv_subdev(priv)->unit = 0x00008000; + nv_subdev(priv)->intr = nouveau_falcon_intr; nv_engine(priv)->cclass = &nve0_bsp_cclass; nv_engine(priv)->sclass = nve0_bsp_sclass; return 0; diff --git a/drivers/gpu/drm/nouveau/core/engine/falcon.c b/drivers/gpu/drm/nouveau/core/engine/falcon.c index 3c7a31f..e03fc8e 100644 --- a/drivers/gpu/drm/nouveau/core/engine/falcon.c +++ b/drivers/gpu/drm/nouveau/core/engine/falcon.c @@ -23,6 +23,25 @@ #include #include +void +nouveau_falcon_intr(struct nouveau_subdev *subdev) +{ + struct nouveau_falcon *falcon = (void *)subdev; + u32 dispatch = nv_ro32(falcon, 0x01c); + u32 intr = nv_ro32(falcon, 0x008) & dispatch & ~(dispatch >> 16); + + if (intr & 0x00000010) { + nv_debug(falcon, "ucode halted\n"); + nv_wo32(falcon, 0x004, 0x00000010); + intr &= ~0x00000010; + } + + if (intr) { + nv_error(falcon, "unhandled intr 0x%08x\n", intr); + nv_wo32(falcon, 0x004, intr); + } +} + u32 _nouveau_falcon_rd32(struct nouveau_object *object, u64 addr) { diff --git a/drivers/gpu/drm/nouveau/core/engine/ppp/nvc0.c b/drivers/gpu/drm/nouveau/core/engine/ppp/nvc0.c index 98072c1..73719aa 100644 --- a/drivers/gpu/drm/nouveau/core/engine/ppp/nvc0.c +++ b/drivers/gpu/drm/nouveau/core/engine/ppp/nvc0.c @@ -90,6 +90,7 @@ nvc0_ppp_ctor(struct nouveau_object *parent, struct nouveau_object *engine, return ret; nv_subdev(priv)->unit = 0x00000002; + nv_subdev(priv)->intr = nouveau_falcon_intr; nv_engine(priv)->cclass = &nvc0_ppp_cclass; nv_engine(priv)->sclass = nvc0_ppp_sclass; return 0; diff --git a/drivers/gpu/drm/nouveau/core/engine/vp/nvc0.c b/drivers/gpu/drm/nouveau/core/engine/vp/nvc0.c index 1879229..ac1f62a 100644 --- a/drivers/gpu/drm/nouveau/core/engine/vp/nvc0.c +++ b/drivers/gpu/drm/nouveau/core/engine/vp/nvc0.c @@ -90,6 +90,7 @@ nvc0_vp_ctor(struct nouveau_object *parent, struct nouveau_object *engine, return ret; nv_subdev(priv)->unit = 0x00020000; + nv_subdev(priv)->intr = nouveau_falcon_intr; nv_engine(priv)->cclass = &nvc0_vp_cclass; nv_engine(priv)->sclass = nvc0_vp_sclass; return 0; diff --git a/drivers/gpu/drm/nouveau/core/engine/vp/nve0.c b/drivers/gpu/drm/nouveau/core/engine/vp/nve0.c index d28ecbf..d4c3108 100644 --- a/drivers/gpu/drm/nouveau/core/engine/vp/nve0.c +++ b/drivers/gpu/drm/nouveau/core/engine/vp/nve0.c @@ -90,6 +90,7 @@ nve0_vp_ctor(struct nouveau_object *parent, struct nouveau_object *engine, return ret; nv_subdev(priv)->unit = 0x00020000; + nv_subdev(priv)->intr = nouveau_falcon_intr; nv_engine(priv)->cclass = &nve0_vp_cclass; nv_engine(priv)->sclass = nve0_vp_sclass; return 0; diff --git a/drivers/gpu/drm/nouveau/core/include/engine/falcon.h b/drivers/gpu/drm/nouveau/core/include/engine/falcon.h index 1edec38..181aa7d 100644 --- a/drivers/gpu/drm/nouveau/core/include/engine/falcon.h +++ b/drivers/gpu/drm/nouveau/core/include/engine/falcon.h @@ -72,6 +72,8 @@ int nouveau_falcon_create_(struct nouveau_object *, struct nouveau_object *, struct nouveau_oclass *, u32, bool, const char *, const char *, int, void **); +void nouveau_falcon_intr(struct nouveau_subdev *subdev); + #define _nouveau_falcon_dtor _nouveau_engine_dtor int _nouveau_falcon_init(struct nouveau_object *); int _nouveau_falcon_fini(struct nouveau_object *, bool); -- cgit v0.10.2 From 0108bc808107b97e101b15af9705729626be6447 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Sun, 7 Jul 2013 10:40:19 +0200 Subject: drm/nouveau: do not allow negative sizes for now The API allows up to 64-bits allocations, but size is handled as int inside nouveau almost everywhere. Until this is fixed it's better to prevent negative sizes. The 256 kB before INT_MAX is paranoia, because of the large page aligning below that could flip it above INT_MAX. Signed-off-by: Maarten Lankhorst Signed-off-by: Ben Skeggs diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 459a445..4e7ee5f 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -198,6 +198,12 @@ nouveau_bo_new(struct drm_device *dev, int size, int align, size_t acc_size; int ret; int type = ttm_bo_type_device; + int max_size = INT_MAX & ~((1 << drm->client.base.vm->vmm->lpg_shift) - 1); + + if (size <= 0 || size > max_size) { + nv_warn(drm, "skipped size %x\n", (u32)size); + return -EINVAL; + } if (sg) type = ttm_bo_type_sg; -- cgit v0.10.2 From 0692282181e248328c226c2520994fc06dfd65bf Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 11 Jul 2013 13:35:40 +0200 Subject: drm/i915: fix up readout of the lvds dither bit on gen2/3 It's in the PFIT_CONTROL register, but very much associated with the lvds encoder. So move the readout for it (in the case of an otherwise disabled pfit) from the pipe to the lvds encoder's get_config function. Otherwise we get a pipe state mismatch if we use pipe B for a non-lvds output and we've left the dither bit enabled behind us. This can happen if the BIOS has set the bit (some seem to unconditionally do that, even in the complete absence of an lvds port), but not enabled pipe B at boot-up. Then we won't clear the pfit control register since we can only touch that if the pfit is associated with our pipe in the crtc configuration - we could trample over the pfit state of the other pipe otherwise since it's shared. Once pipe B is enabled we notice that the 6to8 dither bit is set and complain about the mismatch. Note that testing indicates that we don't actually need to set this bit when the pfit is disabled, dithering on 18bpp panels seems to work regardless. But ripping that code out is not something for a bugfix meant for -rc kernels. v2: While at it clarify the logic in i9xx_get_pfit_config, spurred by comments from Chris on irc. v3: Use Chris suggestion to make the control flow in i9xx_get_pfit_config easier to understand. v4: Kill the extra line, spotted by Chris. Reported-by: Knut Petersen Cc: Knut Petersen Cc: Chris Wilson References: http://lists.freedesktop.org/archives/intel-gfx/2013-July/030092.html Tested-by: Knut Petersen Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 85f3eb7..c59335c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4913,22 +4913,19 @@ static void i9xx_get_pfit_config(struct intel_crtc *crtc, uint32_t tmp; tmp = I915_READ(PFIT_CONTROL); + if (!(tmp & PFIT_ENABLE)) + return; + /* Check whether the pfit is attached to our pipe. */ if (INTEL_INFO(dev)->gen < 4) { if (crtc->pipe != PIPE_B) return; - - /* gen2/3 store dither state in pfit control, needs to match */ - pipe_config->gmch_pfit.control = tmp & PANEL_8TO6_DITHER_ENABLE; } else { if ((tmp & PFIT_PIPE_MASK) != (crtc->pipe << PFIT_PIPE_SHIFT)) return; } - if (!(tmp & PFIT_ENABLE)) - return; - - pipe_config->gmch_pfit.control = I915_READ(PFIT_CONTROL); + pipe_config->gmch_pfit.control = tmp; pipe_config->gmch_pfit.pgm_ratios = I915_READ(PFIT_PGM_RATIOS); if (INTEL_INFO(dev)->gen < 5) pipe_config->gmch_pfit.lvds_border_bits = diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 2abb2d3..1207998 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -109,6 +109,13 @@ static void intel_lvds_get_config(struct intel_encoder *encoder, flags |= DRM_MODE_FLAG_PVSYNC; pipe_config->adjusted_mode.flags |= flags; + + /* gen2/3 store dither state in pfit control, needs to match */ + if (INTEL_INFO(dev)->gen < 4) { + tmp = I915_READ(PFIT_CONTROL); + + pipe_config->gmch_pfit.control |= tmp & PANEL_8TO6_DITHER_ENABLE; + } } /* The LVDS pin pair needs to be on before the DPLLs are enabled. -- cgit v0.10.2 From 21d8a4756af5fdf4a42e79a77cf3b6f52678d443 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 12 Jul 2013 08:07:30 +0200 Subject: drm/i915: fix pfit regression for non-autoscaled resolutions I.e. for letter/pillarboxing. For those cases we need to adjust the mode a bit, but Jesse gmch pfit refactoring in commit 2dd24552cab40ea829ba3fda890eeafd2c4816d8 Author: Jesse Barnes Date: Thu Apr 25 12:55:01 2013 -0700 drm/i915: factor out GMCH panel fitting code and use for eDP v3 broke that by reordering the computation of the gmch pfit state with the block of code that prepared the adjusted mode for it and told the modeset core not to overwrite the adjusted mode with default settings. We might want to switch around the core code to just fill in defaults, but this code predates the pipe_config modeset rework. And in the old crtc helpers we did not have a suitable spot to do this. Cc: Mika Kuoppala Cc: Jesse Barnes Cc: Hans de Bruin Reported-and-tested-by: Hans de Bruin Reviewed-by: Jesse Barnes Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 1207998..9a797c9 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -297,14 +297,11 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder, intel_pch_panel_fitting(intel_crtc, pipe_config, intel_connector->panel.fitting_mode); - return true; } else { intel_gmch_panel_fitting(intel_crtc, pipe_config, intel_connector->panel.fitting_mode); - } - drm_mode_set_crtcinfo(adjusted_mode, 0); - pipe_config->timings_set = true; + } /* * XXX: It would be nice to support lower refresh rates on the diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 80bea1d..45010bb 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -194,6 +194,9 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc, adjusted_mode->vdisplay == mode->vdisplay) goto out; + drm_mode_set_crtcinfo(adjusted_mode, 0); + pipe_config->timings_set = true; + switch (fitting_mode) { case DRM_MODE_SCALE_CENTER: /* -- cgit v0.10.2 From bcf53de4e60d9000b82f541d654529e2902a4c2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Marchesin?= Date: Fri, 12 Jul 2013 13:54:41 -0700 Subject: drm/i915: Preserve the DDI_A_4_LANES bit from the bios MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise the DDI_A_4_LANES bit gets lost and we can't use > 2 lanes on eDP. This fixes eDP on hsw with > 2 lanes. Also s/port_reversal/saved_port_bits/ since the current name is confusing. Signed-off-by: Stéphane Marchesin Reviewed-by: Paulo Zanoni Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 324211a..b042ee5 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -301,7 +301,7 @@ static void intel_ddi_mode_set(struct drm_encoder *encoder, struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder); - intel_dp->DP = intel_dig_port->port_reversal | + intel_dp->DP = intel_dig_port->saved_port_bits | DDI_BUF_CTL_ENABLE | DDI_BUF_EMP_400MV_0DB_HSW; intel_dp->DP |= DDI_PORT_WIDTH(intel_dp->lane_count); @@ -1109,7 +1109,8 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder) * enabling the port. */ I915_WRITE(DDI_BUF_CTL(port), - intel_dig_port->port_reversal | DDI_BUF_CTL_ENABLE); + intel_dig_port->saved_port_bits | + DDI_BUF_CTL_ENABLE); } else if (type == INTEL_OUTPUT_EDP) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder); @@ -1347,8 +1348,9 @@ void intel_ddi_init(struct drm_device *dev, enum port port) intel_encoder->get_config = intel_ddi_get_config; intel_dig_port->port = port; - intel_dig_port->port_reversal = I915_READ(DDI_BUF_CTL(port)) & - DDI_BUF_PORT_REVERSAL; + intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) & + (DDI_BUF_PORT_REVERSAL | + DDI_A_4_LANES); intel_dig_port->dp.output_reg = DDI_BUF_CTL(port); intel_encoder->type = INTEL_OUTPUT_UNKNOWN; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index c8c9b6f..b7d6e09 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -504,7 +504,7 @@ struct intel_dp { struct intel_digital_port { struct intel_encoder base; enum port port; - u32 port_reversal; + u32 saved_port_bits; struct intel_dp dp; struct intel_hdmi hdmi; }; -- cgit v0.10.2 From 7dcd2677ea912573d9ed4bcd629b0023b2d11505 Mon Sep 17 00:00:00 2001 From: Konstantin Khlebnikov Date: Wed, 17 Jul 2013 10:22:58 +0400 Subject: drm/i915: fix long-standing SNB regression in power consumption after resume v2 This patch fixes regression in power consumtion of sandy bridge gpu, which exists since v3.6 Sometimes after resuming from s2ram gpu starts thinking that it's extremely busy. After that it never reaches rc6 state. Bug exists since kernel v3.6: commit b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 Author: Jesse Barnes Date: Thu Jun 14 11:04:48 2012 -0700 drm/i915: load boot context at driver init time For some reason RC6 is already enabled at the beginning of resuming process. Following initliaztion breaks some internal state and confuses RPS engine. This patch disables RC6 at the beginnig of resume and initialization. I've rearranged initialization sequence, because intel_disable_gt_powersave() needs initialized force_wake_get/put and some locks from the dev_priv. Note: The culprit in the initialization sequence seems to be the write to MBCTL added in the above mentioned commit. The first version of this patch just held a forcewake reference across the clock gating init functions, which seems to have been enought to gather quite a few positive test reports. But since that smelled a bit like ad-hoc duct-tape v2 now just disables rps/rc6 across the entire hw setup. References: https://bugs.freedesktop.org/show_bug.cgi?id=54089 References: https://bugzilla.kernel.org/show_bug.cgi?id=58971 References: https://patchwork.kernel.org/patch/2827634/ (patch v1) Signed-off-by: Konstantin Khlebnikov Cc: Daniel Vetter Cc: Chris Wilson Cc: Jesse Barnes [danvet: Add note about v1 vs. v2 of this patch and use standard layout for the commit citation. Also add the tested-bys from v1 and a cc: stable.] Cc: stable@vger.kernel.org (Note: tiny conflict due to the addition of the backlight lock in 3.11) Tested-by: Alexander Kaltsas (v1) Tested-by: rocko (v1) Tested-by: JohnMB (v1) Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index adb319b..5c0663f 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1495,6 +1495,14 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) dev_priv->dev = dev; dev_priv->info = info; + spin_lock_init(&dev_priv->irq_lock); + spin_lock_init(&dev_priv->gpu_error.lock); + spin_lock_init(&dev_priv->rps.lock); + spin_lock_init(&dev_priv->backlight.lock); + mutex_init(&dev_priv->dpio_lock); + mutex_init(&dev_priv->rps.hw_lock); + mutex_init(&dev_priv->modeset_restore_lock); + i915_dump_device_info(dev_priv); if (i915_get_bridge_dev(dev)) { @@ -1586,6 +1594,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) intel_irq_init(dev); intel_gt_init(dev); + intel_gt_reset(dev); /* Try to make sure MCHBAR is enabled before poking at it */ intel_setup_mchbar(dev); @@ -1610,15 +1619,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) if (!IS_I945G(dev) && !IS_I945GM(dev)) pci_enable_msi(dev->pdev); - spin_lock_init(&dev_priv->irq_lock); - spin_lock_init(&dev_priv->gpu_error.lock); - spin_lock_init(&dev_priv->rps.lock); - spin_lock_init(&dev_priv->backlight.lock); - mutex_init(&dev_priv->dpio_lock); - - mutex_init(&dev_priv->rps.hw_lock); - mutex_init(&dev_priv->modeset_restore_lock); - dev_priv->num_plane = 1; if (IS_VALLEYVIEW(dev)) dev_priv->num_plane = 2; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index d10e673..828c426 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5487,6 +5487,9 @@ void intel_gt_reset(struct drm_device *dev) if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) __gen6_gt_force_wake_mt_reset(dev_priv); } + + /* BIOS often leaves RC6 enabled, but disable it for hw init */ + intel_disable_gt_powersave(dev); } void intel_gt_init(struct drm_device *dev) @@ -5495,8 +5498,6 @@ void intel_gt_init(struct drm_device *dev) spin_lock_init(&dev_priv->gt_lock); - intel_gt_reset(dev); - if (IS_VALLEYVIEW(dev)) { dev_priv->gt.force_wake_get = vlv_force_wake_get; dev_priv->gt.force_wake_put = vlv_force_wake_put; -- cgit v0.10.2 From 35c95375f69ceec721fea67a0532bc17ceb5cf64 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 17 Jul 2013 06:55:04 +0200 Subject: drm/i915: Sanitize shared dpll state There seems to be no limit to the amount of gunk the firmware can leave behind. Some platforms leave pch dplls on which are not in active use at all. The example in the bug report is a Apple Macbook Pro. Note that this escape scrunity of the hw state checker until we've tried to use this enabled, but unused pll since we did only check for the inverse case of a in-used, but disabled pll. v2: Add a WARN in the pll state checker which would have caught this case. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66952 Reported-and-tested-by: shui yangwei Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c59335c..76b01c1 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8314,6 +8314,8 @@ check_shared_dpll_state(struct drm_device *dev) pll->active, pll->refcount); WARN(pll->active && !pll->on, "pll in active use but not on in sw tracking\n"); + WARN(pll->on && !pll->active, + "pll in on but not on in use in sw tracking\n"); WARN(pll->on != active, "pll on state mismatch (expected %i, found %i)\n", pll->on, active); @@ -9814,8 +9816,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) } pll->refcount = pll->active; - DRM_DEBUG_KMS("%s hw state readout: refcount %i\n", - pll->name, pll->refcount); + DRM_DEBUG_KMS("%s hw state readout: refcount %i, on %i\n", + pll->name, pll->refcount, pll->on); } list_for_each_entry(encoder, &dev->mode_config.encoder_list, @@ -9866,6 +9868,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev, struct drm_plane *plane; struct intel_crtc *crtc; struct intel_encoder *encoder; + int i; intel_modeset_readout_hw_state(dev); @@ -9881,6 +9884,18 @@ void intel_modeset_setup_hw_state(struct drm_device *dev, intel_dump_pipe_config(crtc, &crtc->config, "[setup_hw_state]"); } + for (i = 0; i < dev_priv->num_shared_dpll; i++) { + struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i]; + + if (!pll->on || pll->active) + continue; + + DRM_DEBUG_KMS("%s enabled but not in use, disabling\n", pll->name); + + pll->disable(dev_priv, pll); + pll->on = false; + } + if (force_restore) { /* * We need to use raw interfaces for restoring state to avoid -- cgit v0.10.2 From 2e57f47d317dd035b18634b0c602272529368fcc Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 17 Jul 2013 12:14:40 +0100 Subject: drm/i915: Fix dereferencing invalid connectors in is_crtc_connector_off() In commit e3de42b68478a8c95dd27520e9adead2af9477a5 Author: Imre Deak Date: Fri May 3 19:44:07 2013 +0200 drm/i915: force full modeset if the connector is in DPMS OFF mode a new function was added that walked over the set of connectors to see if any of the currently associated CRTC was switched off. This function walked an array of connectors, rather than the array of pointers to connectors contained in the drm_mode_set - i.e. it was dereferencing far past the end of the first connector. This only becomes an issue if we attempt to use a clone mode (i.e. more than one connector per CRTC) such that set->num_connectors > 1. Reported-by: Timo Aaltonen Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65927 Signed-off-by: Chris Wilson Cc: Imre Deak Cc: Egbert Eich Cc: Jesse Barnes Cc: Daniel Vetter Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 76b01c1..76b034b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8540,15 +8540,20 @@ static void intel_set_config_restore_state(struct drm_device *dev, } static bool -is_crtc_connector_off(struct drm_crtc *crtc, struct drm_connector *connectors, - int num_connectors) +is_crtc_connector_off(struct drm_mode_set *set) { int i; - for (i = 0; i < num_connectors; i++) - if (connectors[i].encoder && - connectors[i].encoder->crtc == crtc && - connectors[i].dpms != DRM_MODE_DPMS_ON) + if (set->num_connectors == 0) + return false; + + if (WARN_ON(set->connectors == NULL)) + return false; + + for (i = 0; i < set->num_connectors; i++) + if (set->connectors[i]->encoder && + set->connectors[i]->encoder->crtc == set->crtc && + set->connectors[i]->dpms != DRM_MODE_DPMS_ON) return true; return false; @@ -8561,10 +8566,8 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set, /* We should be able to check here if the fb has the same properties * and then just flip_or_move it */ - if (set->connectors != NULL && - is_crtc_connector_off(set->crtc, *set->connectors, - set->num_connectors)) { - config->mode_changed = true; + if (is_crtc_connector_off(set)) { + config->mode_changed = true; } else if (set->crtc->fb != set->fb) { /* If we have no fb then treat it as a full mode set */ if (set->crtc->fb == NULL) { -- cgit v0.10.2 From 94a335dba34ff47cad3d6d0c29b452d43a1be3c8 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 17 Jul 2013 14:51:28 +0200 Subject: drm/i915: correctly restore fences with objects attached MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To avoid stalls we delay tiling changes and especially hold of committing the new fence state for as long as possible. Synchronization points are in the execbuf code and in our gtt fault handler. Unfortunately we've missed that tricky detail when adding proper fence restore code in commit 19b2dbde5732170a03bd82cc8bd442cf88d856f7 Author: Chris Wilson Date: Wed Jun 12 10:15:12 2013 +0100 drm/i915: Restore fences after resume and GPU resets The result was that we've restored fences for objects with no tiling, since the object<->fence link still existed after resume. Now that wouldn't have been too bad since any subsequent access would have fixed things up, but if we've changed from tiled to untiled real havoc happened: The tiling stride is stored -1 in the fence register, so a stride of 0 resulted in all 1s in the top 32bits, and so a completely bogus fence spanning everything from the start of the object to the top of the GTT. The tell-tale in the register dumps looks like: FENCE START 2: 0x0214d001 FENCE END 2: 0xfffff3ff Bit 11 isn't set since the hw doesn't store it, even when writing all 1s (at least on my snb here). To prevent such a gaffle in the future add a sanity check for fences with an untiled object attached in i915_gem_write_fence. v2: Fix the WARN, spotted by Chris. v3: Trying to reuse get_fences looked ugly and obfuscated the code. Instead reuse update_fence and to make it really dtrt also move the fence dirty state clearing into update_fence. Cc: Chris Wilson Cc: Stéphane Marchesin Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=60530 Cc: stable@vger.kernel.org (for 3.10 only) Reviewed-by: Chris Wilson Tested-by: Matthew Garrett Tested-by: Björn Bidar Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 97afd26..d9e2208 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2258,7 +2258,17 @@ void i915_gem_restore_fences(struct drm_device *dev) for (i = 0; i < dev_priv->num_fence_regs; i++) { struct drm_i915_fence_reg *reg = &dev_priv->fence_regs[i]; - i915_gem_write_fence(dev, i, reg->obj); + + /* + * Commit delayed tiling changes if we have an object still + * attached to the fence, otherwise just clear the fence. + */ + if (reg->obj) { + i915_gem_object_update_fence(reg->obj, reg, + reg->obj->tiling_mode); + } else { + i915_gem_write_fence(dev, i, NULL); + } } } @@ -2795,6 +2805,10 @@ static void i915_gem_write_fence(struct drm_device *dev, int reg, if (i915_gem_object_needs_mb(dev_priv->fence_regs[reg].obj)) mb(); + WARN(obj && (!obj->stride || !obj->tiling_mode), + "bogus fence setup with stride: 0x%x, tiling mode: %i\n", + obj->stride, obj->tiling_mode); + switch (INTEL_INFO(dev)->gen) { case 7: case 6: @@ -2836,6 +2850,7 @@ static void i915_gem_object_update_fence(struct drm_i915_gem_object *obj, fence->obj = NULL; list_del_init(&fence->lru_list); } + obj->fence_dirty = false; } static int @@ -2965,7 +2980,6 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj) return 0; i915_gem_object_update_fence(obj, reg, enable); - obj->fence_dirty = false; return 0; } -- cgit v0.10.2 From e85843bec6c2ea7c10ec61238396891cc2b753a9 Mon Sep 17 00:00:00 2001 From: Kamal Mostafa Date: Fri, 19 Jul 2013 15:02:01 -0700 Subject: drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=47941 BugLink: https://bugs.launchpad.net/bugs/1163720 BugLink: https://bugs.launchpad.net/bugs/1162026 Some machines suffer from non-functional backlight controls if BLM_PCH_PWM_ENABLE is set, so provide a quirk to avoid doing so. Apply this quirk to Dell XPS 13 models. Tested-by: Eric Griffith Tested-by: Kent Baxley Cc: # v3.8+ Signed-off-by: Kamal Mostafa Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a416645..204c3ec 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -555,6 +555,7 @@ enum intel_sbi_destination { #define QUIRK_PIPEA_FORCE (1<<0) #define QUIRK_LVDS_SSC_DISABLE (1<<1) #define QUIRK_INVERT_BRIGHTNESS (1<<2) +#define QUIRK_NO_PCH_PWM_ENABLE (1<<3) struct intel_fbdev; struct intel_fbc_work; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 76b034b..5fb3058 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9400,6 +9400,17 @@ static void quirk_invert_brightness(struct drm_device *dev) DRM_INFO("applying inverted panel brightness quirk\n"); } +/* + * Some machines (Dell XPS13) suffer broken backlight controls if + * BLM_PCH_PWM_ENABLE is set. + */ +static void quirk_no_pcm_pwm_enable(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + dev_priv->quirks |= QUIRK_NO_PCH_PWM_ENABLE; + DRM_INFO("applying no-PCH_PWM_ENABLE quirk\n"); +} + struct intel_quirk { int device; int subsystem_vendor; @@ -9469,6 +9480,11 @@ static struct intel_quirk intel_quirks[] = { /* Acer Aspire 4736Z */ { 0x2a42, 0x1025, 0x0260, quirk_invert_brightness }, + + /* Dell XPS13 HD Sandy Bridge */ + { 0x0116, 0x1028, 0x052e, quirk_no_pcm_pwm_enable }, + /* Dell XPS13 HD and XPS13 FHD Ivy Bridge */ + { 0x0166, 0x1028, 0x058b, quirk_no_pcm_pwm_enable }, }; static void intel_init_quirks(struct drm_device *dev) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 45010bb..67e2c1f 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -583,7 +583,8 @@ void intel_panel_enable_backlight(struct drm_device *dev, POSTING_READ(reg); I915_WRITE(reg, tmp | BLM_PWM_ENABLE); - if (HAS_PCH_SPLIT(dev)) { + if (HAS_PCH_SPLIT(dev) && + !(dev_priv->quirks & QUIRK_NO_PCH_PWM_ENABLE)) { tmp = I915_READ(BLC_PWM_PCH_CTL1); tmp |= BLM_PCH_PWM_ENABLE; tmp &= ~BLM_PCH_OVERRIDE_ENABLE; -- cgit v0.10.2 From a7cd1b8fea2f341b626b255d9898a5ca5fabbf0a Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 19 Jul 2013 20:36:51 +0100 Subject: drm/i915: Serialize almost all register access In theory, the different register blocks were meant to be only ever touched when holding either the struct_mutex, mode_config.lock or even a specific localised lock. This does not seem to be the case, and the hardware reacts extremely badly if we attempt to concurrently access two registers within the same cacheline. The HSD suggests that we only need to do this workaround for display range registers. However, upon review we need to serialize the multiple stages in our register write functions - if only for preemption protection. Irrespective of the hardware requirements, the current io functions are a little too loose with respect to the combination of pre- and post-condition testing that we do in conjunction with the actual io. As a result, we may be pre-empted and generate both false-postive and false-negative errors. Note well that this is a "90%" solution, there remains a few direct users of ioread/iowrite which will be fixed up in the next few patches. Since they are more invasive and that this simple change will prevent almost all lockups on Haswell, we kept this patch simple to facilitate backporting to stable. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=63914 Signed-off-by: Chris Wilson Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index f4af1ca..6ddc567 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1253,21 +1253,21 @@ hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg) #define __i915_read(x, y) \ u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \ + unsigned long irqflags; \ u##x val = 0; \ + spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \ 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); \ if (dev_priv->forcewake_count == 0) \ dev_priv->gt.force_wake_get(dev_priv); \ val = read##y(dev_priv->regs + reg); \ if (dev_priv->forcewake_count == 0) \ dev_priv->gt.force_wake_put(dev_priv); \ - spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags); \ } else { \ val = read##y(dev_priv->regs + reg); \ } \ + spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags); \ trace_i915_reg_rw(false, reg, val, sizeof(val)); \ return val; \ } @@ -1280,8 +1280,10 @@ __i915_read(64, q) #define __i915_write(x, y) \ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \ + unsigned long irqflags; \ u32 __fifo_ret = 0; \ trace_i915_reg_rw(true, reg, val, sizeof(val)); \ + spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \ if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \ __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \ } \ @@ -1293,6 +1295,7 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \ gen6_gt_check_fifodbg(dev_priv); \ } \ hsw_unclaimed_reg_check(dev_priv, reg); \ + spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags); \ } __i915_write(8, b) __i915_write(16, w) -- cgit v0.10.2 From 181d1b9e31c668259d3798c521672afb8edd355c Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sun, 21 Jul 2013 13:16:24 +0200 Subject: drm/i915: fix up gt init sequence fallout The regression fix for gen6+ rps fallout commit 7dcd2677ea912573d9ed4bcd629b0023b2d11505 Author: Konstantin Khlebnikov Date: Wed Jul 17 10:22:58 2013 +0400 drm/i915: fix long-standing SNB regression in power consumption after resume unintentionally also changed the init sequence ordering between gt_init and gt_reset - we need to reset BIOS damage like leftover forcewake references before we run our own code. Otherwise we can get nasty dmesg noise like [drm:__gen6_gt_force_wake_mt_get] *ERROR* Timed out waiting for forcewake old ack to clear. again. Since _reset suggests that we first need to have stuff initialized (which isn't the case here) call it sanitze instead. While at it also block out the rps disable introduced by the above commit on ilk: We don't have any knowledge of ilk rps being broken in similar ways. And the disable functions uses the default hw state which is only read out when we're enabling rps. So essentially we've been writing random grabage into that register. Reported-by: Chris Wilson Cc: Chris Wilson Cc: Konstantin Khlebnikov Cc: Jesse Barnes Cc: stable@vger.kernel.org Tested-by: Chris Wilson Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 5c0663f..abf158d 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1593,8 +1593,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) intel_detect_pch(dev); intel_irq_init(dev); + intel_gt_sanitize(dev); intel_gt_init(dev); - intel_gt_reset(dev); /* Try to make sure MCHBAR is enabled before poking at it */ intel_setup_mchbar(dev); diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 6ddc567..45b3c03 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -706,7 +706,7 @@ static int i915_drm_thaw(struct drm_device *dev) { int error = 0; - intel_gt_reset(dev); + intel_gt_sanitize(dev); if (drm_core_check_feature(dev, DRIVER_MODESET)) { mutex_lock(&dev->struct_mutex); @@ -732,7 +732,7 @@ int i915_resume(struct drm_device *dev) pci_set_master(dev->pdev); - intel_gt_reset(dev); + intel_gt_sanitize(dev); /* * Platforms with opregion should have sane BIOS, older ones (gen3 and diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 204c3ec..d2ee334 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1584,7 +1584,7 @@ void i915_handle_error(struct drm_device *dev, bool wedged); extern void intel_irq_init(struct drm_device *dev); extern void intel_hpd_init(struct drm_device *dev); extern void intel_gt_init(struct drm_device *dev); -extern void intel_gt_reset(struct drm_device *dev); +extern void intel_gt_sanitize(struct drm_device *dev); void i915_error_state_free(struct kref *error_ref); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 828c426..6a347f5 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5476,7 +5476,7 @@ static void vlv_force_wake_put(struct drm_i915_private *dev_priv) gen6_gt_check_fifodbg(dev_priv); } -void intel_gt_reset(struct drm_device *dev) +void intel_gt_sanitize(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -5489,7 +5489,8 @@ void intel_gt_reset(struct drm_device *dev) } /* BIOS often leaves RC6 enabled, but disable it for hw init */ - intel_disable_gt_powersave(dev); + if (INTEL_INFO(dev)->gen >= 6) + intel_disable_gt_powersave(dev); } void intel_gt_init(struct drm_device *dev) -- cgit v0.10.2 From 25f397a429dfa43f22c278d0119a60a343aa568f Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 19 Jul 2013 18:57:11 +0200 Subject: drm/crtc-helper: explicit DPMS on after modeset Atm the crtc helper implementation of set_config has really inconsisten semantics: If just an fb update is good enough, dpms state will be left as-is, but if we do a full modeset we force everything to dpms on. This change has already been applied to the i915 modeset code in commit e3de42b68478a8c95dd27520e9adead2af9477a5 Author: Imre Deak Date: Fri May 3 19:44:07 2013 +0200 drm/i915: force full modeset if the connector is in DPMS OFF mode which according to Greg KH seems to aim for a new record in most Bugzilla: links in a commit message. The history of this dpms forcing is pretty interesting. This patch here is an almost-revert of commit 811aaa55ba21ab37407018cfc01770d6b037d3fb Author: Keith Packard Date: Thu Feb 3 16:57:28 2011 -0800 drm: Only set DPMS ON when actually configuring a mode which fixed the bug of trying to dpms on disabled outputs, but introduced the new discrepancy between an fb update only and full modesets. The actual introduction of this goes back to commit bf9dc102e284a5aa78c73fc9d72e11d5ccd8669f Author: Keith Packard Date: Fri Nov 26 10:45:58 2010 -0800 drm: Set connector DPMS status to ON in drm_crtc_helper_set_config And if you'd dig around in the i915 driver code there's even more fun around forcing dpms on and losing our heads and temper of the resulting inconsistencies. Especially the DP re-training code had tons of funny stuff in it. v2: So v1 totally blew up on resume on my radeon system here. After much head-scraching I've figured out that the radeon resume functions resumes the console system _before_ it actually restores all the modeset state. And resuming the console systems means that fbdev doeas an immediate ->set_par call. Now up to this patch that ->set_par did absolutely nothing: All the old sw state from pre-suspend was still around (since the modeset reset wasn't done yet), which means that the set_config calls done as a result of the ->set_par where all treated as no-ops (despite that the real hw state was obviously something completely different). Since v1 of this patch just added a bunch of ->dpms calls if the crtc was enabled, those set_config calls suddenly stopped being no-ops. But because the hw state wasn't restored the ->dpms callbacks resulted in decent amounts of hilarity and eventual full hangs. Since I can't review all kms drivers for such tricky ordering constraints v2 opts for a different approach and forces a full modeset if the connector dpms state isnt' DPMS_ON. Since the ->dpms callbacks implemented by the modeset helpers update the connector->dpms property we have the same effect of ensuring that the pipe is ultimately turned on, even if we just end up updating the fb. This is the same approac we ended up using in the intel driver. Note that besides i915.ko only all other drivers eventually call drm_helper_connector_dpms with the exception of vmwgfx, which does not support dmps at all. v3: Dave Airlie merged the broken first version of this patch, so squash in the revert of commit 372835a8527f85b3eff20a18c2c339e827dfd4e4 Author: Daniel Vetter Date: Sat Jun 15 00:13:13 2013 +0200 drm/crtc-helper: explicit DPMS on after modeset Also fix up the spelling fail a bit in the commit message while at it. Cc: Dave Airlie Reviewed-by: Alex Deucher Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67043 Signed-off-by: Daniel Vetter Signed-off-by: Dave Airlie diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 738a429..6a64749 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -677,6 +677,11 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) /* don't break so fail path works correct */ fail = 1; break; + + if (connector->dpms != DRM_MODE_DPMS_ON) { + DRM_DEBUG_KMS("connector dpms not on, full mode switch\n"); + mode_changed = true; + } } } @@ -754,6 +759,12 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) ret = -EINVAL; goto fail; } + DRM_DEBUG_KMS("Setting connector DPMS state to on\n"); + for (i = 0; i < set->num_connectors; i++) { + DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS on\n", set->connectors[i]->base.id, + drm_get_connector_name(set->connectors[i])); + set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON); + } } drm_helper_disable_unused_functions(dev); } else if (fb_changed) { @@ -771,22 +782,6 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) } } - /* - * crtc set_config helpers implicit set the crtc and all connected - * encoders to DPMS on for a full mode set. But for just an fb update it - * doesn't do that. To not confuse userspace, do an explicit DPMS_ON - * unconditionally. This will also ensure driver internal dpms state is - * consistent again. - */ - if (set->crtc->enabled) { - DRM_DEBUG_KMS("Setting connector DPMS state to on\n"); - for (i = 0; i < set->num_connectors; i++) { - DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS on\n", set->connectors[i]->base.id, - drm_get_connector_name(set->connectors[i])); - set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON); - } - } - kfree(save_connectors); kfree(save_encoders); kfree(save_crtcs); -- cgit v0.10.2