From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Hellstrom Subject: Re: [PATCH] drm/radeon/kms: forbid big bo allocation (fdo 31708) v2 Date: Mon, 29 Nov 2010 11:13:09 +0100 Message-ID: <4CF37CB5.1050209@shipmail.org> References: <1290202485-5410-1-git-send-email-jglisse@redhat.com> <1290444967.4130.198.camel@thor.local> <4CEAA8F7.4000201@shipmail.org> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from smtp-outbound-2.vmware.com (smtp-outbound-2.vmware.com [65.115.85.73]) by gabe.freedesktop.org (Postfix) with ESMTP id 4DF259E748 for ; Mon, 29 Nov 2010 02:13:32 -0800 (PST) In-Reply-To: <4CEAA8F7.4000201@shipmail.org> 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: Jerome Glisse Cc: =?ISO-8859-1?Q?Michel_D=E4nzer?= , jglisse@redhat.com, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On 11/22/2010 06:31 PM, Thomas Hellstrom wrote: > On 11/22/2010 06:07 PM, Jerome Glisse wrote: >> 2010/11/22 Michel D=E4nzer: >>> On Fre, 2010-11-19 at 16:34 -0500, jglisse@redhat.com wrote: >>>> From: Jerome Glisse >>>> >>>> Forbid allocating buffer bigger than visible VRAM or GTT, also >>>> properly set lpfn field. >>>> >>>> v2 - use max macro >>>> - silence warning >>>> >>>> Signed-off-by: Jerome Glisse >>>> --- >>>> drivers/gpu/drm/radeon/radeon_object.c | 34 = >>>> ++++++++++++++++++++++++++----- >>>> 1 files changed, 28 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/radeon/radeon_object.c = >>>> b/drivers/gpu/drm/radeon/radeon_object.c >>>> index 1d06774..c2fa64c 100644 >>>> --- a/drivers/gpu/drm/radeon/radeon_object.c >>>> +++ b/drivers/gpu/drm/radeon/radeon_object.c >>>> @@ -69,18 +69,28 @@ void radeon_ttm_placement_from_domain(struct = >>>> radeon_bo *rbo, u32 domain) >>>> u32 c =3D 0; >>>> >>>> rbo->placement.fpfn =3D 0; >>>> - rbo->placement.lpfn =3D rbo->rdev->mc.active_vram_size>> = >>>> PAGE_SHIFT; >>>> + rbo->placement.lpfn =3D 0; >>>> rbo->placement.placement =3D rbo->placements; >>>> rbo->placement.busy_placement =3D rbo->placements; >>>> - if (domain& RADEON_GEM_DOMAIN_VRAM) >>>> + if (domain& RADEON_GEM_DOMAIN_VRAM) { >>>> + rbo->placement.lpfn =3D = >>>> max((unsigned)rbo->placement.lpfn, = >>>> (unsigned)rbo->rdev->mc.active_vram_size>> PAGE_SHIFT); >>>> rbo->placements[c++] =3D TTM_PL_FLAG_WC | = >>>> TTM_PL_FLAG_UNCACHED | >>>> TTM_PL_FLAG_VRAM; >>>> - if (domain& RADEON_GEM_DOMAIN_GTT) >>>> + } >>>> + if (domain& RADEON_GEM_DOMAIN_GTT) { >>>> + rbo->placement.lpfn =3D = >>>> max((unsigned)rbo->placement.lpfn, = >>>> (unsigned)rbo->rdev->mc.gtt_size>> PAGE_SHIFT); >>>> rbo->placements[c++] =3D TTM_PL_MASK_CACHING | = >>>> TTM_PL_FLAG_TT; >>>> - if (domain& RADEON_GEM_DOMAIN_CPU) >>>> + } >>>> + if (domain& RADEON_GEM_DOMAIN_CPU) { >>>> + /* 4G limit for CPU domain */ >>>> + rbo->placement.lpfn =3D max(rbo->placement.lpfn, = >>>> 0xFFFFFFFF>> PAGE_SHIFT); >>>> rbo->placements[c++] =3D TTM_PL_MASK_CACHING | = >>>> TTM_PL_FLAG_SYSTEM; >>>> - if (!c) >>>> + } >>>> + if (!c) { >>>> + /* 4G limit for CPU domain */ >>>> + rbo->placement.lpfn =3D max(rbo->placement.lpfn, = >>>> 0xFFFFFFFF>> PAGE_SHIFT); >>>> rbo->placements[c++] =3D TTM_PL_MASK_CACHING | = >>>> TTM_PL_FLAG_SYSTEM; >>>> + } >>> I don't think taxing the maximum is the right thing to do: If domain is >>> (RADEON_GEM_DOMAIN_VRAM | RADEON_GEM_DOMAIN_GTT) and VRAM doesn't = >>> happen >>> to be the same size as GTT, lpfn will end up larger than one of them. >>> >>> AFAICT radeon_ttm_placement_from_domain() should just set lpfn to 0 >>> (i.e. unrestricted), the callers that need it to be non-0 already = >>> set it >>> afterwards. >>> >>> Out of curiosity, where does the 4G limit come from? >>> >>> >> > From my hat, but iirc ttm limit things to 1G anyway (vm size for >> mapping object in drm file and iirc we will report error if we can't >> find a mapping for userspace object). I guess at one point we should >> start thinking about what we want to do on that front. > > That limit is taken from *my* hat. Nothing prevents us to increase = > that limit to whatever. > > /Thomas > > Hmm. Did you guys ever run into trouble with this? Looking at the code, it = seems the current limit is not 1GB but 1TB? (0x10000000 << PAGE_SHIFT) /Thomas > >> Doing a v3. >> >> Cheers, >> Jerome >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel