Hi thanks for reviewing the patches. Am 06.05.19 um 14:31 schrieb Gerd Hoffmann: >> + --gbo->pin_count; >> + if (gbo->pin_count) >> + return 0; >> + >> + if (gbo->kmap.virtual) >> + ttm_bo_kunmap(&gbo->kmap); >> + >> + drm_gem_vram_placement(gbo, TTM_PL_FLAG_SYSTEM); >> + for (i = 0; i < gbo->placement.num_placement ; ++i) >> + gbo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT; >> + >> + ret = ttm_bo_validate(&gbo->bo, &gbo->placement, &ctx); >> + if (ret) >> + return ret; >> + >> + return 0; >> +} >> +EXPORT_SYMBOL(drm_gem_vram_push_to_system); > > Very simliar to drm_gem_vram_unpin, can't we just call that function? > > Something like this: > > drm_gem_vram_push_to_system() > { > if (gbo->pin_count == 1 && gbo->kmap.virtual) > ttm_bo_kunmap(&gbo->kmap); > return drm_gem_vram_unpin(); > } This misses the call to drm_gem_vram_placement(), where drm_gem_vram_push_to_system() enforces placement in system memory. We could build a common implementation out of both interfaces, but that would obfuscate the code IMHO. I'd just leave it as it is. The function is only used by ast and mgag200 framebuffer handling. It's actually something that should be done by VRAM MM, but both drivers are non-atomic and could require an overhaul anyway. >> +struct drm_gem_vram_object { >> + /* Supported placements are %TTM_PL_VRAM and %TTM_PL_SYSTEM */ >> + struct ttm_placement placement; >> + struct ttm_place placements[3]; > > placements[2] should be enough I guess? TTM_PL_VRAM has index 2 and TTM_PL_SYSTEM has index 0. There's TTM_PL_TT at index 1. We don't use all three array entries here, but I'm not sure if something in TTM does. I took the line from the drivers and didn't change it for that reason. Best regards Thomas > cheers, > Gerd > -- Thomas Zimmermann Graphics Driver Developer SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 21284 (AG Nürnberg)
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization