Patch #1: Avoid the hardware bug!

E.g. even when we avoid different placements it would be good to have a patch which turns off immediate flipping when switching from VRAM to GTT.

That is as safety net and to document that we need to avoid this condition on the hardware.

Patch #2: Go into amdgpu_bo_pin_restricted() and change the pinning preference.

In other words add something like the following:

if (domain & AMDGPU_GEM_DOMAIN_GTT && bo->preferred_domains & AMDGPU_GEM_DOMAIN_GTT)
    domain = AMDGPU_GEM_DOMAIN_GTT;

That should be everything we need here.

Regards,
Christian.

Am 12.04.2018 um 20:07 schrieb Samuel Li:
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.leichtzumer...@gmail.com 
<mailto:ckoenig.leichtzumer...@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...@amd.com
          <mailto:samuel...@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@lists.freedesktop.org <mailto:amd-gfx@lists.freedesktop.org>
      https://lists.freedesktop.org/mailman/listinfo/amd-gfx
      <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