On Mon, Nov 29, 2010 at 5:13 AM, Thomas Hellstrom <thomas at shipmail.org>
wrote:
> 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?nzer<michel at daenzer.net>:
>>>>
>>>> On Fre, 2010-11-19 at 16:34 -0500, jglisse at redhat.com wrote:
>>>>>
>>>>> From: Jerome Glisse<jglisse at redhat.com>
>>>>>
>>>>> 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<jglisse at redhat.com>
>>>>> ---
>>>>> ?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 = 0;
>>>>>
>>>>> ? ? ? rbo->placement.fpfn = 0;
>>>>> - ? ? rbo->placement.lpfn = rbo->rdev->mc.active_vram_size>>
>>>>> ?PAGE_SHIFT;
>>>>> + ? ? rbo->placement.lpfn = 0;
>>>>> ? ? ? rbo->placement.placement = rbo->placements;
>>>>> ? ? ? rbo->placement.busy_placement = rbo->placements;
>>>>> - ? ? if (domain& ?RADEON_GEM_DOMAIN_VRAM)
>>>>> + ? ? if (domain& ?RADEON_GEM_DOMAIN_VRAM) {
>>>>> + ? ? ? ? ? ? rbo->placement.lpfn = max((unsigned)rbo->placement.lpfn,
>>>>> (unsigned)rbo->rdev->mc.active_vram_size>> ?PAGE_SHIFT);
>>>>> ? ? ? ? ? ? ? rbo->placements[c++] = 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 = max((unsigned)rbo->placement.lpfn,
>>>>> (unsigned)rbo->rdev->mc.gtt_size>> ?PAGE_SHIFT);
>>>>> ? ? ? ? ? ? ? rbo->placements[c++] = 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 = max(rbo->placement.lpfn,
>>>>> 0xFFFFFFFF>> ?PAGE_SHIFT);
>>>>> ? ? ? ? ? ? ? rbo->placements[c++] = TTM_PL_MASK_CACHING |
>>>>> TTM_PL_FLAG_SYSTEM;
>>>>> - ? ? if (!c)
>>>>> + ? ? }
>>>>> + ? ? if (!c) {
>>>>> + ? ? ? ? ? ? /* 4G limit for CPU domain */
>>>>> + ? ? ? ? ? ? rbo->placement.lpfn = max(rbo->placement.lpfn,
>>>>> 0xFFFFFFFF>> ?PAGE_SHIFT);
>>>>> ? ? ? ? ? ? ? rbo->placements[c++] = 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
>
No never run into it, i also wrongly parsed the limit or my memory of
it wasn't good.
Cheers,
Jerome
