[Intel-gfx] [PATCH 12/53] drm/i915/bdw: Populate LR contexts (somewhat)

[Mateo Lozano, Oscar]

> -----Original Message-----
> From: Volkin, Bradley D
> Sent: Monday, June 23, 2014 4:06 PM
> To: Mateo Lozano, Oscar
> Cc: intel-gfx at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 12/53] drm/i915/bdw: Populate LR contexts
> (somewhat)
> 
> On Mon, Jun 23, 2014 at 05:42:50AM -0700, Mateo Lozano, Oscar wrote:
> > > -----Original Message-----
> > > From: Volkin, Bradley D
> > > > +	reg_state[CTX_RING_HEAD+1] = 0;
> > > > +	reg_state[CTX_RING_TAIL] = RING_TAIL(ring->mmio_base);
> > > > +	reg_state[CTX_RING_TAIL+1] = 0;
> > > > +	reg_state[CTX_RING_BUFFER_START] = RING_START(ring-
> > > >mmio_base);
> > > > +	reg_state[CTX_RING_BUFFER_START+1] =
> > > i915_gem_obj_ggtt_offset(ring_obj);
> > > > +	reg_state[CTX_RING_BUFFER_CONTROL] = RING_CTL(ring-
> > > >mmio_base);
> > > > +	reg_state[CTX_RING_BUFFER_CONTROL+1] = (31 * PAGE_SIZE) |
> > > > +RING_VALID;
> > >
> > > The size here doesn't look right to me. Shouldn't it be (number of pages -
> 1)?
> > > See init_ring_common()
> >
> > But that´s exactly what it is, right?
> >
> > Our ringbuf->size = 32 * PAGE_SIZE;
> > so we are setting 31 * PAGE_SIZE
> 
> Ok, on closer inspection, the result is correct because the Buffer Length field
> happens to be bits 20:12. But it looked to me like a size-in-bytes rather than
> the encoding I expected. So I guess I'd prefer that we do it as in
> init_ring_common(), using ring_obj->base.size and the RING_NR_PAGES
> mask so that it's a bit more obvious what's going on.
> 
> Thanks,
> Brad

Yeah, it probably looks less magical that way. Ok, will do.