Re: [PATCH 1/3] drm/amdgpu: allow framebuffer in GART memory as well

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> to make all the files use consistent naming going forward to make the code more
> readable.
OK. Will do.

Sam


On 2018-01-04 05:43 PM, Alex Deucher wrote:
> On Thu, Jan 4, 2018 at 5:26 PM, Samuel Li <samuel.li@xxxxxxx> wrote:
>>>> +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?
> 
> That file is still a bit of a mess but I'm trying to make all the
> files use consistent naming going forward to make the code more
> readable.  The IP files (gfx, uvd, gmc, etc.) already do and I
> recently cleaned up amdgpu_device.c.
> 
>>
>>>> +       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.
>>
> 
> Sounds good.  thanks for checking.
> 
> Alex
> 
>> 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@xxxxxxx> wrote:
>>>> From: Christian König <christian.koenig@xxxxxxx>
>>>>
>>>> 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@xxxxxxx>
>>>> Reviewed-by: Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx>
>>>> Acked-by: Alex Deucher <alexander.deucher@xxxxxxx>
>>>> Signed-off-by: Samuel Li <samuel.li@xxxxxxx>
>>>> ---
>>>>  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@xxxxxxxxxxxxxxxxxxxxx
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux