| Message ID | 1425446318-29235-1-git-send-email-alexander.deucher@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi! IIRC, the interpretation of gpu_offset is fully controlled by the driver. That means to keep a native integer size you should be able to set radeon_gpu_offset := (ttm_gpu_offset << suitable_shift); However, if you feel that this is too intrusive in the driver, I'm not against the current patch as it stands. It's important, however that the interactions with drm_mm is sorted out. I'm not sure the patch that converted drm_mm to 64-bit instead of unsigned long went in. (There were similar arguments against that patch). Thanks, Thomas On 03/04/2015 06:18 AM, Alex Deucher wrote: > We need to store device offsets in 64 bit as the device > address space may be larger than the CPU's. > > Fixes GPU init failures on radeons with 4GB or more of > vram on 32 bit kernels. We put vram at the start of the > GPU's address space so the gart aperture starts at 4 GB > causing all GPU addresses in the gart aperture to get > truncated. > > bug: > https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D89072&d=AwIBAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=vpukPkBtpoNQp2IUKuFviOmPNYWVKmen3Jeeu55zmEA&m=M1rvbqZd6hlmRIsZmBXhSypX4cSQqZSddym13SRVKtg&s=sriG7bnxL46LbaYmq_FnswHY9WOrPvTbdK7um05zoxE&e= > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > Cc: thellstrom@vmware.com > --- > Untested. Someone more familiar with the ttm internals can > probably point out any additional bits I missed. Heading to > bed now. > > drivers/gpu/drm/ttm/ttm_bo.c | 2 +- > include/drm/ttm/ttm_bo_api.h | 2 +- > include/drm/ttm/ttm_bo_driver.h | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index d395b0b..8d9b7de 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -74,7 +74,7 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, int mem_type) > pr_err(" has_type: %d\n", man->has_type); > pr_err(" use_type: %d\n", man->use_type); > pr_err(" flags: 0x%08X\n", man->flags); > - pr_err(" gpu_offset: 0x%08lX\n", man->gpu_offset); > + pr_err(" gpu_offset: 0x%08llX\n", man->gpu_offset); > pr_err(" size: %llu\n", man->size); > pr_err(" available_caching: 0x%08X\n", man->available_caching); > pr_err(" default_caching: 0x%08X\n", man->default_caching); > diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h > index 0ccf7f2..c768ddf 100644 > --- a/include/drm/ttm/ttm_bo_api.h > +++ b/include/drm/ttm/ttm_bo_api.h > @@ -249,7 +249,7 @@ struct ttm_buffer_object { > * either of these locks held. > */ > > - unsigned long offset; > + uint64_t offset; /* GPU address space is independent of CPU word size */ > uint32_t cur_placement; > > struct sg_table *sg; > diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h > index 142d752..813042c 100644 > --- a/include/drm/ttm/ttm_bo_driver.h > +++ b/include/drm/ttm/ttm_bo_driver.h > @@ -277,7 +277,7 @@ struct ttm_mem_type_manager { > bool has_type; > bool use_type; > uint32_t flags; > - unsigned long gpu_offset; > + uint64_t gpu_offset; /* GPU address space is independent of CPU word size */ > uint64_t size; > uint32_t available_caching; > uint32_t default_caching;
Even shifting doesn't help here. We have up to 48bits of physical address space on modern systems with still 4K page size. This means 48-12=36bit address space, that won't fit into a 32bit integer no matter what. Assuming we have already double checked drm_mm the patch is Reviewed-by: Christian König <christian.koenig@amd.com> Regards, Christian. On 04.03.2015 09:01, Thomas Hellstrom wrote: > Hi! > > IIRC, the interpretation of gpu_offset is fully controlled by the > driver. That means to keep a native integer size > you should be able to set > > radeon_gpu_offset := (ttm_gpu_offset << suitable_shift); > > However, if you feel that this is too intrusive in the driver, I'm not > against the current patch as it stands. It's important, however that the > interactions with drm_mm is sorted out. I'm not sure the patch that > converted drm_mm to 64-bit instead > of unsigned long went in. (There were similar arguments against that patch). > > Thanks, > Thomas > > > On 03/04/2015 06:18 AM, Alex Deucher wrote: >> We need to store device offsets in 64 bit as the device >> address space may be larger than the CPU's. >> >> Fixes GPU init failures on radeons with 4GB or more of >> vram on 32 bit kernels. We put vram at the start of the >> GPU's address space so the gart aperture starts at 4 GB >> causing all GPU addresses in the gart aperture to get >> truncated. >> >> bug: >> https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D89072&d=AwIBAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=vpukPkBtpoNQp2IUKuFviOmPNYWVKmen3Jeeu55zmEA&m=M1rvbqZd6hlmRIsZmBXhSypX4cSQqZSddym13SRVKtg&s=sriG7bnxL46LbaYmq_FnswHY9WOrPvTbdK7um05zoxE&e= >> >> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> >> Cc: thellstrom@vmware.com >> --- >> Untested. Someone more familiar with the ttm internals can >> probably point out any additional bits I missed. Heading to >> bed now. >> >> drivers/gpu/drm/ttm/ttm_bo.c | 2 +- >> include/drm/ttm/ttm_bo_api.h | 2 +- >> include/drm/ttm/ttm_bo_driver.h | 2 +- >> 3 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c >> index d395b0b..8d9b7de 100644 >> --- a/drivers/gpu/drm/ttm/ttm_bo.c >> +++ b/drivers/gpu/drm/ttm/ttm_bo.c >> @@ -74,7 +74,7 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, int mem_type) >> pr_err(" has_type: %d\n", man->has_type); >> pr_err(" use_type: %d\n", man->use_type); >> pr_err(" flags: 0x%08X\n", man->flags); >> - pr_err(" gpu_offset: 0x%08lX\n", man->gpu_offset); >> + pr_err(" gpu_offset: 0x%08llX\n", man->gpu_offset); >> pr_err(" size: %llu\n", man->size); >> pr_err(" available_caching: 0x%08X\n", man->available_caching); >> pr_err(" default_caching: 0x%08X\n", man->default_caching); >> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h >> index 0ccf7f2..c768ddf 100644 >> --- a/include/drm/ttm/ttm_bo_api.h >> +++ b/include/drm/ttm/ttm_bo_api.h >> @@ -249,7 +249,7 @@ struct ttm_buffer_object { >> * either of these locks held. >> */ >> >> - unsigned long offset; >> + uint64_t offset; /* GPU address space is independent of CPU word size */ >> uint32_t cur_placement; >> >> struct sg_table *sg; >> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h >> index 142d752..813042c 100644 >> --- a/include/drm/ttm/ttm_bo_driver.h >> +++ b/include/drm/ttm/ttm_bo_driver.h >> @@ -277,7 +277,7 @@ struct ttm_mem_type_manager { >> bool has_type; >> bool use_type; >> uint32_t flags; >> - unsigned long gpu_offset; >> + uint64_t gpu_offset; /* GPU address space is independent of CPU word size */ >> uint64_t size; >> uint32_t available_caching; >> uint32_t default_caching; > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
OK. Understood. Also, conditioned on drm_mm compatibility (which is really a driver issue), Acked-by: Thomas Hellstrom <thellstrom@vmware.com> On 03/04/2015 10:33 AM, Christian König wrote: > Even shifting doesn't help here. We have up to 48bits of physical > address space on modern systems with still 4K page size. > > This means 48-12=36bit address space, that won't fit into a 32bit > integer no matter what. > > Assuming we have already double checked drm_mm the patch is > Reviewed-by: Christian König <christian.koenig@amd.com> > > Regards, > Christian. > > On 04.03.2015 09:01, Thomas Hellstrom wrote: >> Hi! >> >> IIRC, the interpretation of gpu_offset is fully controlled by the >> driver. That means to keep a native integer size >> you should be able to set >> >> radeon_gpu_offset := (ttm_gpu_offset << suitable_shift); >> >> However, if you feel that this is too intrusive in the driver, I'm not >> against the current patch as it stands. It's important, however that the >> interactions with drm_mm is sorted out. I'm not sure the patch that >> converted drm_mm to 64-bit instead >> of unsigned long went in. (There were similar arguments against that >> patch). >> >> Thanks, >> Thomas >> >> >> On 03/04/2015 06:18 AM, Alex Deucher wrote: >>> We need to store device offsets in 64 bit as the device >>> address space may be larger than the CPU's. >>> >>> Fixes GPU init failures on radeons with 4GB or more of >>> vram on 32 bit kernels. We put vram at the start of the >>> GPU's address space so the gart aperture starts at 4 GB >>> causing all GPU addresses in the gart aperture to get >>> truncated. >>> >>> bug: >>> https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D89072&d=AwIBAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=vpukPkBtpoNQp2IUKuFviOmPNYWVKmen3Jeeu55zmEA&m=M1rvbqZd6hlmRIsZmBXhSypX4cSQqZSddym13SRVKtg&s=sriG7bnxL46LbaYmq_FnswHY9WOrPvTbdK7um05zoxE&e= >>> >>> >>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> >>> Cc: thellstrom@vmware.com >>> --- >>> Untested. Someone more familiar with the ttm internals can >>> probably point out any additional bits I missed. Heading to >>> bed now. >>> >>> drivers/gpu/drm/ttm/ttm_bo.c | 2 +- >>> include/drm/ttm/ttm_bo_api.h | 2 +- >>> include/drm/ttm/ttm_bo_driver.h | 2 +- >>> 3 files changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c >>> b/drivers/gpu/drm/ttm/ttm_bo.c >>> index d395b0b..8d9b7de 100644 >>> --- a/drivers/gpu/drm/ttm/ttm_bo.c >>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c >>> @@ -74,7 +74,7 @@ static void ttm_mem_type_debug(struct >>> ttm_bo_device *bdev, int mem_type) >>> pr_err(" has_type: %d\n", man->has_type); >>> pr_err(" use_type: %d\n", man->use_type); >>> pr_err(" flags: 0x%08X\n", man->flags); >>> - pr_err(" gpu_offset: 0x%08lX\n", man->gpu_offset); >>> + pr_err(" gpu_offset: 0x%08llX\n", man->gpu_offset); >>> pr_err(" size: %llu\n", man->size); >>> pr_err(" available_caching: 0x%08X\n", >>> man->available_caching); >>> pr_err(" default_caching: 0x%08X\n", man->default_caching); >>> diff --git a/include/drm/ttm/ttm_bo_api.h >>> b/include/drm/ttm/ttm_bo_api.h >>> index 0ccf7f2..c768ddf 100644 >>> --- a/include/drm/ttm/ttm_bo_api.h >>> +++ b/include/drm/ttm/ttm_bo_api.h >>> @@ -249,7 +249,7 @@ struct ttm_buffer_object { >>> * either of these locks held. >>> */ >>> - unsigned long offset; >>> + uint64_t offset; /* GPU address space is independent of CPU >>> word size */ >>> uint32_t cur_placement; >>> struct sg_table *sg; >>> diff --git a/include/drm/ttm/ttm_bo_driver.h >>> b/include/drm/ttm/ttm_bo_driver.h >>> index 142d752..813042c 100644 >>> --- a/include/drm/ttm/ttm_bo_driver.h >>> +++ b/include/drm/ttm/ttm_bo_driver.h >>> @@ -277,7 +277,7 @@ struct ttm_mem_type_manager { >>> bool has_type; >>> bool use_type; >>> uint32_t flags; >>> - unsigned long gpu_offset; >>> + uint64_t gpu_offset; /* GPU address space is independent of CPU >>> word size */ >>> uint64_t size; >>> uint32_t available_caching; >>> uint32_t default_caching; >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel&d=AwIDaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=vpukPkBtpoNQp2IUKuFviOmPNYWVKmen3Jeeu55zmEA&m=OZUnRVpjH7RJ-rhjZ9RYGh_B9euuww9Hi2CoMW17QZY&s=mxUmoWCnh6WYNIPj7pCHCoCNuaAE-_rZAGKJpVd1r3Q&e= > >
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index d395b0b..8d9b7de 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -74,7 +74,7 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, int mem_type) pr_err(" has_type: %d\n", man->has_type); pr_err(" use_type: %d\n", man->use_type); pr_err(" flags: 0x%08X\n", man->flags); - pr_err(" gpu_offset: 0x%08lX\n", man->gpu_offset); + pr_err(" gpu_offset: 0x%08llX\n", man->gpu_offset); pr_err(" size: %llu\n", man->size); pr_err(" available_caching: 0x%08X\n", man->available_caching); pr_err(" default_caching: 0x%08X\n", man->default_caching); diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 0ccf7f2..c768ddf 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -249,7 +249,7 @@ struct ttm_buffer_object { * either of these locks held. */ - unsigned long offset; + uint64_t offset; /* GPU address space is independent of CPU word size */ uint32_t cur_placement; struct sg_table *sg; diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index 142d752..813042c 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -277,7 +277,7 @@ struct ttm_mem_type_manager { bool has_type; bool use_type; uint32_t flags; - unsigned long gpu_offset; + uint64_t gpu_offset; /* GPU address space is independent of CPU word size */ uint64_t size; uint32_t available_caching; uint32_t default_caching;
We need to store device offsets in 64 bit as the device address space may be larger than the CPU's. Fixes GPU init failures on radeons with 4GB or more of vram on 32 bit kernels. We put vram at the start of the GPU's address space so the gart aperture starts at 4 GB causing all GPU addresses in the gart aperture to get truncated. bug: https://bugs.freedesktop.org/show_bug.cgi?id=89072 Signed-off-by: Alex Deucher <alexander.deucher@amd.com> Cc: thellstrom@vmware.com --- Untested. Someone more familiar with the ttm internals can probably point out any additional bits I missed. Heading to bed now. drivers/gpu/drm/ttm/ttm_bo.c | 2 +- include/drm/ttm/ttm_bo_api.h | 2 +- include/drm/ttm/ttm_bo_driver.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)