And exactly that's the problematical assumption. This doesn't print only when the module is loaded, but rather when a framebuffer object is created. And depending on the use case that can even be many many times per second. Please remove that printing, addition to that I agree with Michel that the module parameter is overkill. Regards, Christian. Am 06.03.2018 um 18:09 schrieb Li, Samuel: > This information is kind of important, and it only prints once typically when module is loaded. > > Regards, > Samuel Li > > >> -----Original Message----- >> From: Alex Deucher [mailto:alexdeucher at gmail.com] >> Sent: Tuesday, March 06, 2018 12:04 PM >> To: Li, Samuel <Samuel.Li at amd.com> >> Cc: Koenig, Christian <Christian.Koenig at amd.com>; amd-gfx list <amd- >> gfx at lists.freedesktop.org> >> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support >> >> On Tue, Mar 6, 2018 at 11:49 AM, Samuel Li <samuel.li at amd.com> wrote: >>>>> domain = amdgpu_display_framebuffer_domains(adev); >>>>> + if (domain == AMDGPU_GEM_DOMAIN_GTT) { >>>>> + DRM_INFO("Scatter gather display: enabled\n"); >>>>> + } else if (domain & AMDGPU_GEM_DOMAIN_GTT) { >>>>> + DRM_INFO("Scatter gather display: auto\n"); >>>>> + } >>>> Dito and printing that here is not a good idea as far as I can see. >>>> >>>> Christian. >>> >>> The intention here is to print out when fb is created. You have other >> suggestions? >> >> Make it DRM_DEBUG? otherwise you'll spam the logs. >> >>> Sam >>> >>> >>> On 2018-03-03 08:41 AM, Christian König wrote: >>>> Am 03.03.2018 um 00:25 schrieb Samuel Li: >>>>> It's enabled by default. -1 is auto, to allow both vram and gtt >>>>> memory be available, for testing purpose only. >>>>> --- >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++-- >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++++ >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 5 +++++ >>>>> 4 files changed, 17 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h >>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h >>>>> index 292c7e7..6b0ee34 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..dfa11b1 100644 >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c >>>>> @@ -513,8 +513,13 @@ 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) { >>>>> + domain |= AMDGPU_GEM_DOMAIN_GTT; >>>>> + } >>>> Coding style, that if shouldn't use "{" and "}". >>>> >>>>> + } >>>>> #endif >>>>> return domain; >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >>>>> index e670936..f0ada24 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..10f1f4f 100644 >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c >>>>> @@ -138,6 +138,11 @@ 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_INFO("Scatter gather display: enabled\n"); >>>>> + } else if (domain & AMDGPU_GEM_DOMAIN_GTT) { >>>>> + DRM_INFO("Scatter gather display: auto\n"); >>>>> + } >>>> Dito and printing that here is not a good idea as far as I can see. >>>> >>>> Christian. >>>> >>>>> height = ALIGN(mode_cmd->height, 8); >>>>> size = mode_cmd->pitches[0] * height; >>> _______________________________________________ >>> amd-gfx mailing list >>> amd-gfx at lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
