Re: [PATCH v2 1/3] drm/i915: Check for matching ringbuffer in logical_ring_wait_request()

Deepak S
2014-11-24T04:17:52Z

On Wednesday 19 November 2014 01:37 AM, Dave Gordon wrote:
> The request queue is per-engine, and may therefore contain requests
> from several different contexts/ringbuffers. In determining which
> request to wait for, this function should only consider requests
> from the ringbuffer that it's checking for space, and ignore any
> that it finds that belong to other contexts.
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 2a1a719..1003b3a 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -835,6 +835,16 @@ static int logical_ring_wait_request(struct intel_ringbuffer *ringbuf,
> }
>
> list_for_each_entry(request, &ring->request_list, list) {
> + /*
> + * The request queue is per-engine, so can contain requests
> + * from multiple ringbuffers. Here, we must ignore any that
> + * aren't from the ringbuffer we're considering.
> + */
> + struct intel_context *ctx = request->ctx;
> + if (ctx->engine[ring->id].ringbuf != ringbuf)
> + continue;
> +
> + /* Would completion of this request free enough space? */
> if (__intel_ring_space(request->tail, ringbuf->tail,
> ringbuf->size) >= bytes) {
> seqno = request->seqno;

Looks fine to me
Reviewed-by: Deepak S <deepak.s@linux.intel.com>