| Message ID | 1354115148-27766-1-git-send-email-j.glisse@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 28, 2012 at 10:05 AM, <j.glisse@gmail.com> wrote: > From: Jerome Glisse <jglisse@redhat.com> > > It make no sense to preserve caching state especialy when > moving from vram to system. It burden the page allocator to > match the vram caching (often WC) which just burn CPU cycle > for no good reasons. > > Signed-off-by: Jerome Glisse <jglisse@redhat.com> > --- > drivers/gpu/drm/ttm/ttm_bo.c | 15 +++------------ > 1 file changed, 3 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index bf6e4b5..39dcc58 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -896,19 +896,12 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo, > } > > static uint32_t ttm_bo_select_caching(struct ttm_mem_type_manager *man, > - uint32_t cur_placement, > uint32_t proposed_placement) > { > uint32_t caching = proposed_placement & TTM_PL_MASK_CACHING; > uint32_t result = proposed_placement & ~TTM_PL_MASK_CACHING; > > - /** > - * Keep current caching if possible. > - */ > - > - if ((cur_placement & caching) != 0) > - result |= (cur_placement & caching); > - else if ((man->default_caching & caching) != 0) > + if ((man->default_caching & caching) != 0) > result |= man->default_caching; > else if ((TTM_PL_FLAG_CACHED & caching) != 0) > result |= TTM_PL_FLAG_CACHED; > @@ -978,8 +971,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, > if (!type_ok) > continue; > > - cur_flags = ttm_bo_select_caching(man, bo->mem.placement, > - cur_flags); > + cur_flags = ttm_bo_select_caching(man, cur_flags); > /* > * Use the access and other non-mapping-related flag bits from > * the memory placement flags to the current flags > @@ -1023,8 +1015,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, > &cur_flags)) > continue; > > - cur_flags = ttm_bo_select_caching(man, bo->mem.placement, > - cur_flags); > + cur_flags = ttm_bo_select_caching(man, cur_flags); > /* > * Use the access and other non-mapping-related flag bits from > * the memory placement flags to the current flags > -- > 1.7.11.7 > Note this is mostly to mitigate performance decrease when a lot of eviction happen. It increase perf by as much of 60% on my computer. Cheers, Jerome
On 11/28/2012 09:09 PM, Jerome Glisse wrote: > On Wed, Nov 28, 2012 at 10:05 AM, <j.glisse@gmail.com> wrote: >> From: Jerome Glisse <jglisse@redhat.com> >> >> It make no sense to preserve caching state especialy when >> moving from vram to system. It burden the page allocator to >> match the vram caching (often WC) which just burn CPU cycle >> for no good reasons. Nack. This is a driver problem. What happens with this patch if you evict write-combined TT memory to system? That's why we want to preserve caching state in the first place. If you need a different behavior, you can fine-tune in driver::evict_flags, or in the radeon-case in radeon_move_ram_vram / radeon_move_vram_ram. /Thomas
On Wed, Nov 28, 2012 at 5:18 PM, Thomas Hellstrom <thomas@shipmail.org> wrote: > On 11/28/2012 09:09 PM, Jerome Glisse wrote: >> >> On Wed, Nov 28, 2012 at 10:05 AM, <j.glisse@gmail.com> wrote: >>> >>> From: Jerome Glisse <jglisse@redhat.com> >>> >>> It make no sense to preserve caching state especialy when >>> moving from vram to system. It burden the page allocator to >>> match the vram caching (often WC) which just burn CPU cycle >>> for no good reasons. > > > Nack. > This is a driver problem. > > What happens with this patch if you evict write-combined TT memory to > system? > That's why we want to preserve caching state in the first place. > > If you need a different behavior, you can fine-tune in driver::evict_flags, > or in the > radeon-case in radeon_move_ram_vram / radeon_move_vram_ram. No you can't. Because cur placement of vram will be wc, and thus it will want to preserve it for gtt. Only way to force it is to declare vram as cached too. Cheers, Jerome
On Wed, Nov 28, 2012 at 6:13 PM, Jerome Glisse <j.glisse@gmail.com> wrote: > On Wed, Nov 28, 2012 at 5:18 PM, Thomas Hellstrom <thomas@shipmail.org> wrote: >> On 11/28/2012 09:09 PM, Jerome Glisse wrote: >>> >>> On Wed, Nov 28, 2012 at 10:05 AM, <j.glisse@gmail.com> wrote: >>>> >>>> From: Jerome Glisse <jglisse@redhat.com> >>>> >>>> It make no sense to preserve caching state especialy when >>>> moving from vram to system. It burden the page allocator to >>>> match the vram caching (often WC) which just burn CPU cycle >>>> for no good reasons. >> >> >> Nack. >> This is a driver problem. >> >> What happens with this patch if you evict write-combined TT memory to >> system? >> That's why we want to preserve caching state in the first place. >> >> If you need a different behavior, you can fine-tune in driver::evict_flags, >> or in the >> radeon-case in radeon_move_ram_vram / radeon_move_vram_ram. > > No you can't. Because cur placement of vram will be wc, and thus it > will want to preserve it for gtt. Only way to force it is to declare > vram as cached too. > > Cheers, > Jerome Now that i think a bit more to it, in all fairness you can do so from the driver side. Cheers, Jerome
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index bf6e4b5..39dcc58 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -896,19 +896,12 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo, } static uint32_t ttm_bo_select_caching(struct ttm_mem_type_manager *man, - uint32_t cur_placement, uint32_t proposed_placement) { uint32_t caching = proposed_placement & TTM_PL_MASK_CACHING; uint32_t result = proposed_placement & ~TTM_PL_MASK_CACHING; - /** - * Keep current caching if possible. - */ - - if ((cur_placement & caching) != 0) - result |= (cur_placement & caching); - else if ((man->default_caching & caching) != 0) + if ((man->default_caching & caching) != 0) result |= man->default_caching; else if ((TTM_PL_FLAG_CACHED & caching) != 0) result |= TTM_PL_FLAG_CACHED; @@ -978,8 +971,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, if (!type_ok) continue; - cur_flags = ttm_bo_select_caching(man, bo->mem.placement, - cur_flags); + cur_flags = ttm_bo_select_caching(man, cur_flags); /* * Use the access and other non-mapping-related flag bits from * the memory placement flags to the current flags @@ -1023,8 +1015,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, &cur_flags)) continue; - cur_flags = ttm_bo_select_caching(man, bo->mem.placement, - cur_flags); + cur_flags = ttm_bo_select_caching(man, cur_flags); /* * Use the access and other non-mapping-related flag bits from * the memory placement flags to the current flags