From 9535c4757b881e06fae72a857485ad57c422b8d2 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Tue, 21 Apr 2015 09:49:11 -0700 Subject: [PATCH 1/3] drm/i915: cope with large i2c transfers The hardware, according to the specs, is limited to 256 byte transfers, and current driver has no protections in case users attempt to do larger transfers. The code will just stomp over status register and mayhem ensues. Let's split larger transfers into digestable chunks. Doing this allows Atmel MXT driver on Pixel 1 function properly (it hasn't since commit 9d8dc3e529a19e427fd379118acd132520935c5d "Input: atmel_mxt_ts - implement T44 message handling" which tries to consume multiple touchscreen/touchpad reports in a single transaction). Cc: stable@vger.kernel.org Reviewed-by: Chris Wilson Signed-off-by: Dmitry Torokhov Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_i2c.c | 66 +++++++++++++++++++++++++++----- 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index b522eb6e59a4..3da1af46625c 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1807,6 +1807,7 @@ enum skl_disp_power_wells { #define GMBUS_CYCLE_INDEX (2<<25) #define GMBUS_CYCLE_STOP (4<<25) #define GMBUS_BYTE_COUNT_SHIFT 16 +#define GMBUS_BYTE_COUNT_MAX 256U #define GMBUS_SLAVE_INDEX_SHIFT 8 #define GMBUS_SLAVE_ADDR_SHIFT 1 #define GMBUS_SLAVE_READ (1<<0) diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index b31088a551f2..56e437e31580 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -270,18 +270,17 @@ gmbus_wait_idle(struct drm_i915_private *dev_priv) } static int -gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg, - u32 gmbus1_index) +gmbus_xfer_read_chunk(struct drm_i915_private *dev_priv, + unsigned short addr, u8 *buf, unsigned int len, + u32 gmbus1_index) { int reg_offset = dev_priv->gpio_mmio_base; - u16 len = msg->len; - u8 *buf = msg->buf; I915_WRITE(GMBUS1 + reg_offset, gmbus1_index | GMBUS_CYCLE_WAIT | (len << GMBUS_BYTE_COUNT_SHIFT) | - (msg->addr << GMBUS_SLAVE_ADDR_SHIFT) | + (addr << GMBUS_SLAVE_ADDR_SHIFT) | GMBUS_SLAVE_READ | GMBUS_SW_RDY); while (len) { int ret; @@ -303,11 +302,35 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg, } static int -gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) +gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg, + u32 gmbus1_index) +{ + u8 *buf = msg->buf; + unsigned int rx_size = msg->len; + unsigned int len; + int ret; + + do { + len = min(rx_size, GMBUS_BYTE_COUNT_MAX); + + ret = gmbus_xfer_read_chunk(dev_priv, msg->addr, + buf, len, gmbus1_index); + if (ret) + return ret; + + rx_size -= len; + buf += len; + } while (rx_size != 0); + + return 0; +} + +static int +gmbus_xfer_write_chunk(struct drm_i915_private *dev_priv, + unsigned short addr, u8 *buf, unsigned int len) { int reg_offset = dev_priv->gpio_mmio_base; - u16 len = msg->len; - u8 *buf = msg->buf; + unsigned int chunk_size = len; u32 val, loop; val = loop = 0; @@ -319,8 +342,8 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) I915_WRITE(GMBUS3 + reg_offset, val); I915_WRITE(GMBUS1 + reg_offset, GMBUS_CYCLE_WAIT | - (msg->len << GMBUS_BYTE_COUNT_SHIFT) | - (msg->addr << GMBUS_SLAVE_ADDR_SHIFT) | + (chunk_size << GMBUS_BYTE_COUNT_SHIFT) | + (addr << GMBUS_SLAVE_ADDR_SHIFT) | GMBUS_SLAVE_WRITE | GMBUS_SW_RDY); while (len) { int ret; @@ -337,6 +360,29 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) if (ret) return ret; } + + return 0; +} + +static int +gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) +{ + u8 *buf = msg->buf; + unsigned int tx_size = msg->len; + unsigned int len; + int ret; + + do { + len = min(tx_size, GMBUS_BYTE_COUNT_MAX); + + ret = gmbus_xfer_write_chunk(dev_priv, msg->addr, buf, len); + if (ret) + return ret; + + buf += len; + tx_size -= len; + } while (tx_size != 0); + return 0; } From 53292cdb066950611e5bc2e0eb109c7edb42af78 Mon Sep 17 00:00:00 2001 From: Michel Thierry Date: Wed, 15 Apr 2015 18:11:33 +0100 Subject: [PATCH 2/3] drm/i915: Workaround to avoid lite restore with HEAD==TAIL WaIdleLiteRestore is an execlists-only workaround, and requires the driver to ensure that any context always has HEAD!=TAIL when attempting lite restore. Add two extra MI_NOOP instructions at the end of each request, but keep the requests tail pointing before the MI_NOOPs. We may not need to executed them, and this is why request->tail is sampled before adding these extra instructions. If we submit a context to the ELSP which has previously been submitted, move the tail pointer past the MI_NOOPs. This ensures HEAD!=TAIL. v2: Move overallocation to gen8_emit_request, and added note about sampling request->tail in commit message (Chris). v3: Remove redundant request->tail assignment in __i915_add_request, in lrc mode this is already set in execlists_context_queue. Do not add wa implementation details inside gem (Chris). v4: Apply the wa whenever the req has been resubmitted and update comment (Chris). Cc: Chris Wilson Reviewed-by: Chris Wilson Signed-off-by: Thomas Daniel Signed-off-by: Michel Thierry Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_gem.c | 3 ++- drivers/gpu/drm/i915/intel_lrc.c | 35 +++++++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index d07c0b1fb498..53394f998a1f 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2377,10 +2377,11 @@ int __i915_add_request(struct intel_engine_cs *ring, ret = ring->add_request(ring); if (ret) return ret; + + request->tail = intel_ring_get_tail(ringbuf); } request->head = request_start; - request->tail = intel_ring_get_tail(ringbuf); /* Whilst this request exists, batch_obj will be on the * active_list, and so will hold the active reference. Only when this diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index fcb074bd55dc..09df74b8e917 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -393,6 +393,26 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring) } } + if (IS_GEN8(ring->dev) || IS_GEN9(ring->dev)) { + /* + * WaIdleLiteRestore: make sure we never cause a lite + * restore with HEAD==TAIL + */ + if (req0 && req0->elsp_submitted) { + /* + * Apply the wa NOOPS to prevent ring:HEAD == req:TAIL + * as we resubmit the request. See gen8_emit_request() + * for where we prepare the padding after the end of the + * request. + */ + struct intel_ringbuffer *ringbuf; + + ringbuf = req0->ctx->engine[ring->id].ringbuf; + req0->tail += 8; + req0->tail &= ringbuf->size - 1; + } + } + WARN_ON(req1 && req1->elsp_submitted); execlists_submit_contexts(ring, req0->ctx, req0->tail, @@ -1315,7 +1335,12 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf, u32 cmd; int ret; - ret = intel_logical_ring_begin(ringbuf, request->ctx, 6); + /* + * Reserve space for 2 NOOPs at the end of each request to be + * used as a workaround for not being allowed to do lite + * restore with HEAD==TAIL (WaIdleLiteRestore). + */ + ret = intel_logical_ring_begin(ringbuf, request->ctx, 8); if (ret) return ret; @@ -1333,6 +1358,14 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf, intel_logical_ring_emit(ringbuf, MI_NOOP); intel_logical_ring_advance_and_submit(ringbuf, request->ctx, request); + /* + * Here we add two extra NOOPs as padding to avoid + * lite restore of a context with HEAD==TAIL. + */ + intel_logical_ring_emit(ringbuf, MI_NOOP); + intel_logical_ring_emit(ringbuf, MI_NOOP); + intel_logical_ring_advance(ringbuf); + return 0; } From b5f1c97f944482e98e6e39208af356630389d1ea Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Wed, 15 Apr 2015 16:52:30 -0700 Subject: [PATCH 3/3] drm/i915: vlv: fix save/restore of GFX_MAX_REQ_COUNT reg Due this typo we don't save/restore the GFX_MAX_REQ_COUNT register across suspend/resume, so fix this. This was introduced in commit ddeea5b0c36f3665446518c609be91f9336ef674 Author: Imre Deak Date: Mon May 5 15:19:56 2014 +0300 drm/i915: vlv: add runtime PM support I noticed this only by reading the code. To my knowledge it shouldn't cause any real problems at the moment, since the power well backing this register remains on across a runtime s/r. This may change once system-wide s0ix functionality is enabled in the kernel. v2: - resend after a missing git add -u :/ Cc: stable@vger.kernel.org Signed-off-by: Imre Deak Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) Signed-off-by: Rodrigo Vivi Reviewed-by: Mika Kuoppala Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 82f8be4b6745..9cc953e9b867 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1038,7 +1038,7 @@ static void vlv_save_gunit_s0ix_state(struct drm_i915_private *dev_priv) s->lra_limits[i] = I915_READ(GEN7_LRA_LIMITS_BASE + i * 4); s->media_max_req_count = I915_READ(GEN7_MEDIA_MAX_REQ_COUNT); - s->gfx_max_req_count = I915_READ(GEN7_MEDIA_MAX_REQ_COUNT); + s->gfx_max_req_count = I915_READ(GEN7_GFX_MAX_REQ_COUNT); s->render_hwsp = I915_READ(RENDER_HWS_PGA_GEN7); s->ecochk = I915_READ(GAM_ECOCHK); @@ -1119,7 +1119,7 @@ static void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv) I915_WRITE(GEN7_LRA_LIMITS_BASE + i * 4, s->lra_limits[i]); I915_WRITE(GEN7_MEDIA_MAX_REQ_COUNT, s->media_max_req_count); - I915_WRITE(GEN7_MEDIA_MAX_REQ_COUNT, s->gfx_max_req_count); + I915_WRITE(GEN7_GFX_MAX_REQ_COUNT, s->gfx_max_req_count); I915_WRITE(RENDER_HWS_PGA_GEN7, s->render_hwsp); I915_WRITE(GAM_ECOCHK, s->ecochk);