>> +uint32_t amdgpu_framebuffer_domains(struct amdgpu_device *adev)
>
> Please rename this amdgpu_display_framebuffer_domains() for consistency.
Currently all the functions in this file are named without _display_. Am I missing something?
>> + if (plane->type != DRM_PLANE_TYPE_CURSOR)
>
> Do cursors have to be in vram? It seems like they shouldn't.
I checked some design documentation and related implementation just now. Looks like cursor is still supposed to be put in vram now.
Regards,
Sam
On 2018-01-04 04:18 PM, Alex Deucher wrote:
> On Thu, Jan 4, 2018 at 4:11 PM, Samuel Li <Samuel.Li at amd.com> wrote:
>> From: Christian König <christian.koenig at amd.com>
>>
>> On CZ and newer APUs we can pin the fb into GART as well as VRAM.
>>
>> v2: Don't enable gpu_vm_support for Raven yet since it leads to
>> a black screen. Need to debug this further before enabling.
>>
>> Change-Id: Id0f8af3110e54a3aabf7a258871867bc121cc1a2
>> Signed-off-by: Christian König <christian.koenig at amd.com>
>> Reviewed-by: Andrey Grodzovsky <andrey.grodzovsky at amd.com>
>> Acked-by: Alex Deucher <alexander.deucher at amd.com>
>> Signed-off-by: Samuel Li <samuel.li at amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 14 +++++++++++++-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_display.h | 1 +
>> drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 10 ++++++----
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 11 +++++++++--
>> 4 files changed, 29 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> index d704a45..d9fdc19 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> @@ -29,6 +29,7 @@
>> #include "amdgpu_i2c.h"
>> #include "atom.h"
>> #include "amdgpu_connectors.h"
>> +#include "amdgpu_display.h"
>> #include <asm/div64.h>
>>
>> #include <linux/pm_runtime.h>
>> @@ -188,7 +189,7 @@ int amdgpu_crtc_page_flip_target(struct drm_crtc *crtc,
>> goto cleanup;
>> }
>>
>> - r = amdgpu_bo_pin(new_abo, AMDGPU_GEM_DOMAIN_VRAM, &base);
>> + r = amdgpu_bo_pin(new_abo, amdgpu_framebuffer_domains(adev), &base);
>> if (unlikely(r != 0)) {
>> DRM_ERROR("failed to pin new abo buffer before flip\n");
>> goto unreserve;
>> @@ -501,6 +502,17 @@ static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
>> .create_handle = amdgpu_user_framebuffer_create_handle,
>> };
>>
>> +uint32_t amdgpu_framebuffer_domains(struct amdgpu_device *adev)
>
> Please rename this amdgpu_display_framebuffer_domains() for consistency.
>
>> +{
>> + uint32_t domain = AMDGPU_GEM_DOMAIN_VRAM;
>> +
>> + if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type < CHIP_RAVEN &&
>> + adev->flags & AMD_IS_APU)
>> + domain |= AMDGPU_GEM_DOMAIN_GTT;
>> +
>> + return domain;
>> +}
>> +
>> int
>> amdgpu_framebuffer_init(struct drm_device *dev,
>> struct amdgpu_framebuffer *rfb,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
>> index 11ae4ab..f241949 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
>> @@ -23,6 +23,7 @@
>> #ifndef __AMDGPU_DISPLAY_H__
>> #define __AMDGPU_DISPLAY_H__
>>
>> +uint32_t amdgpu_framebuffer_domains(struct amdgpu_device *adev);
>> struct drm_framebuffer *
>> amdgpu_user_framebuffer_create(struct drm_device *dev,
>> struct drm_file *file_priv,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>> index 90fa8e8..9be3228 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>> @@ -38,6 +38,8 @@
>>
>> #include <linux/vga_switcheroo.h>
>>
>> +#include "amdgpu_display.h"
>> +
>> /* object hierarchy -
>> this contains a helper + a amdgpu fb
>> the helper contains a pointer to amdgpu framebuffer baseclass.
>> @@ -124,7 +126,7 @@ static int amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
>> struct drm_gem_object *gobj = NULL;
>> struct amdgpu_bo *abo = NULL;
>> bool fb_tiled = false; /* useful for testing */
>> - u32 tiling_flags = 0;
>> + u32 tiling_flags = 0, domain;
>> int ret;
>> int aligned_size, size;
>> int height = mode_cmd->height;
>> @@ -135,12 +137,12 @@ static int amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
>> /* need to align pitch with crtc limits */
>> mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
>> fb_tiled);
>> + domain = amdgpu_framebuffer_domains(adev);
>>
>> height = ALIGN(mode_cmd->height, 8);
>> size = mode_cmd->pitches[0] * height;
>> aligned_size = ALIGN(size, PAGE_SIZE);
>> - ret = amdgpu_gem_object_create(adev, aligned_size, 0,
>> - AMDGPU_GEM_DOMAIN_VRAM,
>> + ret = amdgpu_gem_object_create(adev, aligned_size, 0, domain,
>> AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
>> AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
>> AMDGPU_GEM_CREATE_VRAM_CLEARED,
>> @@ -166,7 +168,7 @@ static int amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
>> }
>>
>>
>> - ret = amdgpu_bo_pin(abo, AMDGPU_GEM_DOMAIN_VRAM, NULL);
>> + ret = amdgpu_bo_pin(abo, domain, NULL);
>> if (ret) {
>> amdgpu_bo_unreserve(abo);
>> goto out_unref;
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index a3bf021..9b05abd 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -2982,11 +2982,13 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane,
>> {
>> struct amdgpu_framebuffer *afb;
>> struct drm_gem_object *obj;
>> + struct amdgpu_device *adev;
>> struct amdgpu_bo *rbo;
>> uint64_t chroma_addr = 0;
>> - int r;
>> struct dm_plane_state *dm_plane_state_new, *dm_plane_state_old;
>> unsigned int awidth;
>> + uint32_t domain;
>> + int r;
>>
>> dm_plane_state_old = to_dm_plane_state(plane->state);
>> dm_plane_state_new = to_dm_plane_state(new_state);
>> @@ -3000,12 +3002,17 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane,
>>
>> obj = afb->obj;
>> rbo = gem_to_amdgpu_bo(obj);
>> + adev = amdgpu_ttm_adev(rbo->tbo.bdev);
>> r = amdgpu_bo_reserve(rbo, false);
>> if (unlikely(r != 0))
>> return r;
>>
>> - r = amdgpu_bo_pin(rbo, AMDGPU_GEM_DOMAIN_VRAM, &afb->address);
>> + if (plane->type != DRM_PLANE_TYPE_CURSOR)
>
> Do cursors have to be in vram? It seems like they shouldn't.
>
> Alex
>
>> + domain = amdgpu_framebuffer_domains(adev);
>> + else
>> + domain = AMDGPU_GEM_DOMAIN_VRAM;
>>
>> + r = amdgpu_bo_pin(rbo, domain, &afb->address);
>>
>> amdgpu_bo_unreserve(rbo);
>>
>> --
>> 2.7.4
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
