| Message ID | 1424366285-29232-9-git-send-email-John.C.Harrison@Intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19/02/2015 17:17, John.C.Harrison@Intel.com wrote: > From: John Harrison <John.C.Harrison@Intel.com> > > The alloc_request() function does not actually return the newly allocated > request. Instead, it must be pulled from ring->outstanding_lazy_request. This > patch fixes this so that code can create a request and start using it knowing > exactly which request it actually owns. > > For: VIZ-5115 > Signed-off-by: John Harrison <John.C.Harrison@Intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 3 ++- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 ++- > drivers/gpu/drm/i915/intel_lrc.c | 13 +++++++++---- > drivers/gpu/drm/i915/intel_lrc.h | 3 ++- > drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ++++++++++---- > drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++- > 6 files changed, 27 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 87a4a2e..90223f208 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1909,7 +1909,8 @@ struct drm_i915_private { > /* Abstract the submission mechanism (legacy ringbuffer or execlists) away */ > struct { > int (*alloc_request)(struct intel_engine_cs *ring, > - struct intel_context *ctx); > + struct intel_context *ctx, > + struct drm_i915_gem_request **req_out); > int (*do_execbuf)(struct i915_execbuffer_params *params, > struct drm_i915_gem_execbuffer2 *args, > struct list_head *vmas); > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 61471e9..37dcc6f 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1353,6 +1353,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > struct i915_address_space *vm; > struct i915_execbuffer_params params_master; /* XXX: will be removed later */ > struct i915_execbuffer_params *params = ¶ms_master; > + struct drm_i915_gem_request *request; Please initialize request to NULL. If you accidentally dereference it before it is allocated (seeing as the allocation is several pages down from here) you get a null pointer exception, which is a clear indication that you did something stupid. Otherwise it's not clear what will happen (sure, page fault, but null pointer exception is a better failure indication). Thanks, Tomas > const u32 ctx_id = i915_execbuffer2_get_context_id(*args); > u32 dispatch_flags; > int ret; > @@ -1531,7 +1532,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > params->batch_obj_vm_offset = i915_gem_obj_offset(batch_obj, vm); > > /* Allocate a request for this batch buffer nice and early. */ > - ret = dev_priv->gt.alloc_request(ring, ctx); > + ret = dev_priv->gt.alloc_request(ring, ctx, &request); > if (ret) > goto err; > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 8628abf..c3c783f 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -857,13 +857,17 @@ void intel_lr_context_unpin(struct intel_engine_cs *ring, > } > > int intel_logical_ring_alloc_request(struct intel_engine_cs *ring, > - struct intel_context *ctx) > + struct intel_context *ctx, > + struct drm_i915_gem_request **req_out) > { > struct drm_i915_gem_request *request; > struct drm_i915_private *dev_private = ring->dev->dev_private; > int ret; > > - if (ring->outstanding_lazy_request) > + if (!req_out) > + return -EINVAL; > + > + if ((*req_out = ring->outstanding_lazy_request) != NULL) > return 0; > > request = kzalloc(sizeof(*request), GFP_KERNEL); > @@ -898,7 +902,7 @@ int intel_logical_ring_alloc_request(struct intel_engine_cs *ring, > i915_gem_context_reference(request->ctx); > request->ringbuf = ctx->engine[ring->id].ringbuf; > > - ring->outstanding_lazy_request = request; > + *req_out = ring->outstanding_lazy_request = request; > return 0; > } > > @@ -1051,6 +1055,7 @@ static int logical_ring_prepare(struct intel_ringbuffer *ringbuf, > int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, > struct intel_context *ctx, int num_dwords) > { > + struct drm_i915_gem_request *req; > struct intel_engine_cs *ring = ringbuf->ring; > struct drm_device *dev = ring->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -1066,7 +1071,7 @@ int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, > return ret; > > /* Preallocate the olr before touching the ring */ > - ret = intel_logical_ring_alloc_request(ring, ctx); > + ret = intel_logical_ring_alloc_request(ring, ctx, &req); > if (ret) > return ret; > > diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h > index 3cc38bd..77de8ac 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.h > +++ b/drivers/gpu/drm/i915/intel_lrc.h > @@ -37,7 +37,8 @@ > > /* Logical Rings */ > int __must_check intel_logical_ring_alloc_request(struct intel_engine_cs *ring, > - struct intel_context *ctx); > + struct intel_context *ctx, > + struct drm_i915_gem_request **req_out); > void intel_logical_ring_stop(struct intel_engine_cs *ring); > void intel_logical_ring_cleanup(struct intel_engine_cs *ring); > int intel_logical_rings_init(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 635707a..1a9f884 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -2164,13 +2164,18 @@ int intel_ring_idle(struct intel_engine_cs *ring) > } > > int > -intel_ring_alloc_request(struct intel_engine_cs *ring, struct intel_context *ctx) > +intel_ring_alloc_request(struct intel_engine_cs *ring, > + struct intel_context *ctx, > + struct drm_i915_gem_request **req_out) > { > int ret; > struct drm_i915_gem_request *request; > struct drm_i915_private *dev_private = ring->dev->dev_private; > > - if (ring->outstanding_lazy_request) > + if (!req_out) > + return -EINVAL; > + > + if ((*req_out = ring->outstanding_lazy_request) != NULL) > return 0; > > request = kzalloc(sizeof(*request), GFP_KERNEL); > @@ -2188,7 +2193,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring, struct intel_context *ctx > return ret; > } > > - ring->outstanding_lazy_request = request; > + *req_out = ring->outstanding_lazy_request = request; > return 0; > } > > @@ -2216,6 +2221,7 @@ static int __intel_ring_prepare(struct intel_engine_cs *ring, > int intel_ring_begin(struct intel_engine_cs *ring, > int num_dwords) > { > + struct drm_i915_gem_request *req; > struct drm_i915_private *dev_priv = ring->dev->dev_private; > int ret; > > @@ -2229,7 +2235,7 @@ int intel_ring_begin(struct intel_engine_cs *ring, > return ret; > > /* Preallocate the olr before touching the ring */ > - ret = intel_ring_alloc_request(ring, NULL); > + ret = intel_ring_alloc_request(ring, NULL, &req); > if (ret) > return ret; > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 2fd960a..4f8a14a 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -393,7 +393,8 @@ void intel_cleanup_ring_buffer(struct intel_engine_cs *ring); > int __must_check intel_ring_begin(struct intel_engine_cs *ring, int n); > int __must_check intel_ring_cacheline_align(struct intel_engine_cs *ring); > int __must_check intel_ring_alloc_request(struct intel_engine_cs *ring, > - struct intel_context *ctx); > + struct intel_context *ctx, > + struct drm_i915_gem_request **req_out); > static inline void intel_ring_emit(struct intel_engine_cs *ring, > u32 data) > { >
On 05/03/2015 15:27, Tomas Elf wrote: > On 19/02/2015 17:17, John.C.Harrison@Intel.com wrote: >> From: John Harrison <John.C.Harrison@Intel.com> >> >> The alloc_request() function does not actually return the newly >> allocated >> request. Instead, it must be pulled from >> ring->outstanding_lazy_request. This >> patch fixes this so that code can create a request and start using it >> knowing >> exactly which request it actually owns. >> >> For: VIZ-5115 >> Signed-off-by: John Harrison <John.C.Harrison@Intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 3 ++- >> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 ++- >> drivers/gpu/drm/i915/intel_lrc.c | 13 +++++++++---- >> drivers/gpu/drm/i915/intel_lrc.h | 3 ++- >> drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ++++++++++---- >> drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++- >> 6 files changed, 27 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h >> index 87a4a2e..90223f208 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -1909,7 +1909,8 @@ struct drm_i915_private { >> /* Abstract the submission mechanism (legacy ringbuffer or >> execlists) away */ >> struct { >> int (*alloc_request)(struct intel_engine_cs *ring, >> - struct intel_context *ctx); >> + struct intel_context *ctx, >> + struct drm_i915_gem_request **req_out); >> int (*do_execbuf)(struct i915_execbuffer_params *params, >> struct drm_i915_gem_execbuffer2 *args, >> struct list_head *vmas); >> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> index 61471e9..37dcc6f 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> @@ -1353,6 +1353,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, >> void *data, >> struct i915_address_space *vm; >> struct i915_execbuffer_params params_master; /* XXX: will be >> removed later */ >> struct i915_execbuffer_params *params = ¶ms_master; >> + struct drm_i915_gem_request *request; > > Please initialize request to NULL. If you accidentally dereference it > before it is allocated (seeing as the allocation is several pages down > from here) you get a null pointer exception, which is a clear > indication that you did something stupid. Otherwise it's not clear > what will happen (sure, page fault, but null pointer exception is a > better failure indication). That should generate a 'use before assignment' compiler warning. > > Thanks, > Tomas > >> const u32 ctx_id = i915_execbuffer2_get_context_id(*args); >> u32 dispatch_flags; >> int ret; >> @@ -1531,7 +1532,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, >> void *data, >> params->batch_obj_vm_offset = >> i915_gem_obj_offset(batch_obj, vm); >> >> /* Allocate a request for this batch buffer nice and early. */ >> - ret = dev_priv->gt.alloc_request(ring, ctx); >> + ret = dev_priv->gt.alloc_request(ring, ctx, &request); >> if (ret) >> goto err; >> >> diff --git a/drivers/gpu/drm/i915/intel_lrc.c >> b/drivers/gpu/drm/i915/intel_lrc.c >> index 8628abf..c3c783f 100644 >> --- a/drivers/gpu/drm/i915/intel_lrc.c >> +++ b/drivers/gpu/drm/i915/intel_lrc.c >> @@ -857,13 +857,17 @@ void intel_lr_context_unpin(struct >> intel_engine_cs *ring, >> } >> >> int intel_logical_ring_alloc_request(struct intel_engine_cs *ring, >> - struct intel_context *ctx) >> + struct intel_context *ctx, >> + struct drm_i915_gem_request **req_out) >> { >> struct drm_i915_gem_request *request; >> struct drm_i915_private *dev_private = ring->dev->dev_private; >> int ret; >> >> - if (ring->outstanding_lazy_request) >> + if (!req_out) >> + return -EINVAL; >> + >> + if ((*req_out = ring->outstanding_lazy_request) != NULL) >> return 0; >> >> request = kzalloc(sizeof(*request), GFP_KERNEL); >> @@ -898,7 +902,7 @@ int intel_logical_ring_alloc_request(struct >> intel_engine_cs *ring, >> i915_gem_context_reference(request->ctx); >> request->ringbuf = ctx->engine[ring->id].ringbuf; >> >> - ring->outstanding_lazy_request = request; >> + *req_out = ring->outstanding_lazy_request = request; >> return 0; >> } >> >> @@ -1051,6 +1055,7 @@ static int logical_ring_prepare(struct >> intel_ringbuffer *ringbuf, >> int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, >> struct intel_context *ctx, int num_dwords) >> { >> + struct drm_i915_gem_request *req; >> struct intel_engine_cs *ring = ringbuf->ring; >> struct drm_device *dev = ring->dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> @@ -1066,7 +1071,7 @@ int intel_logical_ring_begin(struct >> intel_ringbuffer *ringbuf, >> return ret; >> >> /* Preallocate the olr before touching the ring */ >> - ret = intel_logical_ring_alloc_request(ring, ctx); >> + ret = intel_logical_ring_alloc_request(ring, ctx, &req); >> if (ret) >> return ret; >> >> diff --git a/drivers/gpu/drm/i915/intel_lrc.h >> b/drivers/gpu/drm/i915/intel_lrc.h >> index 3cc38bd..77de8ac 100644 >> --- a/drivers/gpu/drm/i915/intel_lrc.h >> +++ b/drivers/gpu/drm/i915/intel_lrc.h >> @@ -37,7 +37,8 @@ >> >> /* Logical Rings */ >> int __must_check intel_logical_ring_alloc_request(struct >> intel_engine_cs *ring, >> - struct intel_context *ctx); >> + struct intel_context *ctx, >> + struct drm_i915_gem_request **req_out); >> void intel_logical_ring_stop(struct intel_engine_cs *ring); >> void intel_logical_ring_cleanup(struct intel_engine_cs *ring); >> int intel_logical_rings_init(struct drm_device *dev); >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c >> b/drivers/gpu/drm/i915/intel_ringbuffer.c >> index 635707a..1a9f884 100644 >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c >> @@ -2164,13 +2164,18 @@ int intel_ring_idle(struct intel_engine_cs >> *ring) >> } >> >> int >> -intel_ring_alloc_request(struct intel_engine_cs *ring, struct >> intel_context *ctx) >> +intel_ring_alloc_request(struct intel_engine_cs *ring, >> + struct intel_context *ctx, >> + struct drm_i915_gem_request **req_out) >> { >> int ret; >> struct drm_i915_gem_request *request; >> struct drm_i915_private *dev_private = ring->dev->dev_private; >> >> - if (ring->outstanding_lazy_request) >> + if (!req_out) >> + return -EINVAL; >> + >> + if ((*req_out = ring->outstanding_lazy_request) != NULL) >> return 0; >> >> request = kzalloc(sizeof(*request), GFP_KERNEL); >> @@ -2188,7 +2193,7 @@ intel_ring_alloc_request(struct intel_engine_cs >> *ring, struct intel_context *ctx >> return ret; >> } >> >> - ring->outstanding_lazy_request = request; >> + *req_out = ring->outstanding_lazy_request = request; >> return 0; >> } >> >> @@ -2216,6 +2221,7 @@ static int __intel_ring_prepare(struct >> intel_engine_cs *ring, >> int intel_ring_begin(struct intel_engine_cs *ring, >> int num_dwords) >> { >> + struct drm_i915_gem_request *req; >> struct drm_i915_private *dev_priv = ring->dev->dev_private; >> int ret; >> >> @@ -2229,7 +2235,7 @@ int intel_ring_begin(struct intel_engine_cs *ring, >> return ret; >> >> /* Preallocate the olr before touching the ring */ >> - ret = intel_ring_alloc_request(ring, NULL); >> + ret = intel_ring_alloc_request(ring, NULL, &req); >> if (ret) >> return ret; >> >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h >> b/drivers/gpu/drm/i915/intel_ringbuffer.h >> index 2fd960a..4f8a14a 100644 >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h >> @@ -393,7 +393,8 @@ void intel_cleanup_ring_buffer(struct >> intel_engine_cs *ring); >> int __must_check intel_ring_begin(struct intel_engine_cs *ring, int >> n); >> int __must_check intel_ring_cacheline_align(struct intel_engine_cs >> *ring); >> int __must_check intel_ring_alloc_request(struct intel_engine_cs >> *ring, >> - struct intel_context *ctx); >> + struct intel_context *ctx, >> + struct drm_i915_gem_request **req_out); >> static inline void intel_ring_emit(struct intel_engine_cs *ring, >> u32 data) >> { >> >
On 05/03/2015 15:46, John Harrison wrote: > On 05/03/2015 15:27, Tomas Elf wrote: >> On 19/02/2015 17:17, John.C.Harrison@Intel.com wrote: >>> From: John Harrison <John.C.Harrison@Intel.com> >>> >>> The alloc_request() function does not actually return the newly >>> allocated >>> request. Instead, it must be pulled from >>> ring->outstanding_lazy_request. This >>> patch fixes this so that code can create a request and start using it >>> knowing >>> exactly which request it actually owns. >>> >>> For: VIZ-5115 >>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com> >>> --- >>> drivers/gpu/drm/i915/i915_drv.h | 3 ++- >>> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 ++- >>> drivers/gpu/drm/i915/intel_lrc.c | 13 +++++++++---- >>> drivers/gpu/drm/i915/intel_lrc.h | 3 ++- >>> drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ++++++++++---- >>> drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++- >>> 6 files changed, 27 insertions(+), 12 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_drv.h >>> b/drivers/gpu/drm/i915/i915_drv.h >>> index 87a4a2e..90223f208 100644 >>> --- a/drivers/gpu/drm/i915/i915_drv.h >>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>> @@ -1909,7 +1909,8 @@ struct drm_i915_private { >>> /* Abstract the submission mechanism (legacy ringbuffer or >>> execlists) away */ >>> struct { >>> int (*alloc_request)(struct intel_engine_cs *ring, >>> - struct intel_context *ctx); >>> + struct intel_context *ctx, >>> + struct drm_i915_gem_request **req_out); >>> int (*do_execbuf)(struct i915_execbuffer_params *params, >>> struct drm_i915_gem_execbuffer2 *args, >>> struct list_head *vmas); >>> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c >>> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c >>> index 61471e9..37dcc6f 100644 >>> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c >>> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c >>> @@ -1353,6 +1353,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, >>> void *data, >>> struct i915_address_space *vm; >>> struct i915_execbuffer_params params_master; /* XXX: will be >>> removed later */ >>> struct i915_execbuffer_params *params = ¶ms_master; >>> + struct drm_i915_gem_request *request; >> >> Please initialize request to NULL. If you accidentally dereference it >> before it is allocated (seeing as the allocation is several pages down >> from here) you get a null pointer exception, which is a clear >> indication that you did something stupid. Otherwise it's not clear >> what will happen (sure, page fault, but null pointer exception is a >> better failure indication). > > That should generate a 'use before assignment' compiler warning. That assumes that the developer in question isn't too busy hacking to check for warnings (in all honesty that developer probably would've been me ;)). Sure, you should always check for warnings but if we can save this developer some time by giving them a clear run-time indication aside from the compile-time warning then that would not be a bad thing. I've been there myself a few times and I know times in the past where this would've saved me the time it takes to rebuild and redeploy the kernel once. Thanks, Tomas > >> >> Thanks, >> Tomas >> >>> const u32 ctx_id = i915_execbuffer2_get_context_id(*args); >>> u32 dispatch_flags; >>> int ret; >>> @@ -1531,7 +1532,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, >>> void *data, >>> params->batch_obj_vm_offset = >>> i915_gem_obj_offset(batch_obj, vm); >>> >>> /* Allocate a request for this batch buffer nice and early. */ >>> - ret = dev_priv->gt.alloc_request(ring, ctx); >>> + ret = dev_priv->gt.alloc_request(ring, ctx, &request); >>> if (ret) >>> goto err; >>> >>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c >>> b/drivers/gpu/drm/i915/intel_lrc.c >>> index 8628abf..c3c783f 100644 >>> --- a/drivers/gpu/drm/i915/intel_lrc.c >>> +++ b/drivers/gpu/drm/i915/intel_lrc.c >>> @@ -857,13 +857,17 @@ void intel_lr_context_unpin(struct >>> intel_engine_cs *ring, >>> } >>> >>> int intel_logical_ring_alloc_request(struct intel_engine_cs *ring, >>> - struct intel_context *ctx) >>> + struct intel_context *ctx, >>> + struct drm_i915_gem_request **req_out) >>> { >>> struct drm_i915_gem_request *request; >>> struct drm_i915_private *dev_private = ring->dev->dev_private; >>> int ret; >>> >>> - if (ring->outstanding_lazy_request) >>> + if (!req_out) >>> + return -EINVAL; >>> + >>> + if ((*req_out = ring->outstanding_lazy_request) != NULL) >>> return 0; >>> >>> request = kzalloc(sizeof(*request), GFP_KERNEL); >>> @@ -898,7 +902,7 @@ int intel_logical_ring_alloc_request(struct >>> intel_engine_cs *ring, >>> i915_gem_context_reference(request->ctx); >>> request->ringbuf = ctx->engine[ring->id].ringbuf; >>> >>> - ring->outstanding_lazy_request = request; >>> + *req_out = ring->outstanding_lazy_request = request; >>> return 0; >>> } >>> >>> @@ -1051,6 +1055,7 @@ static int logical_ring_prepare(struct >>> intel_ringbuffer *ringbuf, >>> int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, >>> struct intel_context *ctx, int num_dwords) >>> { >>> + struct drm_i915_gem_request *req; >>> struct intel_engine_cs *ring = ringbuf->ring; >>> struct drm_device *dev = ring->dev; >>> struct drm_i915_private *dev_priv = dev->dev_private; >>> @@ -1066,7 +1071,7 @@ int intel_logical_ring_begin(struct >>> intel_ringbuffer *ringbuf, >>> return ret; >>> >>> /* Preallocate the olr before touching the ring */ >>> - ret = intel_logical_ring_alloc_request(ring, ctx); >>> + ret = intel_logical_ring_alloc_request(ring, ctx, &req); >>> if (ret) >>> return ret; >>> >>> diff --git a/drivers/gpu/drm/i915/intel_lrc.h >>> b/drivers/gpu/drm/i915/intel_lrc.h >>> index 3cc38bd..77de8ac 100644 >>> --- a/drivers/gpu/drm/i915/intel_lrc.h >>> +++ b/drivers/gpu/drm/i915/intel_lrc.h >>> @@ -37,7 +37,8 @@ >>> >>> /* Logical Rings */ >>> int __must_check intel_logical_ring_alloc_request(struct >>> intel_engine_cs *ring, >>> - struct intel_context *ctx); >>> + struct intel_context *ctx, >>> + struct drm_i915_gem_request **req_out); >>> void intel_logical_ring_stop(struct intel_engine_cs *ring); >>> void intel_logical_ring_cleanup(struct intel_engine_cs *ring); >>> int intel_logical_rings_init(struct drm_device *dev); >>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c >>> b/drivers/gpu/drm/i915/intel_ringbuffer.c >>> index 635707a..1a9f884 100644 >>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c >>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c >>> @@ -2164,13 +2164,18 @@ int intel_ring_idle(struct intel_engine_cs >>> *ring) >>> } >>> >>> int >>> -intel_ring_alloc_request(struct intel_engine_cs *ring, struct >>> intel_context *ctx) >>> +intel_ring_alloc_request(struct intel_engine_cs *ring, >>> + struct intel_context *ctx, >>> + struct drm_i915_gem_request **req_out) >>> { >>> int ret; >>> struct drm_i915_gem_request *request; >>> struct drm_i915_private *dev_private = ring->dev->dev_private; >>> >>> - if (ring->outstanding_lazy_request) >>> + if (!req_out) >>> + return -EINVAL; >>> + >>> + if ((*req_out = ring->outstanding_lazy_request) != NULL) >>> return 0; >>> >>> request = kzalloc(sizeof(*request), GFP_KERNEL); >>> @@ -2188,7 +2193,7 @@ intel_ring_alloc_request(struct intel_engine_cs >>> *ring, struct intel_context *ctx >>> return ret; >>> } >>> >>> - ring->outstanding_lazy_request = request; >>> + *req_out = ring->outstanding_lazy_request = request; >>> return 0; >>> } >>> >>> @@ -2216,6 +2221,7 @@ static int __intel_ring_prepare(struct >>> intel_engine_cs *ring, >>> int intel_ring_begin(struct intel_engine_cs *ring, >>> int num_dwords) >>> { >>> + struct drm_i915_gem_request *req; >>> struct drm_i915_private *dev_priv = ring->dev->dev_private; >>> int ret; >>> >>> @@ -2229,7 +2235,7 @@ int intel_ring_begin(struct intel_engine_cs *ring, >>> return ret; >>> >>> /* Preallocate the olr before touching the ring */ >>> - ret = intel_ring_alloc_request(ring, NULL); >>> + ret = intel_ring_alloc_request(ring, NULL, &req); >>> if (ret) >>> return ret; >>> >>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h >>> b/drivers/gpu/drm/i915/intel_ringbuffer.h >>> index 2fd960a..4f8a14a 100644 >>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h >>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h >>> @@ -393,7 +393,8 @@ void intel_cleanup_ring_buffer(struct >>> intel_engine_cs *ring); >>> int __must_check intel_ring_begin(struct intel_engine_cs *ring, int >>> n); >>> int __must_check intel_ring_cacheline_align(struct intel_engine_cs >>> *ring); >>> int __must_check intel_ring_alloc_request(struct intel_engine_cs >>> *ring, >>> - struct intel_context *ctx); >>> + struct intel_context *ctx, >>> + struct drm_i915_gem_request **req_out); >>> static inline void intel_ring_emit(struct intel_engine_cs *ring, >>> u32 data) >>> { >>> >> >
On Thu, Mar 05, 2015 at 08:13:07PM +0000, Tomas Elf wrote: > On 05/03/2015 15:46, John Harrison wrote: > >On 05/03/2015 15:27, Tomas Elf wrote: > >>On 19/02/2015 17:17, John.C.Harrison@Intel.com wrote: > >>>From: John Harrison <John.C.Harrison@Intel.com> > >>> > >>>The alloc_request() function does not actually return the newly > >>>allocated > >>>request. Instead, it must be pulled from > >>>ring->outstanding_lazy_request. This > >>>patch fixes this so that code can create a request and start using it > >>>knowing > >>>exactly which request it actually owns. > >>> > >>>For: VIZ-5115 > >>>Signed-off-by: John Harrison <John.C.Harrison@Intel.com> > >>>--- > >>> drivers/gpu/drm/i915/i915_drv.h | 3 ++- > >>> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 ++- > >>> drivers/gpu/drm/i915/intel_lrc.c | 13 +++++++++---- > >>> drivers/gpu/drm/i915/intel_lrc.h | 3 ++- > >>> drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ++++++++++---- > >>> drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++- > >>> 6 files changed, 27 insertions(+), 12 deletions(-) > >>> > >>>diff --git a/drivers/gpu/drm/i915/i915_drv.h > >>>b/drivers/gpu/drm/i915/i915_drv.h > >>>index 87a4a2e..90223f208 100644 > >>>--- a/drivers/gpu/drm/i915/i915_drv.h > >>>+++ b/drivers/gpu/drm/i915/i915_drv.h > >>>@@ -1909,7 +1909,8 @@ struct drm_i915_private { > >>> /* Abstract the submission mechanism (legacy ringbuffer or > >>>execlists) away */ > >>> struct { > >>> int (*alloc_request)(struct intel_engine_cs *ring, > >>>- struct intel_context *ctx); > >>>+ struct intel_context *ctx, > >>>+ struct drm_i915_gem_request **req_out); > >>> int (*do_execbuf)(struct i915_execbuffer_params *params, > >>> struct drm_i915_gem_execbuffer2 *args, > >>> struct list_head *vmas); > >>>diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > >>>b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > >>>index 61471e9..37dcc6f 100644 > >>>--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > >>>+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > >>>@@ -1353,6 +1353,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, > >>>void *data, > >>> struct i915_address_space *vm; > >>> struct i915_execbuffer_params params_master; /* XXX: will be > >>>removed later */ > >>> struct i915_execbuffer_params *params = ¶ms_master; > >>>+ struct drm_i915_gem_request *request; > >> > >>Please initialize request to NULL. If you accidentally dereference it > >>before it is allocated (seeing as the allocation is several pages down > >>from here) you get a null pointer exception, which is a clear > >>indication that you did something stupid. Otherwise it's not clear > >>what will happen (sure, page fault, but null pointer exception is a > >>better failure indication). > > > >That should generate a 'use before assignment' compiler warning. > > That assumes that the developer in question isn't too busy hacking to check > for warnings (in all honesty that developer probably would've been me ;)). > Sure, you should always check for warnings but if we can save this developer > some time by giving them a clear run-time indication aside from the > compile-time warning then that would not be a bad thing. I've been there > myself a few times and I know times in the past where this would've saved me > the time it takes to rebuild and redeploy the kernel once. kbuild is _very_ good at catching and reporting these. And the linux kernel generally has a 0 warnings rule, which means even when it slips through a lot of developers will get mad at me for not spotting it. I think relying upon gcc to do its job here is safe enough. We only initialize stack variables if it's really needed, or if gcc is too dense to figure it out itself. -Daniel
On 06/03/2015 16:18, Daniel Vetter wrote: > On Thu, Mar 05, 2015 at 08:13:07PM +0000, Tomas Elf wrote: >> On 05/03/2015 15:46, John Harrison wrote: >>> On 05/03/2015 15:27, Tomas Elf wrote: >>>> On 19/02/2015 17:17, John.C.Harrison@Intel.com wrote: >>>>> From: John Harrison <John.C.Harrison@Intel.com> >>>>> >>>>> The alloc_request() function does not actually return the newly >>>>> allocated >>>>> request. Instead, it must be pulled from >>>>> ring->outstanding_lazy_request. This >>>>> patch fixes this so that code can create a request and start using it >>>>> knowing >>>>> exactly which request it actually owns. >>>>> >>>>> For: VIZ-5115 >>>>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com> >>>>> --- >>>>> drivers/gpu/drm/i915/i915_drv.h | 3 ++- >>>>> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 ++- >>>>> drivers/gpu/drm/i915/intel_lrc.c | 13 +++++++++---- >>>>> drivers/gpu/drm/i915/intel_lrc.h | 3 ++- >>>>> drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ++++++++++---- >>>>> drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++- >>>>> 6 files changed, 27 insertions(+), 12 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h >>>>> b/drivers/gpu/drm/i915/i915_drv.h >>>>> index 87a4a2e..90223f208 100644 >>>>> --- a/drivers/gpu/drm/i915/i915_drv.h >>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>>>> @@ -1909,7 +1909,8 @@ struct drm_i915_private { >>>>> /* Abstract the submission mechanism (legacy ringbuffer or >>>>> execlists) away */ >>>>> struct { >>>>> int (*alloc_request)(struct intel_engine_cs *ring, >>>>> - struct intel_context *ctx); >>>>> + struct intel_context *ctx, >>>>> + struct drm_i915_gem_request **req_out); >>>>> int (*do_execbuf)(struct i915_execbuffer_params *params, >>>>> struct drm_i915_gem_execbuffer2 *args, >>>>> struct list_head *vmas); >>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c >>>>> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c >>>>> index 61471e9..37dcc6f 100644 >>>>> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c >>>>> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c >>>>> @@ -1353,6 +1353,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, >>>>> void *data, >>>>> struct i915_address_space *vm; >>>>> struct i915_execbuffer_params params_master; /* XXX: will be >>>>> removed later */ >>>>> struct i915_execbuffer_params *params = ¶ms_master; >>>>> + struct drm_i915_gem_request *request; >>>> Please initialize request to NULL. If you accidentally dereference it >>>> before it is allocated (seeing as the allocation is several pages down >>> >from here) you get a null pointer exception, which is a clear >>>> indication that you did something stupid. Otherwise it's not clear >>>> what will happen (sure, page fault, but null pointer exception is a >>>> better failure indication). >>> That should generate a 'use before assignment' compiler warning. >> That assumes that the developer in question isn't too busy hacking to check >> for warnings (in all honesty that developer probably would've been me ;)). >> Sure, you should always check for warnings but if we can save this developer >> some time by giving them a clear run-time indication aside from the >> compile-time warning then that would not be a bad thing. I've been there >> myself a few times and I know times in the past where this would've saved me >> the time it takes to rebuild and redeploy the kernel once. > kbuild is _very_ good at catching and reporting these. And the linux > kernel generally has a 0 warnings rule, which means even when it slips > through a lot of developers will get mad at me for not spotting it. > > I think relying upon gcc to do its job here is safe enough. We only > initialize stack variables if it's really needed, or if gcc is too dense > to figure it out itself. > -Daniel I agree with Daniel. If you ignore warnings in your own test builds, you get what you deserve. If you check warnings into the tree then you should be shot. Or worse.
On 06/03/2015 17:36, John Harrison wrote: > On 06/03/2015 16:18, Daniel Vetter wrote: >> On Thu, Mar 05, 2015 at 08:13:07PM +0000, Tomas Elf wrote: >>> On 05/03/2015 15:46, John Harrison wrote: >>>> On 05/03/2015 15:27, Tomas Elf wrote: >>>>> On 19/02/2015 17:17, John.C.Harrison@Intel.com wrote: >>>>>> From: John Harrison <John.C.Harrison@Intel.com> >>>>>> >>>>>> The alloc_request() function does not actually return the newly >>>>>> allocated >>>>>> request. Instead, it must be pulled from >>>>>> ring->outstanding_lazy_request. This >>>>>> patch fixes this so that code can create a request and start using it >>>>>> knowing >>>>>> exactly which request it actually owns. >>>>>> >>>>>> For: VIZ-5115 >>>>>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com> >>>>>> --- >>>>>> drivers/gpu/drm/i915/i915_drv.h | 3 ++- >>>>>> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 ++- >>>>>> drivers/gpu/drm/i915/intel_lrc.c | 13 +++++++++---- >>>>>> drivers/gpu/drm/i915/intel_lrc.h | 3 ++- >>>>>> drivers/gpu/drm/i915/intel_ringbuffer.c | 14 ++++++++++---- >>>>>> drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++- >>>>>> 6 files changed, 27 insertions(+), 12 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h >>>>>> b/drivers/gpu/drm/i915/i915_drv.h >>>>>> index 87a4a2e..90223f208 100644 >>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h >>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>>>>> @@ -1909,7 +1909,8 @@ struct drm_i915_private { >>>>>> /* Abstract the submission mechanism (legacy ringbuffer or >>>>>> execlists) away */ >>>>>> struct { >>>>>> int (*alloc_request)(struct intel_engine_cs *ring, >>>>>> - struct intel_context *ctx); >>>>>> + struct intel_context *ctx, >>>>>> + struct drm_i915_gem_request **req_out); >>>>>> int (*do_execbuf)(struct i915_execbuffer_params *params, >>>>>> struct drm_i915_gem_execbuffer2 *args, >>>>>> struct list_head *vmas); >>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c >>>>>> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c >>>>>> index 61471e9..37dcc6f 100644 >>>>>> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c >>>>>> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c >>>>>> @@ -1353,6 +1353,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, >>>>>> void *data, >>>>>> struct i915_address_space *vm; >>>>>> struct i915_execbuffer_params params_master; /* XXX: will be >>>>>> removed later */ >>>>>> struct i915_execbuffer_params *params = ¶ms_master; >>>>>> + struct drm_i915_gem_request *request; >>>>> Please initialize request to NULL. If you accidentally dereference it >>>>> before it is allocated (seeing as the allocation is several pages down >>>> >from here) you get a null pointer exception, which is a clear >>>>> indication that you did something stupid. Otherwise it's not clear >>>>> what will happen (sure, page fault, but null pointer exception is a >>>>> better failure indication). >>>> That should generate a 'use before assignment' compiler warning. >>> That assumes that the developer in question isn't too busy hacking to >>> check >>> for warnings (in all honesty that developer probably would've been me >>> ;)). >>> Sure, you should always check for warnings but if we can save this >>> developer >>> some time by giving them a clear run-time indication aside from the >>> compile-time warning then that would not be a bad thing. I've been there >>> myself a few times and I know times in the past where this would've >>> saved me >>> the time it takes to rebuild and redeploy the kernel once. >> kbuild is _very_ good at catching and reporting these. And the linux >> kernel generally has a 0 warnings rule, which means even when it slips >> through a lot of developers will get mad at me for not spotting it. >> >> I think relying upon gcc to do its job here is safe enough. We only >> initialize stack variables if it's really needed, or if gcc is too dense >> to figure it out itself. >> -Daniel > > I agree with Daniel. If you ignore warnings in your own test builds, you > get what you deserve. If you check warnings into the tree then you > should be shot. Or worse. > > Reviewed-by: Tomas Elf <tomas.elf@intel.com> Thanks, Tomas
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 87a4a2e..90223f208 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1909,7 +1909,8 @@ struct drm_i915_private { /* Abstract the submission mechanism (legacy ringbuffer or execlists) away */ struct { int (*alloc_request)(struct intel_engine_cs *ring, - struct intel_context *ctx); + struct intel_context *ctx, + struct drm_i915_gem_request **req_out); int (*do_execbuf)(struct i915_execbuffer_params *params, struct drm_i915_gem_execbuffer2 *args, struct list_head *vmas); diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 61471e9..37dcc6f 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1353,6 +1353,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, struct i915_address_space *vm; struct i915_execbuffer_params params_master; /* XXX: will be removed later */ struct i915_execbuffer_params *params = ¶ms_master; + struct drm_i915_gem_request *request; const u32 ctx_id = i915_execbuffer2_get_context_id(*args); u32 dispatch_flags; int ret; @@ -1531,7 +1532,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, params->batch_obj_vm_offset = i915_gem_obj_offset(batch_obj, vm); /* Allocate a request for this batch buffer nice and early. */ - ret = dev_priv->gt.alloc_request(ring, ctx); + ret = dev_priv->gt.alloc_request(ring, ctx, &request); if (ret) goto err; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 8628abf..c3c783f 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -857,13 +857,17 @@ void intel_lr_context_unpin(struct intel_engine_cs *ring, } int intel_logical_ring_alloc_request(struct intel_engine_cs *ring, - struct intel_context *ctx) + struct intel_context *ctx, + struct drm_i915_gem_request **req_out) { struct drm_i915_gem_request *request; struct drm_i915_private *dev_private = ring->dev->dev_private; int ret; - if (ring->outstanding_lazy_request) + if (!req_out) + return -EINVAL; + + if ((*req_out = ring->outstanding_lazy_request) != NULL) return 0; request = kzalloc(sizeof(*request), GFP_KERNEL); @@ -898,7 +902,7 @@ int intel_logical_ring_alloc_request(struct intel_engine_cs *ring, i915_gem_context_reference(request->ctx); request->ringbuf = ctx->engine[ring->id].ringbuf; - ring->outstanding_lazy_request = request; + *req_out = ring->outstanding_lazy_request = request; return 0; } @@ -1051,6 +1055,7 @@ static int logical_ring_prepare(struct intel_ringbuffer *ringbuf, int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, struct intel_context *ctx, int num_dwords) { + struct drm_i915_gem_request *req; struct intel_engine_cs *ring = ringbuf->ring; struct drm_device *dev = ring->dev; struct drm_i915_private *dev_priv = dev->dev_private; @@ -1066,7 +1071,7 @@ int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, return ret; /* Preallocate the olr before touching the ring */ - ret = intel_logical_ring_alloc_request(ring, ctx); + ret = intel_logical_ring_alloc_request(ring, ctx, &req); if (ret) return ret; diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index 3cc38bd..77de8ac 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -37,7 +37,8 @@ /* Logical Rings */ int __must_check intel_logical_ring_alloc_request(struct intel_engine_cs *ring, - struct intel_context *ctx); + struct intel_context *ctx, + struct drm_i915_gem_request **req_out); void intel_logical_ring_stop(struct intel_engine_cs *ring); void intel_logical_ring_cleanup(struct intel_engine_cs *ring); int intel_logical_rings_init(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 635707a..1a9f884 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2164,13 +2164,18 @@ int intel_ring_idle(struct intel_engine_cs *ring) } int -intel_ring_alloc_request(struct intel_engine_cs *ring, struct intel_context *ctx) +intel_ring_alloc_request(struct intel_engine_cs *ring, + struct intel_context *ctx, + struct drm_i915_gem_request **req_out) { int ret; struct drm_i915_gem_request *request; struct drm_i915_private *dev_private = ring->dev->dev_private; - if (ring->outstanding_lazy_request) + if (!req_out) + return -EINVAL; + + if ((*req_out = ring->outstanding_lazy_request) != NULL) return 0; request = kzalloc(sizeof(*request), GFP_KERNEL); @@ -2188,7 +2193,7 @@ intel_ring_alloc_request(struct intel_engine_cs *ring, struct intel_context *ctx return ret; } - ring->outstanding_lazy_request = request; + *req_out = ring->outstanding_lazy_request = request; return 0; } @@ -2216,6 +2221,7 @@ static int __intel_ring_prepare(struct intel_engine_cs *ring, int intel_ring_begin(struct intel_engine_cs *ring, int num_dwords) { + struct drm_i915_gem_request *req; struct drm_i915_private *dev_priv = ring->dev->dev_private; int ret; @@ -2229,7 +2235,7 @@ int intel_ring_begin(struct intel_engine_cs *ring, return ret; /* Preallocate the olr before touching the ring */ - ret = intel_ring_alloc_request(ring, NULL); + ret = intel_ring_alloc_request(ring, NULL, &req); if (ret) return ret; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 2fd960a..4f8a14a 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -393,7 +393,8 @@ void intel_cleanup_ring_buffer(struct intel_engine_cs *ring); int __must_check intel_ring_begin(struct intel_engine_cs *ring, int n); int __must_check intel_ring_cacheline_align(struct intel_engine_cs *ring); int __must_check intel_ring_alloc_request(struct intel_engine_cs *ring, - struct intel_context *ctx); + struct intel_context *ctx, + struct drm_i915_gem_request **req_out); static inline void intel_ring_emit(struct intel_engine_cs *ring, u32 data) {