On Thu, Aug 28, 2014 at 9:46 PM, Michel Dänzer wrote: > On 29.08.2014 00:01, Alex Deucher wrote: >> On Thu, Aug 28, 2014 at 4:57 AM, Christian König >> wrote: >>> Am 28.08.2014 um 08:56 schrieb Michel Dänzer: >>> >>>> From: Michel Dänzer >>>> >>>> This flag is a hint that userspace expects the BO to be accessed by the >>>> CPU. We can use that hint to prevent such BOs from ever being stored in >>>> the CPU inaccessible part of VRAM. >>>> >>>> Signed-off-by: Michel Dänzer >>> >>> >>> This patch is Reviewed-by: Christian König >> >> Applied to my -next tree. > > Thanks! > > >>> I think we need a similar negative flags as well, e.g. >>> RADEON_GEM_NO_CPU_ACCESS. >>> >>> This way we can stop forcing buffers into the visible VRAM while pinning >>> them for scanout. >> >> How about the attached patch? > > [...] > >> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c >> index 09b039a..b71e8e0 100644 >> --- a/drivers/gpu/drm/radeon/radeon_object.c >> +++ b/drivers/gpu/drm/radeon/radeon_object.c >> @@ -314,10 +314,14 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, >> unsigned lpfn = 0; >> >> /* force to pin into visible video ram */ >> - if (bo->placements[i].flags & TTM_PL_FLAG_VRAM) >> - lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT; >> - else >> + if (bo->placements[i].flags & TTM_PL_FLAG_VRAM) { >> + if (bo->flags & RADEON_GEM_NO_CPU_ACCESS) >> + lpfn = bo->rdev->mc.real_vram_size >> PAGE_SHIFT; >> + else >> + lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT; > > lpfn can be left at 0 if RADEON_GEM_NO_CPU_ACCESS is set, so this can > be simplified to: > > if (!(bo->flags & RADEON_GEM_NO_CPU_ACCESS)) > lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT; > > >> diff --git a/include/uapi/drm/radeon_drm.h b/include/uapi/drm/radeon_drm.h >> index f755f20..d2346fd 100644 >> --- a/include/uapi/drm/radeon_drm.h >> +++ b/include/uapi/drm/radeon_drm.h >> @@ -803,6 +803,8 @@ struct drm_radeon_gem_info { >> #define RADEON_GEM_GTT_WC (1 << 2) >> /* BO is expected to be accessed by the CPU */ >> #define RADEON_GEM_CPU_ACCESS (1 << 3) >> +/* BO is expected to not be accessed by the CPU */ >> +#define RADEON_GEM_NO_CPU_ACCESS (1 << 4) > > I'd use stronger wording for this, e.g. > > /* CPU access is not expected to work for this BO */ Updated version with comments integrated. Alex