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 >>>> <[email protected] >>>> <mailto:[email protected]>> 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 <[email protected] >>>> <mailto:[email protected]>> >>>> --- >>>> 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 >>>> [email protected] <mailto:[email protected]> >>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >>>> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx> >>>> >>>> >>> > _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
