On Wed, Nov 25, 2015 at 09:51:08PM +0200, Ville Syrjälä wrote:
> On Fri, Nov 20, 2015 at 03:31:23PM +0000, Chris Wilson wrote:
> > @@ -1097,6 +1003,7 @@ static bool check_cmd(const struct intel_engine_cs *ring,
> > }
> >
> > #define LENGTH_BIAS 2
> > +#define MAX_PARTIAL 256
>
> There seems to some confusion whether this is bytes or dwords.
Indeed, I can't remember of the top of my head.
(Double checked that the set of commands that I was thinking were 132
bytes.)
> Also I guess we already end up allocating two pages anyway, so
> maybe MAX_PARTIAL should just be one page? It's still not big
> enough to cover the max legal cmd length AFAICS, so I think
> the WARN in the check needs to be removed.
Sure, rounding up the next 8192 byte slab cache doesn't seem like it
will bite us.
So #define MAX_PARTIAL_BYTES PAGE_SIZE
> > + in = offset_in_page(batch_start_offset);
> > + partial = 0;
> > + for (src_iter = batch_start_offset >> PAGE_SHIFT;
> > + src_iter < batch_obj->base.size >> PAGE_SHIFT;
> > + src_iter++) {
>
> So we're iterating over all the pages. Should be enough to iterate
> until batch_start_offset+batch_len I suppose, but as long as we bail
> out when we run out of batch it should be fine.
Right, this was mostly convenience for writing the loop bounds - it was
more or less a simple conversion from the old iterator.
> I see there's a batch_len check at the end, but I don't see us handling
> the case when the user already gives us something with batch_len==0.
> Maybe that should be rejected somewhere higher up?
batch_len = 0 is filtered out in the caller...
> Also what happens if we don't find MI_BATCH_BUFFER_END before running
> out of batch? Oh, I see, we set ret=-EINVAL, and clear it to 0 when we
> find MI_BATCH_BUFFER_END. So that part seems to be fine.
>
> > + u32 *cmd, *page_end, *batch_end;
> > + u32 this;
> > +
> > + this = batch_len;
>
> I was a bit concerned about batch_len & 3, but we already check for
> batch_len&7==0 in i915_gem_check_execbuffer(), so it should be good here.
cmdparser_assert(batch_len > 0 && (batch_len & 3) == 0);
as documentation for the contract?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
