Am 06.03.2018 um 18:22 schrieb Li, Samuel:
And exactly that's the problematical assumption.
Not assumption, I tested. You have any typical use case that it might become a 
problem?


Not 100% sure, but I doubt you have tried that with different userspace programs.

Especially animated boot screens could allocated quite a number of framebuffers.


addition to that I agree with Michel that the module parameter is overkill.
That I already explained. Currently SG display feature needs to provide options 
for all kinds of use cases. All amd drivers so far provides options, and the 
default configuration is actually to allocate everything from GTT when possible.

That isn't job of the kernel to have this parameter. Saying that we should probably add a DDX and/or environment option to control that.

Regards,
Christian.


Regards,
Samuel Li


-----Original Message-----
From: Koenig, Christian
Sent: Tuesday, March 06, 2018 12:12 PM
To: Li, Samuel <samuel...@amd.com>; Alex Deucher
<alexdeuc...@gmail.com>
Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

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:alexdeuc...@gmail.com]
Sent: Tuesday, March 06, 2018 12:04 PM
To: Li, Samuel <samuel...@amd.com>
Cc: Koenig, Christian <christian.koe...@amd.com>; amd-gfx list <amd-
g...@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...@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to