* [PATCH v2 1/1] drm/amdgpu: Enable scatter gather display support
@ 2018-03-27 22:22 Samuel Li
[not found] ` <1522189371-13767-1-git-send-email-Samuel.Li-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Samuel Li @ 2018-03-27 22:22 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 potential hang issue introduced by change
"allow framebuffer in GART memory as well" due to CZ/ST hardware
limitation.
v2: Change default setting to auto, also some misc changes.
Signed-off-by: Samuel Li <Samuel.Li@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)) {
--
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] 12+ messages in thread
* Re: [PATCH v2 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <1522189371-13767-1-git-send-email-Samuel.Li-5C7GfCeVMHo@public.gmane.org>
@ 2018-03-28 7:46 ` Christian König
[not found] ` <7cc47f78-66eb-148d-96a2-b97760f767bf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2018-03-28 7:46 UTC (permalink / raw)
To: Samuel Li, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
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.Li@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
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <7cc47f78-66eb-148d-96a2-b97760f767bf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-04-12 16:40 ` Marek Olšák
[not found] ` <CAAxE2A4msxZa19Cvv6giMDDYg57-M_qawgFRM2aaK6sUERuqSA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Marek Olšák @ 2018-04-12 16:40 UTC (permalink / raw)
To: Christian König; +Cc: amd-gfx mailing list, Samuel Li
[-- Attachment #1.1: Type: text/plain, Size: 6327 bytes --]
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.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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.Li-5C7GfCeVMHo@public.gmane.org>
>> ---
>> 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-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
[-- Attachment #1.2: Type: text/html, Size: 8022 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <CAAxE2A4msxZa19Cvv6giMDDYg57-M_qawgFRM2aaK6sUERuqSA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2018-04-12 17:03 ` Christian König
[not found] ` <15196bc3-76cb-0a52-c05f-f23131f77e7f-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2018-04-12 17:03 UTC (permalink / raw)
To: Marek Olšák; +Cc: amd-gfx mailing list, Samuel Li
[-- Attachment #1.1: Type: text/plain, Size: 7783 bytes --]
> 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.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
> <mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> 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.Li-5C7GfCeVMHo@public.gmane.org
> <mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>
> ---
> 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-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
>
>
[-- Attachment #1.2: Type: text/html, Size: 12234 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <15196bc3-76cb-0a52-c05f-f23131f77e7f-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-12 17:21 ` Samuel Li
[not found] ` <20bdb4b1-1986-22e4-b0d1-1713b8794f83-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Samuel Li @ 2018-04-12 17:21 UTC (permalink / raw)
To: Christian König, Alexander.Deucher-5C7GfCeVMHo,
Michel.Daenzer-5C7GfCeVMHo, Marek Olšák
Cc: amd-gfx mailing list
> 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.leichtzumerken@gmail.com <mailto:ckoenig.leichtzumerken@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.Li@amd.com
>> <mailto:Samuel.Li@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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <20bdb4b1-1986-22e4-b0d1-1713b8794f83-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-12 18:00 ` Christian König
[not found] ` <de4659b0-918a-e548-d6ee-12f3a9e88df5-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2018-04-12 18:00 UTC (permalink / raw)
To: Samuel Li, Alexander.Deucher-5C7GfCeVMHo,
Michel.Daenzer-5C7GfCeVMHo, Marek Olšák
Cc: amd-gfx mailing list
> 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.leichtzumerken@gmail.com <mailto:ckoenig.leichtzumerken@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.Li@amd.com
>>> <mailto:Samuel.Li@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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <de4659b0-918a-e548-d6ee-12f3a9e88df5-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-12 18:07 ` Samuel Li
[not found] ` <277e44be-d618-ad29-06d2-ea12466571c9-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Samuel Li @ 2018-04-12 18:07 UTC (permalink / raw)
To: Christian König, Alexander.Deucher-5C7GfCeVMHo,
Michel.Daenzer-5C7GfCeVMHo, Marek Olšák
Cc: amd-gfx mailing list
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.leichtzumerken@gmail.com <mailto:ckoenig.leichtzumerken@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.Li@amd.com
>>>> <mailto:Samuel.Li@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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <277e44be-d618-ad29-06d2-ea12466571c9-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-12 18:47 ` Christian König
[not found] ` <18325399-d78e-2442-ccf4-55e30d9f5c52-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2018-04-12 18:47 UTC (permalink / raw)
To: Samuel Li, Alexander.Deucher-5C7GfCeVMHo,
Michel.Daenzer-5C7GfCeVMHo, Marek Olšák
Cc: amd-gfx mailing list
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.leichtzumerken@gmail.com <mailto:ckoenig.leichtzumerken@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.Li@amd.com
>>>>> <mailto:Samuel.Li@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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <18325399-d78e-2442-ccf4-55e30d9f5c52-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-12 20:01 ` Samuel Li
[not found] ` <b461df89-0638-77a7-409a-346c64521a40-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Samuel Li @ 2018-04-12 20:01 UTC (permalink / raw)
To: Christian König, Alexander.Deucher-5C7GfCeVMHo,
Michel.Daenzer-5C7GfCeVMHo, Marek Olšák
Cc: amd-gfx mailing list
The 4th proposal :)
> 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.
This is basically against the idea of Marek's change: in Mesa, both GTT/VRAM are allowed; but now in your kernel change, all buffers uses GTT only(not limited to display buffer now).
To compare, current patch still seems better, since it only circumscribes display buffer.
Sam
On 2018-04-12 02:47 PM, Christian König wrote:
> 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.leichtzumerken@gmail.com <mailto:ckoenig.leichtzumerken@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.Li@amd.com
>>>>>> <mailto:Samuel.Li@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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <b461df89-0638-77a7-409a-346c64521a40-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-12 20:13 ` Christian König
[not found] ` <5f492910-cac3-643f-efec-472f791e551e-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2018-04-12 20:13 UTC (permalink / raw)
To: Samuel Li, Alexander.Deucher-5C7GfCeVMHo,
Michel.Daenzer-5C7GfCeVMHo, Marek Olšák
Cc: amd-gfx mailing list
Am 12.04.2018 um 22:01 schrieb Samuel Li:
> The 4th proposal :)
>
>> 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.
> This is basically against the idea of Marek's change: in Mesa, both GTT/VRAM are allowed; but now in your kernel change, all buffers uses GTT only(not limited to display buffer now).
> To compare, current patch still seems better, since it only circumscribes display buffer.
What I suggested here is for pinning preference, that only affects BOs
used for scanout and it also only affects them while they are scanned out.
Please implement as advised or otherwise we need to assign the work to
somebody else.
Thanks,
Christian.
>
> Sam
>
>
> On 2018-04-12 02:47 PM, Christian König wrote:
>> 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.leichtzumerken@gmail.com <mailto:ckoenig.leichtzumerken@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.Li@amd.com
>>>>>>> <mailto:Samuel.Li@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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <5f492910-cac3-643f-efec-472f791e551e-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-12 20:42 ` Samuel Li
[not found] ` <83a2320e-74a9-4539-5b85-0e80a35525fb-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Samuel Li @ 2018-04-12 20:42 UTC (permalink / raw)
To: Christian König, Alexander.Deucher-5C7GfCeVMHo,
Michel.Daenzer-5C7GfCeVMHo, Marek Olšák
Cc: amd-gfx mailing list
Hi Christian,
If you have any good proposal, please describe here.
Otherwise kindly avoid saying anything based on your emotion.
Regards,
Sam
On 2018-04-12 04:13 PM, Christian König wrote:
> Am 12.04.2018 um 22:01 schrieb Samuel Li:
>> The 4th proposal :)
>>
>>> 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.
>> This is basically against the idea of Marek's change: in Mesa, both GTT/VRAM are allowed; but now in your kernel change, all buffers uses GTT only(not limited to display buffer now).
>> To compare, current patch still seems better, since it only circumscribes display buffer.
>
> What I suggested here is for pinning preference, that only affects BOs used for scanout and it also only affects them while they are scanned out.
>
> Please implement as advised or otherwise we need to assign the work to somebody else.
>
> Thanks,
> Christian.
>
>>
>> Sam
>>
>>
>> On 2018-04-12 02:47 PM, Christian König wrote:
>>> 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.leichtzumerken@gmail.com <mailto:ckoenig.leichtzumerken@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.Li@amd.com
>>>>>>>> <mailto:Samuel.Li@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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/1] drm/amdgpu: Enable scatter gather display support
[not found] ` <83a2320e-74a9-4539-5b85-0e80a35525fb-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-12 22:48 ` Christian König
0 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2018-04-12 22:48 UTC (permalink / raw)
To: Samuel Li, Christian König, Alexander.Deucher-5C7GfCeVMHo,
Michel.Daenzer-5C7GfCeVMHo, Marek Olšák
Cc: amd-gfx mailing list
Hi Sam,
yeah sorry for that. It's already rather late here and I got a bit taken
away.
But I still insist that you at least try as I advised, I'm pretty sure
that this approach will work and cover all use cases discussed so far.
Regards,
Christian.
Am 12.04.2018 um 22:42 schrieb Samuel Li:
> Hi Christian,
>
> If you have any good proposal, please describe here.
> Otherwise kindly avoid saying anything based on your emotion.
>
> Regards,
> Sam
>
>
>
> On 2018-04-12 04:13 PM, Christian König wrote:
>> Am 12.04.2018 um 22:01 schrieb Samuel Li:
>>> The 4th proposal :)
>>>
>>>> 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.
>>> This is basically against the idea of Marek's change: in Mesa, both GTT/VRAM are allowed; but now in your kernel change, all buffers uses GTT only(not limited to display buffer now).
>>> To compare, current patch still seems better, since it only circumscribes display buffer.
>> What I suggested here is for pinning preference, that only affects BOs used for scanout and it also only affects them while they are scanned out.
>>
>> Please implement as advised or otherwise we need to assign the work to somebody else.
>>
>> Thanks,
>> Christian.
>>
>>> Sam
>>>
>>>
>>> On 2018-04-12 02:47 PM, Christian König wrote:
>>>> 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.leichtzumerken@gmail.com <mailto:ckoenig.leichtzumerken@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.Li@amd.com
>>>>>>>>> <mailto:Samuel.Li@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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-04-12 22:48 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-27 22:22 [PATCH v2 1/1] drm/amdgpu: Enable scatter gather display support Samuel Li
[not found] ` <1522189371-13767-1-git-send-email-Samuel.Li-5C7GfCeVMHo@public.gmane.org>
2018-03-28 7:46 ` Christian König
[not found] ` <7cc47f78-66eb-148d-96a2-b97760f767bf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-04-12 16:40 ` Marek Olšák
[not found] ` <CAAxE2A4msxZa19Cvv6giMDDYg57-M_qawgFRM2aaK6sUERuqSA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-04-12 17:03 ` Christian König
[not found] ` <15196bc3-76cb-0a52-c05f-f23131f77e7f-5C7GfCeVMHo@public.gmane.org>
2018-04-12 17:21 ` Samuel Li
[not found] ` <20bdb4b1-1986-22e4-b0d1-1713b8794f83-5C7GfCeVMHo@public.gmane.org>
2018-04-12 18:00 ` Christian König
[not found] ` <de4659b0-918a-e548-d6ee-12f3a9e88df5-5C7GfCeVMHo@public.gmane.org>
2018-04-12 18:07 ` Samuel Li
[not found] ` <277e44be-d618-ad29-06d2-ea12466571c9-5C7GfCeVMHo@public.gmane.org>
2018-04-12 18:47 ` Christian König
[not found] ` <18325399-d78e-2442-ccf4-55e30d9f5c52-5C7GfCeVMHo@public.gmane.org>
2018-04-12 20:01 ` Samuel Li
[not found] ` <b461df89-0638-77a7-409a-346c64521a40-5C7GfCeVMHo@public.gmane.org>
2018-04-12 20:13 ` Christian König
[not found] ` <5f492910-cac3-643f-efec-472f791e551e-5C7GfCeVMHo@public.gmane.org>
2018-04-12 20:42 ` Samuel Li
[not found] ` <83a2320e-74a9-4539-5b85-0e80a35525fb-5C7GfCeVMHo@public.gmane.org>
2018-04-12 22:48 ` 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.