[Beignet] clEnqueueNDRangeKernel and kernel completion

[Zhigang Gong]

I thought that all intel platform may keep consistent at DRM layer.
Anyway, I'm ok with moving it to the driver side. Back to the patch
itself.

I have one comment as below,

 LOCAL void
 intel_batchbuffer_reset(intel_batchbuffer_t *batch, size_t sz)
 {
+  if (batch->last_bo != NULL) {
+       drm_intel_bo_unreference(batch->last_bo);
+       batch->last_bo = NULL;
+  }
+  batch->last_bo = batch->buffer;
As the patch will unreference last_bo, we may not just set batch->buffer
to batch->last_bo. Otherwise, the reference counter of the bo will not
be balanced. We may need to add one line here, Right?
   dri_bo_unreference(batch->last_bo);
Without the above fix, this patch will break the unit test case.

Or we can just ignore batch->last_bo and don't unreference it.

Which method do you prefer?
 
+


+
+  drm_intel_bo_unreference(batch->last_bo);
  maybe use dri_bo_unreference is better?  Right?
   cl_free(batch);
 }




On Thu, Jun 13, 2013 at 02:04:52AM +0000, Zou, Nanhai wrote:
> >>Hi Nanhai,
> 
> >>I have a quick look at this patch. And have one question here, the original bug should be that it forgets to set the queue's last batch's value, so it always do nothing when call clFinish().
> >>So IMHO, the easiest fix is to add code to set the queue's last_batch maybe.
> 
> >>Your fix seems to not only fix that bug, but also move the whole things from hardware independent layer to the intel driver layer. Right? Is there any specific reason to do so
> 
> Batch buffer handle and waiting code etc. is hardware specific.
> Put batch buffer code in frontend looks not reasonable for me, the hardware sync logic may change between platforms.
> 
> Thanks
> Zou Nanhai
> 
> 
> 
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet