Re: [PATCH 1/2] drm/i915: Tidy workaround batch buffer emission

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Feb 15, 2017 at 04:06:33PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> 
> Use the "*batch++ = " style as in the ring emission for better
> readability and also simplify the logic a bit by consolidating
> the offset and size calculations and overflow checking. The
> latter is a programming error so it is not required to check
> for it after each write to the object, but rather do it once the
> whole state has been written and fail the driver if something
> went wrong.
> 
> v2: Rebase.

I did not spot any mistakes in the mechanical translations.

> +	/*
> +	 * Emit the two workaround batch buffers, recording the offset from the
> +	 * start of the workaround batch buffer object for each and their
> +	 * respective sizes.
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(wa_bb_f); i++) {
> +		wa_bb[i]->offset = ALIGN(batch_ptr - batch, CACHELINE_DWORDS);
> +		batch_ptr = wa_bb_f[i](engine, batch_ptr);

I'm just a bit more familar with using _fn for function pointers.

> +		wa_bb[i]->size = batch_ptr - &batch[wa_bb[i]->offset];

Surprised me that we store the offset/size in dwords not bytes. And then
convert to bytes to store in the registers.

>  	}
>  
> -out:
> +	BUG_ON(batch_ptr - batch > CTX_WA_BB_OBJ_SIZE / sizeof(u32));

Would it make sense going forward just use void *batch, batch_ptr here
and compute bytes?

Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux