[Intel-gfx] [PATCH] Use ALIGN macro instead of open coding it.

Matt Turner mattst88 at gmail.com
Thu Mar 18 17:56:13 CET 2010


On Thu, Mar 18, 2010 at 12:50 PM, Carl Worth <cworth at cworth.org> wrote:
> On Fri, 12 Mar 2010 00:30:23 -0500, Matt Turner <mattst88 at gmail.com> wrote:
>> Also, kill off ROUND_TO, ROUND_DOWN, ROUND_TO_PAGE, and ROUND_TO_MB
>> macros. ROUND_{TO,DOWN} are only useful if rounding to a non-power of
>> two, which they always were, and ROUND_TO_PAGE and ROUND_TO_MB
>> inherently round to a power of two, so just use ALIGN.
>
> I see you've followed this patch up with an improved version in a later
> thread.
>
> For me, I would prefer if updates to a patch would be sent as replies to
> the original patch. That helps me avoid merging the old patch without
> realizing it, (while processing my patch queue in chronological order).

Sure, I'll do that in the future.

> Thanks!
>
> -Carl
>
> PS. Your commit message includes:
>
>> Also, kill off i830_pad_drawable_width.
>
> That word "also" is a red flag for me. It suggests an independent
> change, and independent changes should almost always be in separate
> commits. I don't know how many times I've bisected a bug down to an
> excessively large commit with the word "also" in it.

I think if you check out the patch, you won't see anything wrong with
removing that function. It was just an ALIGN(..., 64) so it seemed
sensible to get rid of it by replacing calls to it with ALIGN(..., 64)
given the purpose of the patch.

I understand what you're saying, but in this case I don't think there
would be any benefit of splitting it into a separate patch.

Thanks,
Matt



More information about the Intel-gfx mailing list