Please clarify, Christian. How would you like it to be implemented? Sam On 2018-04-12 02:00 PM, Christian König wrote: >> 1) Turn off immediate mode when flipping between VRAM/GTT. > That must be implemented independently. > > See working around the hardware bug should be a different patch than implementing a placement policy. > >> As per discussion, the 3rd one, which is the current patch, seems the best so far. > And I can only repeat myself. Alex and I are the maintainers of the kernel module, so we are the one who decide on how to implement things here. > > And we both noted that this approach of overriding user space decisions is not a good design. > > The placement policy I suggest by preferring GTT over VRAM on APUs should be trivial to implement and as far as I can see avoids all negative side effects. > > Regards, > Christian. > > Am 12.04.2018 um 19:21 schrieb Samuel Li: >>> The point is this kernel change now needs to be reworked and adapted to what Mesa is doing. >> Three options have been brought up for kernel, >> 1) Turn off immediate mode when flipping between VRAM/GTT. >> 2) Check the domain of the current fb and then adjust the new one before pinning it. >> 3) Use only VRAM or GTT depending on a threshhold. >> >> As per discussion, the 3rd one, which is the current patch, seems the best so far. >> >> Regards, >> Samuel Li >> >> >> >> On 2018-04-12 01:03 PM, Christian König wrote: >>>> Can you be more specific, Christian? Mesa has this, I don't think it needs anything else: >>> Completely agree, that's what I suggested to implement. >>> >>> The point is this kernel change now needs to be reworked and adapted to what Mesa is doing. >>> >>> Regards, >>> Christian. >>> >>> Am 12.04.2018 um 18:40 schrieb Marek Olšák: >>>> Can you be more specific, Christian? Mesa has this, I don't think it needs anything else: >>>> https://cgit.freedesktop.org/mesa/mesa/commit/?id=7d2079908d9ef05ec3f35b7078833e57846cab5b >>>> >>>> Marek >>>> >>>> On Wed, Mar 28, 2018 at 3:46 AM, Christian König <ckoenig.leichtzumerken at gmail.com <mailto:ckoenig.leichtzumerken at gmail.com>> wrote: >>>> >>>>     Am 28.03.2018 um 00:22 schrieb Samuel Li: >>>> >>>>         It's auto by default. For CZ/ST, auto setting enables sg display >>>>         when vram size is small; otherwise still uses vram. >>>>         This patch fixed some potential hang issue introduced by change >>>>         "allow framebuffer in GART memory as well" due to CZ/ST hardware >>>>         limitation. >>>> >>>> >>>>     Well that is still a NAK. >>>> >>>>     As discussed now multiple times please implement the necessary >>>>     changes in Mesa. >>>> >>>>     Regards, >>>>     Christian. >>>> >>>> >>>> >>>>         v2: Change default setting to auto, also some misc changes. >>>>         Signed-off-by: Samuel Li <Samuel.Li at amd.com >>>>         <mailto:Samuel.Li at amd.com>> >>>>         --- >>>>          drivers/gpu/drm/amd/amdgpu/amdgpu.h   | 1 + >>>>          drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 10 ++++++++-- >>>>          drivers/gpu/drm/amd/amdgpu/amdgpu_display.h   | 2 ++ >>>>          drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 4 ++++ >>>>          drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c  | 2 ++ >>>>          drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++- >>>>          6 files changed, 19 insertions(+), 3 deletions(-) >>>> >>>>         diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h >>>>         b/drivers/gpu/drm/amd/amdgpu/amdgpu.h >>>>         index a7e2229..c942362 100644 >>>>         --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h >>>>         +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h >>>>         @@ -129,6 +129,7 @@ extern int amdgpu_lbpw; >>>>          extern int amdgpu_compute_multipipe; >>>>          extern int amdgpu_gpu_recovery; >>>>          extern int amdgpu_emu_mode; >>>>         +extern int amdgpu_sg_display; >>>>           #ifdef CONFIG_DRM_AMDGPU_SI >>>>          extern int amdgpu_si_support; >>>>         diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c >>>>         b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c >>>>         index 5495b29..1e7b950 100644 >>>>         --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c >>>>         +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c >>>>         @@ -513,8 +513,14 @@ uint32_t >>>>         amdgpu_display_framebuffer_domains(struct amdgpu_device *adev) >>>>          #if defined(CONFIG_DRM_AMD_DC) >>>>             if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type >>>>         < CHIP_RAVEN && >>>>               adev->flags & AMD_IS_APU && >>>>         -      amdgpu_device_asic_has_dc_support(adev->asic_type)) >>>>         -        domain |= AMDGPU_GEM_DOMAIN_GTT; >>>>         +      amdgpu_device_asic_has_dc_support(adev->asic_type)) { >>>>         +        if (amdgpu_sg_display == 1) >>>>         +            domain = AMDGPU_GEM_DOMAIN_GTT; >>>>         +        else if (amdgpu_sg_display == -1) { >>>>         +            if (adev->gmc.real_vram_size < >>>>         AMDGPU_SG_THRESHOLD) >>>>         +                domain = AMDGPU_GEM_DOMAIN_GTT; >>>>         +        } >>>>         +    } >>>>          #endif >>>>             return domain; >>>>         diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h >>>>         b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h >>>>         index 2b11d80..2b25393 100644 >>>>         --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h >>>>         +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h >>>>         @@ -23,6 +23,8 @@ >>>>          #ifndef __AMDGPU_DISPLAY_H__ >>>>          #define __AMDGPU_DISPLAY_H__ >>>>          +#define AMDGPU_SG_THRESHOLD (256*1024*1024) >>>>         + >>>>          uint32_t amdgpu_display_framebuffer_domains(struct >>>>         amdgpu_device *adev); >>>>          struct drm_framebuffer * >>>>          amdgpu_display_user_framebuffer_create(struct drm_device *dev, >>>>         diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >>>>         b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >>>>         index 1bfce79..19f11a5 100644 >>>>         --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >>>>         +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >>>>         @@ -132,6 +132,7 @@ int amdgpu_lbpw = -1; >>>>          int amdgpu_compute_multipipe = -1; >>>>          int amdgpu_gpu_recovery = -1; /* auto */ >>>>          int amdgpu_emu_mode = 0; >>>>         +int amdgpu_sg_display = -1; >>>>           MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in >>>>         megabytes"); >>>>          module_param_named(vramlimit, amdgpu_vram_limit, int, 0600); >>>>         @@ -290,6 +291,9 @@ module_param_named(gpu_recovery, >>>>         amdgpu_gpu_recovery, int, 0444); >>>>          MODULE_PARM_DESC(emu_mode, "Emulation mode, (1 = enable, 0 = >>>>         disable)"); >>>>          module_param_named(emu_mode, amdgpu_emu_mode, int, 0444); >>>>          +MODULE_PARM_DESC(sg_display, "Enable scatter gather >>>>         display, (1 = enable, 0 = disable, -1 = auto"); >>>>         +module_param_named(sg_display, amdgpu_sg_display, int, 0444); >>>>         + >>>>          #ifdef CONFIG_DRM_AMDGPU_SI >>>>           #if defined(CONFIG_DRM_RADEON) || >>>>         defined(CONFIG_DRM_RADEON_MODULE) >>>>         diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c >>>>         b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c >>>>         index 1206301..f57c355 100644 >>>>         --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c >>>>         +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c >>>>         @@ -138,6 +138,8 @@ static int >>>>         amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev, >>>>             mode_cmd->pitches[0] = amdgpu_align_pitch(adev, >>>>         mode_cmd->width, cpp, >>>>         fb_tiled); >>>>             domain = amdgpu_display_framebuffer_domains(adev); >>>>         +    if (domain & AMDGPU_GEM_DOMAIN_GTT) >>>>         +        DRM_DEBUG_DRIVER("Scatter gather display: >>>>         enabled\n"); >>>>             height = ALIGN(mode_cmd->height, 8); >>>>             size = mode_cmd->pitches[0] * height; >>>>         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 68ab325..7e9f247 100644 >>>>         --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >>>>         +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >>>>         @@ -3074,7 +3074,8 @@ static int >>>>         dm_plane_helper_prepare_fb(struct drm_plane *plane, >>>>                 domain = AMDGPU_GEM_DOMAIN_VRAM; >>>>             r = amdgpu_bo_pin(rbo, domain, &afb->address); >>>>         - >>>>         +    rbo->preferred_domains = domain; >>>>         +    rbo->allowed_domains = domain; >>>>             amdgpu_bo_unreserve(rbo); >>>>             if (unlikely(r != 0)) { >>>> >>>> >>>>     _______________________________________________ >>>>     amd-gfx mailing list >>>>     amd-gfx at lists.freedesktop.org <mailto:amd-gfx at lists.freedesktop.org> >>>>     https://lists.freedesktop.org/mailman/listinfo/amd-gfx >>>>     <https://lists.freedesktop.org/mailman/listinfo/amd-gfx> >>>> >>>> >>> >
