* [PATCH 1/1] drm/amdgpu: Enable scatter gather display support
@ 2018-04-18 0:40 Samuel Li
[not found] ` <1524012025-7244-1-git-send-email-Samuel.Li-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 8+ messages in thread
From: Samuel Li @ 2018-04-18 0:40 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: 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 potention hang issue introduced by change
"allow framebuffer in GART memory as well" due to CZ/ST hardware
limitation.
v2: Change default setting to auto.
v3: Move some logic from amdgpu_display_framebuffer_domains()
to pin function, suggested by Christian.
Signed-off-by: Samuel Li <Samuel.Li@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 ++--
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/amdgpu/amdgpu_object.c | 25 +++++++++++++++++++++--
drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 2 +-
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
8 files changed, 35 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index b3d047d..26429de 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;
@@ -137,6 +138,7 @@ extern int amdgpu_si_support;
extern int amdgpu_cik_support;
#endif
+#define AMDGPU_SG_THRESHOLD (256*1024*1024)
#define AMDGPU_DEFAULT_GTT_SIZE_MB 3072ULL /* 3GB by default */
#define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS 3000
#define AMDGPU_MAX_USEC_TIMEOUT 100000 /* 100 ms */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 50f98df..0caa3d2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -189,7 +189,7 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc *crtc,
goto cleanup;
}
- r = amdgpu_bo_pin(new_abo, amdgpu_display_framebuffer_domains(adev), &base);
+ r = amdgpu_bo_pin(new_abo, amdgpu_display_supported_domains(adev), &base);
if (unlikely(r != 0)) {
DRM_ERROR("failed to pin new abo buffer before flip\n");
goto unreserve;
@@ -484,7 +484,7 @@ static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
.create_handle = drm_gem_fb_create_handle,
};
-uint32_t amdgpu_display_framebuffer_domains(struct amdgpu_device *adev)
+uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev)
{
uint32_t domain = AMDGPU_GEM_DOMAIN_VRAM;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
index 2b11d80..f66e3e3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
@@ -23,7 +23,7 @@
#ifndef __AMDGPU_DISPLAY_H__
#define __AMDGPU_DISPLAY_H__
-uint32_t amdgpu_display_framebuffer_domains(struct amdgpu_device *adev);
+uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev);
struct drm_framebuffer *
amdgpu_display_user_framebuffer_create(struct drm_device *dev,
struct drm_file *file_priv,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 0b19482..85dcd1c 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 ff89e84..bc5fd8e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -137,7 +137,7 @@ static int amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
/* need to align pitch with crtc limits */
mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
fb_tiled);
- domain = amdgpu_display_framebuffer_domains(adev);
+ domain = amdgpu_display_supported_domains(adev);
height = ALIGN(mode_cmd->height, 8);
size = mode_cmd->pitches[0] * height;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 24f582c..f0f1f8a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -689,8 +689,29 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
return -EINVAL;
/* A shared bo cannot be migrated to VRAM */
- if (bo->prime_shared_count && (domain == AMDGPU_GEM_DOMAIN_VRAM))
- return -EINVAL;
+ if (bo->prime_shared_count) {
+ if (domain & AMDGPU_GEM_DOMAIN_GTT)
+ domain = AMDGPU_GEM_DOMAIN_GTT;
+ else
+ return -EINVAL;
+ }
+
+ /* display buffer */
+#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_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {
+ 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;
+ else
+ domain = AMDGPU_GEM_DOMAIN_VRAM;
+ }
+ }
+#endif
if (bo->pin_count) {
uint32_t mem_type = bo->tbo.mem.mem_type;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
index 4b584cb7..cf0749f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
@@ -209,7 +209,7 @@ static int amdgpu_gem_begin_cpu_access(struct dma_buf *dma_buf,
struct amdgpu_bo *bo = gem_to_amdgpu_bo(dma_buf->priv);
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
struct ttm_operation_ctx ctx = { true, false };
- u32 domain = amdgpu_display_framebuffer_domains(adev);
+ u32 domain = amdgpu_display_supported_domains(adev);
int ret;
bool reads = (direction == DMA_BIDIRECTIONAL ||
direction == DMA_FROM_DEVICE);
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 6f92a19..1f5603a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3109,12 +3109,11 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane,
return r;
if (plane->type != DRM_PLANE_TYPE_CURSOR)
- domain = amdgpu_display_framebuffer_domains(adev);
+ domain = amdgpu_display_supported_domains(adev);
else
domain = AMDGPU_GEM_DOMAIN_VRAM;
r = amdgpu_bo_pin(rbo, domain, &afb->address);
-
amdgpu_bo_unreserve(rbo);
if (unlikely(r != 0)) {
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <1524012025-7244-1-git-send-email-Samuel.Li-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-18 4:14 ` Alex Deucher
[not found] ` <CADnq5_NOVEDmf1Ccgkai_Zeu-SA7yKgturNbobF0VWK9rXcykA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 8+ messages in thread
From: Alex Deucher @ 2018-04-18 4:14 UTC (permalink / raw)
To: Samuel Li; +Cc: amd-gfx list
On Tue, Apr 17, 2018 at 8:40 PM, Samuel Li <Samuel.Li@amd.com> wrote:
> 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 potention hang issue introduced by change
> "allow framebuffer in GART memory as well" due to CZ/ST hardware
> limitation.
>
> v2: Change default setting to auto.
> v3: Move some logic from amdgpu_display_framebuffer_domains()
> to pin function, suggested by Christian.
> Signed-off-by: Samuel Li <Samuel.Li@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 ++--
> 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/amdgpu/amdgpu_object.c | 25 +++++++++++++++++++++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 2 +-
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
> 8 files changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index b3d047d..26429de 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;
> @@ -137,6 +138,7 @@ extern int amdgpu_si_support;
> extern int amdgpu_cik_support;
> #endif
>
> +#define AMDGPU_SG_THRESHOLD (256*1024*1024)
> #define AMDGPU_DEFAULT_GTT_SIZE_MB 3072ULL /* 3GB by default */
> #define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS 3000
> #define AMDGPU_MAX_USEC_TIMEOUT 100000 /* 100 ms */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index 50f98df..0caa3d2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -189,7 +189,7 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc *crtc,
> goto cleanup;
> }
>
> - r = amdgpu_bo_pin(new_abo, amdgpu_display_framebuffer_domains(adev), &base);
> + r = amdgpu_bo_pin(new_abo, amdgpu_display_supported_domains(adev), &base);
> if (unlikely(r != 0)) {
> DRM_ERROR("failed to pin new abo buffer before flip\n");
> goto unreserve;
> @@ -484,7 +484,7 @@ static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
> .create_handle = drm_gem_fb_create_handle,
> };
>
> -uint32_t amdgpu_display_framebuffer_domains(struct amdgpu_device *adev)
> +uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev)
This change should be a separate patch,
> {
> uint32_t domain = AMDGPU_GEM_DOMAIN_VRAM;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
> index 2b11d80..f66e3e3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
> @@ -23,7 +23,7 @@
> #ifndef __AMDGPU_DISPLAY_H__
> #define __AMDGPU_DISPLAY_H__
>
> -uint32_t amdgpu_display_framebuffer_domains(struct amdgpu_device *adev);
> +uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev);
> struct drm_framebuffer *
> amdgpu_display_user_framebuffer_create(struct drm_device *dev,
> struct drm_file *file_priv,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 0b19482..85dcd1c 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 ff89e84..bc5fd8e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> @@ -137,7 +137,7 @@ static int amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
> /* need to align pitch with crtc limits */
> mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
> fb_tiled);
> - domain = amdgpu_display_framebuffer_domains(adev);
> + domain = amdgpu_display_supported_domains(adev);
>
> height = ALIGN(mode_cmd->height, 8);
> size = mode_cmd->pitches[0] * height;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 24f582c..f0f1f8a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -689,8 +689,29 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
> return -EINVAL;
>
> /* A shared bo cannot be migrated to VRAM */
> - if (bo->prime_shared_count && (domain == AMDGPU_GEM_DOMAIN_VRAM))
> - return -EINVAL;
> + if (bo->prime_shared_count) {
> + if (domain & AMDGPU_GEM_DOMAIN_GTT)
> + domain = AMDGPU_GEM_DOMAIN_GTT;
> + else
> + return -EINVAL;
> + }
This is a bug fix and should be split out into a separate patch.
> +
> + /* display buffer */
> +#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_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {
> + 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;
> + else
> + domain = AMDGPU_GEM_DOMAIN_VRAM;
> + }
I thought we were dropping the module parameter. Also, we had talked
about taking preferred domains into account here as well, but that can
be a follow on patch.
Alex
> + }
> +#endif
>
> if (bo->pin_count) {
> uint32_t mem_type = bo->tbo.mem.mem_type;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> index 4b584cb7..cf0749f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> @@ -209,7 +209,7 @@ static int amdgpu_gem_begin_cpu_access(struct dma_buf *dma_buf,
> struct amdgpu_bo *bo = gem_to_amdgpu_bo(dma_buf->priv);
> struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> struct ttm_operation_ctx ctx = { true, false };
> - u32 domain = amdgpu_display_framebuffer_domains(adev);
> + u32 domain = amdgpu_display_supported_domains(adev);
> int ret;
> bool reads = (direction == DMA_BIDIRECTIONAL ||
> direction == DMA_FROM_DEVICE);
> 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 6f92a19..1f5603a 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3109,12 +3109,11 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane,
> return r;
>
> if (plane->type != DRM_PLANE_TYPE_CURSOR)
> - domain = amdgpu_display_framebuffer_domains(adev);
> + domain = amdgpu_display_supported_domains(adev);
> else
> domain = AMDGPU_GEM_DOMAIN_VRAM;
>
> r = amdgpu_bo_pin(rbo, domain, &afb->address);
> -
> amdgpu_bo_unreserve(rbo);
>
> if (unlikely(r != 0)) {
> --
> 2.7.4
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <CADnq5_NOVEDmf1Ccgkai_Zeu-SA7yKgturNbobF0VWK9rXcykA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2018-04-18 8:34 ` Christian König
[not found] ` <d466b146-8c24-d134-277d-df72c95129f8-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-04-18 15:29 ` Samuel Li
1 sibling, 1 reply; 8+ messages in thread
From: Christian König @ 2018-04-18 8:34 UTC (permalink / raw)
To: Alex Deucher, Samuel Li; +Cc: amd-gfx list
Am 18.04.2018 um 06:14 schrieb Alex Deucher:
> On Tue, Apr 17, 2018 at 8:40 PM, Samuel Li <Samuel.Li@amd.com> wrote:
>> 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 potention hang issue introduced by change
>> "allow framebuffer in GART memory as well" due to CZ/ST hardware
>> limitation.
>>
>> v2: Change default setting to auto.
>> v3: Move some logic from amdgpu_display_framebuffer_domains()
>> to pin function, suggested by Christian.
>> Signed-off-by: Samuel Li <Samuel.Li@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 ++--
>> 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/amdgpu/amdgpu_object.c | 25 +++++++++++++++++++++--
>> drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 2 +-
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
>> 8 files changed, 35 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index b3d047d..26429de 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;
>> @@ -137,6 +138,7 @@ extern int amdgpu_si_support;
>> extern int amdgpu_cik_support;
>> #endif
>>
>> +#define AMDGPU_SG_THRESHOLD (256*1024*1024)
>> #define AMDGPU_DEFAULT_GTT_SIZE_MB 3072ULL /* 3GB by default */
>> #define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS 3000
>> #define AMDGPU_MAX_USEC_TIMEOUT 100000 /* 100 ms */
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> index 50f98df..0caa3d2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> @@ -189,7 +189,7 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc *crtc,
>> goto cleanup;
>> }
>>
>> - r = amdgpu_bo_pin(new_abo, amdgpu_display_framebuffer_domains(adev), &base);
>> + r = amdgpu_bo_pin(new_abo, amdgpu_display_supported_domains(adev), &base);
>> if (unlikely(r != 0)) {
>> DRM_ERROR("failed to pin new abo buffer before flip\n");
>> goto unreserve;
>> @@ -484,7 +484,7 @@ static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
>> .create_handle = drm_gem_fb_create_handle,
>> };
>>
>> -uint32_t amdgpu_display_framebuffer_domains(struct amdgpu_device *adev)
>> +uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev)
> This change should be a separate patch,
>
>> {
>> uint32_t domain = AMDGPU_GEM_DOMAIN_VRAM;
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
>> index 2b11d80..f66e3e3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
>> @@ -23,7 +23,7 @@
>> #ifndef __AMDGPU_DISPLAY_H__
>> #define __AMDGPU_DISPLAY_H__
>>
>> -uint32_t amdgpu_display_framebuffer_domains(struct amdgpu_device *adev);
>> +uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev);
>> struct drm_framebuffer *
>> amdgpu_display_user_framebuffer_create(struct drm_device *dev,
>> struct drm_file *file_priv,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 0b19482..85dcd1c 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 ff89e84..bc5fd8e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>> @@ -137,7 +137,7 @@ static int amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
>> /* need to align pitch with crtc limits */
>> mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
>> fb_tiled);
>> - domain = amdgpu_display_framebuffer_domains(adev);
>> + domain = amdgpu_display_supported_domains(adev);
>>
>> height = ALIGN(mode_cmd->height, 8);
>> size = mode_cmd->pitches[0] * height;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 24f582c..f0f1f8a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -689,8 +689,29 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
>> return -EINVAL;
>>
>> /* A shared bo cannot be migrated to VRAM */
>> - if (bo->prime_shared_count && (domain == AMDGPU_GEM_DOMAIN_VRAM))
>> - return -EINVAL;
>> + if (bo->prime_shared_count) {
>> + if (domain & AMDGPU_GEM_DOMAIN_GTT)
>> + domain = AMDGPU_GEM_DOMAIN_GTT;
>> + else
>> + return -EINVAL;
>> + }
> This is a bug fix and should be split out into a separate patch.
>
>> +
>> + /* display buffer */
>> +#if defined(CONFIG_DRM_AMD_DC)
Please drop that #if, we certainly don't want this to be depend on any
define.
>> + 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) &&
Those checks are static and don't depend on the BO. So they should be in
amdgpu_display_supported_domains().
Regards,
Christian.
>> + domain == (AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {
>> + 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;
>> + else
>> + domain = AMDGPU_GEM_DOMAIN_VRAM;
>> + }
> I thought we were dropping the module parameter. Also, we had talked
> about taking preferred domains into account here as well, but that can
> be a follow on patch.
>
> Alex
>
>> + }
>> +#endif
>>
>> if (bo->pin_count) {
>> uint32_t mem_type = bo->tbo.mem.mem_type;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>> index 4b584cb7..cf0749f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>> @@ -209,7 +209,7 @@ static int amdgpu_gem_begin_cpu_access(struct dma_buf *dma_buf,
>> struct amdgpu_bo *bo = gem_to_amdgpu_bo(dma_buf->priv);
>> struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>> struct ttm_operation_ctx ctx = { true, false };
>> - u32 domain = amdgpu_display_framebuffer_domains(adev);
>> + u32 domain = amdgpu_display_supported_domains(adev);
>> int ret;
>> bool reads = (direction == DMA_BIDIRECTIONAL ||
>> direction == DMA_FROM_DEVICE);
>> 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 6f92a19..1f5603a 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -3109,12 +3109,11 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane,
>> return r;
>>
>> if (plane->type != DRM_PLANE_TYPE_CURSOR)
>> - domain = amdgpu_display_framebuffer_domains(adev);
>> + domain = amdgpu_display_supported_domains(adev);
>> else
>> domain = AMDGPU_GEM_DOMAIN_VRAM;
>>
>> r = amdgpu_bo_pin(rbo, domain, &afb->address);
>> -
>> amdgpu_bo_unreserve(rbo);
>>
>> if (unlikely(r != 0)) {
>> --
>> 2.7.4
>>
>> _______________________________________________
>> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <CADnq5_NOVEDmf1Ccgkai_Zeu-SA7yKgturNbobF0VWK9rXcykA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-04-18 8:34 ` Christian König
@ 2018-04-18 15:29 ` Samuel Li
[not found] ` <16d5d169-c067-0b4c-1049-1dc9cedf5ddc-5C7GfCeVMHo@public.gmane.org>
1 sibling, 1 reply; 8+ messages in thread
From: Samuel Li @ 2018-04-18 15:29 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx list
On 2018-04-18 12:14 AM, Alex Deucher wrote:
> On Tue, Apr 17, 2018 at 8:40 PM, Samuel Li <Samuel.Li@amd.com> wrote:
>> 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 potention hang issue introduced by change
>> "allow framebuffer in GART memory as well" due to CZ/ST hardware
>> limitation.
>>
>> v2: Change default setting to auto.
>> v3: Move some logic from amdgpu_display_framebuffer_domains()
>> to pin function, suggested by Christian.
>> Signed-off-by: Samuel Li <Samuel.Li@amd.com>
>> [...]
>> @@ -484,7 +484,7 @@ static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
>> .create_handle = drm_gem_fb_create_handle,
>> };
>>
>> -uint32_t amdgpu_display_framebuffer_domains(struct amdgpu_device *adev)
>> +uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev)
>
> This change should be a separate patch,
>
OK.
[...]
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 24f582c..f0f1f8a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -689,8 +689,29 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
>> return -EINVAL;
>>
>> /* A shared bo cannot be migrated to VRAM */
>> - if (bo->prime_shared_count && (domain == AMDGPU_GEM_DOMAIN_VRAM))
>> - return -EINVAL;
>> + if (bo->prime_shared_count) {
>> + if (domain & AMDGPU_GEM_DOMAIN_GTT)
>> + domain = AMDGPU_GEM_DOMAIN_GTT;
>> + else
>> + return -EINVAL;
>> + }
>
> This is a bug fix and should be split out into a separate patch.
>
OK.
>> +
>> + /* display buffer */
>> +#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_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {
>> + 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;
>> + else
>> + domain = AMDGPU_GEM_DOMAIN_VRAM;
>> + }
>
> I thought we were dropping the module parameter. Also, we had talked
> about taking preferred domains into account here as well, but that can
> be a follow on patch.
As per the documents, SG display feature can affect other features. The option has been used for debugging on Windows by development, testing and supported teams. So I prefer to keep it.
Regards,
Samuel Li
>
> Alex
>
>> + }
>> +#endif
>>
>> if (bo->pin_count) {
>> uint32_t mem_type = bo->tbo.mem.mem_type;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>> index 4b584cb7..cf0749f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>> @@ -209,7 +209,7 @@ static int amdgpu_gem_begin_cpu_access(struct dma_buf *dma_buf,
>> struct amdgpu_bo *bo = gem_to_amdgpu_bo(dma_buf->priv);
>> struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>> struct ttm_operation_ctx ctx = { true, false };
>> - u32 domain = amdgpu_display_framebuffer_domains(adev);
>> + u32 domain = amdgpu_display_supported_domains(adev);
>> int ret;
>> bool reads = (direction == DMA_BIDIRECTIONAL ||
>> direction == DMA_FROM_DEVICE);
>> 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 6f92a19..1f5603a 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -3109,12 +3109,11 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane,
>> return r;
>>
>> if (plane->type != DRM_PLANE_TYPE_CURSOR)
>> - domain = amdgpu_display_framebuffer_domains(adev);
>> + domain = amdgpu_display_supported_domains(adev);
>> else
>> domain = AMDGPU_GEM_DOMAIN_VRAM;
>>
>> r = amdgpu_bo_pin(rbo, domain, &afb->address);
>> -
>> amdgpu_bo_unreserve(rbo);
>>
>> if (unlikely(r != 0)) {
>> --
>> 2.7.4
>>
>> _______________________________________________
>> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <d466b146-8c24-d134-277d-df72c95129f8-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-04-18 16:01 ` Samuel Li
0 siblings, 0 replies; 8+ messages in thread
From: Samuel Li @ 2018-04-18 16:01 UTC (permalink / raw)
To: christian.koenig-5C7GfCeVMHo, Alex Deucher; +Cc: amd-gfx list
On 2018-04-18 04:34 AM, Christian König wrote:
> Am 18.04.2018 um 06:14 schrieb Alex Deucher:
>> On Tue, Apr 17, 2018 at 8:40 PM, Samuel Li <Samuel.Li@amd.com> wrote:
>>> 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 potention hang issue introduced by change
>>> "allow framebuffer in GART memory as well" due to CZ/ST hardware
>>> limitation.
>>>
>>> v2: Change default setting to auto.
>>> v3: Move some logic from amdgpu_display_framebuffer_domains()
>>> to pin function, suggested by Christian.
>>> Signed-off-by: Samuel Li <Samuel.Li@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 ++--
>>> 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/amdgpu/amdgpu_object.c | 25 +++++++++++++++++++++--
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 2 +-
>>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
>>> 8 files changed, 35 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
[...]
>> This is a bug fix and should be split out into a separate patch.
>>
>>> +
>>> + /* display buffer */
>>> +#if defined(CONFIG_DRM_AMD_DC)
>
> Please drop that #if, we certainly don't want this to be depend on any define.
OK.
>
>>> + 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) &&
>
> Those checks are static and don't depend on the BO. So they should be in amdgpu_display_supported_domains().
OK. That is going to be risky for the future though.
Regards,
Samuel Li
>
> Regards,
> Christian.
>
>>> + domain == (AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {
>>> + 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;
>>> + else
>>> + domain = AMDGPU_GEM_DOMAIN_VRAM;
>>> + }
>> I thought we were dropping the module parameter. Also, we had talked
>> about taking preferred domains into account here as well, but that can
>> be a follow on patch.
>>
>> Alex
>>
>>> + }
>>> +#endif
>>>
>>> if (bo->pin_count) {
>>> uint32_t mem_type = bo->tbo.mem.mem_type;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>> index 4b584cb7..cf0749f 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>> @@ -209,7 +209,7 @@ static int amdgpu_gem_begin_cpu_access(struct dma_buf *dma_buf,
>>> struct amdgpu_bo *bo = gem_to_amdgpu_bo(dma_buf->priv);
>>> struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>> struct ttm_operation_ctx ctx = { true, false };
>>> - u32 domain = amdgpu_display_framebuffer_domains(adev);
>>> + u32 domain = amdgpu_display_supported_domains(adev);
>>> int ret;
>>> bool reads = (direction == DMA_BIDIRECTIONAL ||
>>> direction == DMA_FROM_DEVICE);
>>> 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 6f92a19..1f5603a 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -3109,12 +3109,11 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane,
>>> return r;
>>>
>>> if (plane->type != DRM_PLANE_TYPE_CURSOR)
>>> - domain = amdgpu_display_framebuffer_domains(adev);
>>> + domain = amdgpu_display_supported_domains(adev);
>>> else
>>> domain = AMDGPU_GEM_DOMAIN_VRAM;
>>>
>>> r = amdgpu_bo_pin(rbo, domain, &afb->address);
>>> -
>>> amdgpu_bo_unreserve(rbo);
>>>
>>> if (unlikely(r != 0)) {
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <16d5d169-c067-0b4c-1049-1dc9cedf5ddc-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-18 16:16 ` Christian König
[not found] ` <6edfc88e-d8da-ca23-1a10-b459d0ee8966-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 8+ messages in thread
From: Christian König @ 2018-04-18 16:16 UTC (permalink / raw)
To: Samuel Li, Alex Deucher; +Cc: amd-gfx list
Am 18.04.2018 um 17:29 schrieb Samuel Li:
>
> On 2018-04-18 12:14 AM, Alex Deucher wrote:
>> On Tue, Apr 17, 2018 at 8:40 PM, Samuel Li <Samuel.Li@amd.com> wrote:
>>> 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 potention hang issue introduced by change
>>> "allow framebuffer in GART memory as well" due to CZ/ST hardware
>>> limitation.
>>>
>>> v2: Change default setting to auto.
>>> v3: Move some logic from amdgpu_display_framebuffer_domains()
>>> to pin function, suggested by Christian.
>>> Signed-off-by: Samuel Li <Samuel.Li@amd.com>
>>> [...]
>>> @@ -484,7 +484,7 @@ static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
>>> .create_handle = drm_gem_fb_create_handle,
>>> };
>>>
>>> -uint32_t amdgpu_display_framebuffer_domains(struct amdgpu_device *adev)
>>> +uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev)
>> This change should be a separate patch,
>>
> OK.
>
> [...]
>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> index 24f582c..f0f1f8a 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> @@ -689,8 +689,29 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
>>> return -EINVAL;
>>>
>>> /* A shared bo cannot be migrated to VRAM */
>>> - if (bo->prime_shared_count && (domain == AMDGPU_GEM_DOMAIN_VRAM))
>>> - return -EINVAL;
>>> + if (bo->prime_shared_count) {
>>> + if (domain & AMDGPU_GEM_DOMAIN_GTT)
>>> + domain = AMDGPU_GEM_DOMAIN_GTT;
>>> + else
>>> + return -EINVAL;
>>> + }
>> This is a bug fix and should be split out into a separate patch.
>>
> OK.
>
>>> +
>>> + /* display buffer */
>>> +#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_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {
>>> + 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;
>>> + else
>>> + domain = AMDGPU_GEM_DOMAIN_VRAM;
>>> + }
>> I thought we were dropping the module parameter. Also, we had talked
>> about taking preferred domains into account here as well, but that can
>> be a follow on patch.
> As per the documents, SG display feature can affect other features. The option has been used for debugging on Windows by development, testing and supported teams. So I prefer to keep it.
Mhm, for developer testing we can easily modify
amdgpu_display_supported_domains().
The real question is should we give an end user the ability to modify
the behavior? I currently can't think of a reason for that.
Regards,
Christian.
>
> Regards,
> Samuel Li
>
>> Alex
>>
>>> + }
>>> +#endif
>>>
>>> if (bo->pin_count) {
>>> uint32_t mem_type = bo->tbo.mem.mem_type;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>> index 4b584cb7..cf0749f 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>> @@ -209,7 +209,7 @@ static int amdgpu_gem_begin_cpu_access(struct dma_buf *dma_buf,
>>> struct amdgpu_bo *bo = gem_to_amdgpu_bo(dma_buf->priv);
>>> struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>> struct ttm_operation_ctx ctx = { true, false };
>>> - u32 domain = amdgpu_display_framebuffer_domains(adev);
>>> + u32 domain = amdgpu_display_supported_domains(adev);
>>> int ret;
>>> bool reads = (direction == DMA_BIDIRECTIONAL ||
>>> direction == DMA_FROM_DEVICE);
>>> 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 6f92a19..1f5603a 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -3109,12 +3109,11 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane,
>>> return r;
>>>
>>> if (plane->type != DRM_PLANE_TYPE_CURSOR)
>>> - domain = amdgpu_display_framebuffer_domains(adev);
>>> + domain = amdgpu_display_supported_domains(adev);
>>> else
>>> domain = AMDGPU_GEM_DOMAIN_VRAM;
>>>
>>> r = amdgpu_bo_pin(rbo, domain, &afb->address);
>>> -
>>> amdgpu_bo_unreserve(rbo);
>>>
>>> if (unlikely(r != 0)) {
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <6edfc88e-d8da-ca23-1a10-b459d0ee8966-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-04-18 16:21 ` Samuel Li
[not found] ` <918bc673-b6ce-510a-20ed-a4566c81acd9-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 8+ messages in thread
From: Samuel Li @ 2018-04-18 16:21 UTC (permalink / raw)
To: christian.koenig-5C7GfCeVMHo, Alex Deucher; +Cc: amd-gfx list
On 2018-04-18 12:16 PM, Christian König wrote:
> Am 18.04.2018 um 17:29 schrieb Samuel Li:
>>
>> On 2018-04-18 12:14 AM, Alex Deucher wrote:
>>> On Tue, Apr 17, 2018 at 8:40 PM, Samuel Li <Samuel.Li@amd.com> wrote:
>>>> 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 potention hang issue introduced by change
>>>> "allow framebuffer in GART memory as well" due to CZ/ST hardware
>>>> limitation.
>>>>
>>>
>> OK.
>>
[...]
>
> Mhm, for developer testing we can easily modify amdgpu_display_supported_domains().
>
> The real question is should we give an end user the ability to modify the behavior? I currently can't think of a reason for that.
Yes, we do. For example, there are cases that user prefers GTT(VRAM can run out), also there are cases user prefers VRAM for performance.
Regards,
Sam
>
> Regards,
> Christian.
>
>>
>> Regards,
>> Samuel Li
>>
>>> Alex
>>>
>>>> + }
>>>> +#endif
>>>>
>>>> if (bo->pin_count) {
>>>> uint32_t mem_type = bo->tbo.mem.mem_type;
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>>> index 4b584cb7..cf0749f 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>>> @@ -209,7 +209,7 @@ static int amdgpu_gem_begin_cpu_access(struct dma_buf *dma_buf,
>>>> struct amdgpu_bo *bo = gem_to_amdgpu_bo(dma_buf->priv);
>>>> struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>>> struct ttm_operation_ctx ctx = { true, false };
>>>> - u32 domain = amdgpu_display_framebuffer_domains(adev);
>>>> + u32 domain = amdgpu_display_supported_domains(adev);
>>>> int ret;
>>>> bool reads = (direction == DMA_BIDIRECTIONAL ||
>>>> direction == DMA_FROM_DEVICE);
>>>> 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 6f92a19..1f5603a 100644
>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> @@ -3109,12 +3109,11 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane,
>>>> return r;
>>>>
>>>> if (plane->type != DRM_PLANE_TYPE_CURSOR)
>>>> - domain = amdgpu_display_framebuffer_domains(adev);
>>>> + domain = amdgpu_display_supported_domains(adev);
>>>> else
>>>> domain = AMDGPU_GEM_DOMAIN_VRAM;
>>>>
>>>> r = amdgpu_bo_pin(rbo, domain, &afb->address);
>>>> -
>>>> amdgpu_bo_unreserve(rbo);
>>>>
>>>> if (unlikely(r != 0)) {
>>>> --
>>>> 2.7.4
>>>>
>>>> _______________________________________________
>>>> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <918bc673-b6ce-510a-20ed-a4566c81acd9-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-19 6:57 ` Christian König
0 siblings, 0 replies; 8+ messages in thread
From: Christian König @ 2018-04-19 6:57 UTC (permalink / raw)
To: Samuel Li, Alex Deucher; +Cc: amd-gfx list
Am 18.04.2018 um 18:21 schrieb Samuel Li:
>
> On 2018-04-18 12:16 PM, Christian König wrote:
>> Am 18.04.2018 um 17:29 schrieb Samuel Li:
>>> On 2018-04-18 12:14 AM, Alex Deucher wrote:
>>>> On Tue, Apr 17, 2018 at 8:40 PM, Samuel Li <Samuel.Li@amd.com> wrote:
>>>>> 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 potention hang issue introduced by change
>>>>> "allow framebuffer in GART memory as well" due to CZ/ST hardware
>>>>> limitation.
>>>>>
>>> OK.
>>>
> [...]
>
>> Mhm, for developer testing we can easily modify amdgpu_display_supported_domains().
>>
>> The real question is should we give an end user the ability to modify the behavior? I currently can't think of a reason for that.
> Yes, we do. For example, there are cases that user prefers GTT(VRAM can run out), also there are cases user prefers VRAM for performance.
Yeah, but that is exactly the reason why Alex and I wanted to take
preferred_domains into account here, so that we can let userspace
control this behavior.
Regards,
Christian.
>
> Regards,
> Sam
>
>
>> Regards,
>> Christian.
>>
>>> Regards,
>>> Samuel Li
>>>
>>>> Alex
>>>>
>>>>> + }
>>>>> +#endif
>>>>>
>>>>> if (bo->pin_count) {
>>>>> uint32_t mem_type = bo->tbo.mem.mem_type;
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>>>> index 4b584cb7..cf0749f 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>>>> @@ -209,7 +209,7 @@ static int amdgpu_gem_begin_cpu_access(struct dma_buf *dma_buf,
>>>>> struct amdgpu_bo *bo = gem_to_amdgpu_bo(dma_buf->priv);
>>>>> struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>>>> struct ttm_operation_ctx ctx = { true, false };
>>>>> - u32 domain = amdgpu_display_framebuffer_domains(adev);
>>>>> + u32 domain = amdgpu_display_supported_domains(adev);
>>>>> int ret;
>>>>> bool reads = (direction == DMA_BIDIRECTIONAL ||
>>>>> direction == DMA_FROM_DEVICE);
>>>>> 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 6f92a19..1f5603a 100644
>>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>> @@ -3109,12 +3109,11 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane,
>>>>> return r;
>>>>>
>>>>> if (plane->type != DRM_PLANE_TYPE_CURSOR)
>>>>> - domain = amdgpu_display_framebuffer_domains(adev);
>>>>> + domain = amdgpu_display_supported_domains(adev);
>>>>> else
>>>>> domain = AMDGPU_GEM_DOMAIN_VRAM;
>>>>>
>>>>> r = amdgpu_bo_pin(rbo, domain, &afb->address);
>>>>> -
>>>>> amdgpu_bo_unreserve(rbo);
>>>>>
>>>>> if (unlikely(r != 0)) {
>>>>> --
>>>>> 2.7.4
>>>>>
>>>>> _______________________________________________
>>>>> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-04-19 6:57 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-18 0:40 [PATCH 1/1] drm/amdgpu: Enable scatter gather display support Samuel Li
[not found] ` <1524012025-7244-1-git-send-email-Samuel.Li-5C7GfCeVMHo@public.gmane.org>
2018-04-18 4:14 ` Alex Deucher
[not found] ` <CADnq5_NOVEDmf1Ccgkai_Zeu-SA7yKgturNbobF0VWK9rXcykA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-04-18 8:34 ` Christian König
[not found] ` <d466b146-8c24-d134-277d-df72c95129f8-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-04-18 16:01 ` Samuel Li
2018-04-18 15:29 ` Samuel Li
[not found] ` <16d5d169-c067-0b4c-1049-1dc9cedf5ddc-5C7GfCeVMHo@public.gmane.org>
2018-04-18 16:16 ` Christian König
[not found] ` <6edfc88e-d8da-ca23-1a10-b459d0ee8966-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-04-18 16:21 ` Samuel Li
[not found] ` <918bc673-b6ce-510a-20ed-a4566c81acd9-5C7GfCeVMHo@public.gmane.org>
2018-04-19 6:57 ` Christian König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.