From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michel =?ISO-8859-1?Q?D=E4nzer?= Subject: Re: [PATCH 1/4] drm/radeon: fix and simplify pot argument checks Date: Tue, 23 Oct 2012 14:44:01 +0200 Message-ID: <1350996241.11117.19.camel@thor.local> References: <1350995014-4392-1-git-send-email-deathsimple@vodafone.de> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail.gna.ch (darkcity.gna.ch [195.226.6.51]) by gabe.freedesktop.org (Postfix) with ESMTP id C5B0D9E82D for ; Tue, 23 Oct 2012 05:44:14 -0700 (PDT) In-Reply-To: <1350995014-4392-1-git-send-email-deathsimple@vodafone.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Christian =?ISO-8859-1?Q?K=F6nig?= Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Die, 2012-10-23 at 14:23 +0200, Christian K=F6nig wrote: = > GART and VRAM size limits need to be a power of two. > Fix values greater than 1GB and simplify those checks a bit. > = > Signed-off-by: Christian K=F6nig > --- > drivers/gpu/drm/radeon/radeon_device.c | 55 ++++++++++++--------------= ------ > 1 file changed, 21 insertions(+), 34 deletions(-) > = > diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/rad= eon/radeon_device.c > index bd13ca0..3277aa1 100644 > --- a/drivers/gpu/drm/radeon/radeon_device.c > +++ b/drivers/gpu/drm/radeon/radeon_device.c > @@ -835,6 +835,19 @@ static unsigned int radeon_vga_set_decode(void *cook= ie, bool state) > } > = > /** > + * radeon_check_pot_argument - check that argument is a power of two > + * > + * @arg: value to check > + * > + * Validates that a certain argument is a power of two (all asics). > + * Returns true if argument is valid. > + */ > +static bool radeon_ckeck_pot_argument(int arg) > +{ > + return (arg & ((1 << __fls(arg)) - 1)) =3D=3D 0; > +} This could be simplified as return (arg & (arg - 1)) =3D=3D 0; > - radeon_vram_limit =3D radeon_vram_limit << 20; > + radeon_vram_limit =3D (uint64_t)radeon_vram_limit << 20; Does this cast have any effect without also changing the type of radeon_vram_limit to something other than int? If the point is to allow the shift to set the sign bit, I think casting to unsigned int or uint32_t would be slightly less confusing, but a comment is probably warranted anyway to prevent this from getting broken accidentally. The commit message of patch 2 has a typo (vzmalloc instead of vzalloc), other than that patches 2-4 are Reviewed-by: Michel D=E4nzer -- = Earthling Michel D=E4nzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer