[PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Samuel Li]

It's enabled by default. -1 is auto, to allow both vram and gtt
memory be available, for testing purpose only.
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h         | 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c     | 4 ++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c      | 5 +++++
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 292c7e7..6b0ee34 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -129,6 +129,7 @@ extern int amdgpu_lbpw;
 extern int amdgpu_compute_multipipe;
 extern int amdgpu_gpu_recovery;
 extern int amdgpu_emu_mode;
+extern int amdgpu_sg_display;
 
 #ifdef CONFIG_DRM_AMDGPU_SI
 extern int amdgpu_si_support;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 5495b29..dfa11b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -513,8 +513,13 @@ uint32_t amdgpu_display_framebuffer_domains(struct amdgpu_device *adev)
 #if defined(CONFIG_DRM_AMD_DC)
 	if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type < CHIP_RAVEN &&
 	    adev->flags & AMD_IS_APU &&
-	    amdgpu_device_asic_has_dc_support(adev->asic_type))
-		domain |= AMDGPU_GEM_DOMAIN_GTT;
+	    amdgpu_device_asic_has_dc_support(adev->asic_type)) {
+		if (amdgpu_sg_display == 1) {
+			domain = AMDGPU_GEM_DOMAIN_GTT;
+		} else if (amdgpu_sg_display == -1) {
+			domain |= AMDGPU_GEM_DOMAIN_GTT;
+		}
+	}
 #endif
 
 	return domain;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index e670936..f0ada24 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -132,6 +132,7 @@ int amdgpu_lbpw = -1;
 int amdgpu_compute_multipipe = -1;
 int amdgpu_gpu_recovery = -1; /* auto */
 int amdgpu_emu_mode = 0;
+int amdgpu_sg_display = 1;
 
 MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in megabytes");
 module_param_named(vramlimit, amdgpu_vram_limit, int, 0600);
@@ -290,6 +291,9 @@ module_param_named(gpu_recovery, amdgpu_gpu_recovery, int, 0444);
 MODULE_PARM_DESC(emu_mode, "Emulation mode, (1 = enable, 0 = disable)");
 module_param_named(emu_mode, amdgpu_emu_mode, int, 0444);
 
+MODULE_PARM_DESC(sg_display, "Enable scatter gather display, (1 = enable, 0 = disable, -1 = auto");
+module_param_named(sg_display, amdgpu_sg_display, int, 0444);
+
 #ifdef CONFIG_DRM_AMDGPU_SI
 
 #if defined(CONFIG_DRM_RADEON) || defined(CONFIG_DRM_RADEON_MODULE)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index 1206301..10f1f4f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -138,6 +138,11 @@ static int amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
 	mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
 						  fb_tiled);
 	domain = amdgpu_display_framebuffer_domains(adev);
+	if (domain == AMDGPU_GEM_DOMAIN_GTT) {
+		DRM_INFO("Scatter gather display: enabled\n");
+	} else if (domain & AMDGPU_GEM_DOMAIN_GTT) {
+		DRM_INFO("Scatter gather display: auto\n");
+	}
 
 	height = ALIGN(mode_cmd->height, 8);
 	size = mode_cmd->pitches[0] * height;
-- 
2.7.4

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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

Am 03.03.2018 um 00:25 schrieb Samuel Li:
> It's enabled by default. -1 is auto, to allow both vram and gtt
> memory be available, for testing purpose only.
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h         | 1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c     | 4 ++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c      | 5 +++++
>   4 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 292c7e7..6b0ee34 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -129,6 +129,7 @@ extern int amdgpu_lbpw;
>   extern int amdgpu_compute_multipipe;
>   extern int amdgpu_gpu_recovery;
>   extern int amdgpu_emu_mode;
> +extern int amdgpu_sg_display;
>   
>   #ifdef CONFIG_DRM_AMDGPU_SI
>   extern int amdgpu_si_support;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index 5495b29..dfa11b1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -513,8 +513,13 @@ uint32_t amdgpu_display_framebuffer_domains(struct amdgpu_device *adev)
>   #if defined(CONFIG_DRM_AMD_DC)
>   	if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type < CHIP_RAVEN &&
>   	    adev->flags & AMD_IS_APU &&
> -	    amdgpu_device_asic_has_dc_support(adev->asic_type))
> -		domain |= AMDGPU_GEM_DOMAIN_GTT;
> +	    amdgpu_device_asic_has_dc_support(adev->asic_type)) {
> +		if (amdgpu_sg_display == 1) {
> +			domain = AMDGPU_GEM_DOMAIN_GTT;
> +		} else if (amdgpu_sg_display == -1) {
> +			domain |= AMDGPU_GEM_DOMAIN_GTT;
> +		}

Coding style, that if shouldn't use "{" and "}".

> +	}
>   #endif
>   
>   	return domain;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index e670936..f0ada24 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -132,6 +132,7 @@ int amdgpu_lbpw = -1;
>   int amdgpu_compute_multipipe = -1;
>   int amdgpu_gpu_recovery = -1; /* auto */
>   int amdgpu_emu_mode = 0;
> +int amdgpu_sg_display = 1;
>   
>   MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in megabytes");
>   module_param_named(vramlimit, amdgpu_vram_limit, int, 0600);
> @@ -290,6 +291,9 @@ module_param_named(gpu_recovery, amdgpu_gpu_recovery, int, 0444);
>   MODULE_PARM_DESC(emu_mode, "Emulation mode, (1 = enable, 0 = disable)");
>   module_param_named(emu_mode, amdgpu_emu_mode, int, 0444);
>   
> +MODULE_PARM_DESC(sg_display, "Enable scatter gather display, (1 = enable, 0 = disable, -1 = auto");
> +module_param_named(sg_display, amdgpu_sg_display, int, 0444);
> +
>   #ifdef CONFIG_DRM_AMDGPU_SI
>   
>   #if defined(CONFIG_DRM_RADEON) || defined(CONFIG_DRM_RADEON_MODULE)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> index 1206301..10f1f4f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> @@ -138,6 +138,11 @@ static int amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
>   	mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
>   						  fb_tiled);
>   	domain = amdgpu_display_framebuffer_domains(adev);
> +	if (domain == AMDGPU_GEM_DOMAIN_GTT) {
> +		DRM_INFO("Scatter gather display: enabled\n");
> +	} else if (domain & AMDGPU_GEM_DOMAIN_GTT) {
> +		DRM_INFO("Scatter gather display: auto\n");
> +	}

Dito and printing that here is not a good idea as far as I can see.

Christian.

>   
>   	height = ALIGN(mode_cmd->height, 8);
>   	size = mode_cmd->pitches[0] * height;

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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Michel Dänzer]

On 2018-03-03 12:25 AM, Samuel Li wrote:
> It's enabled by default. -1 is auto, to allow both vram and gtt
> memory be available, for testing purpose only.

Do we really need a module parameter for this? There's already too many
of them. The driver should know which to use in which cases, and
developers can easily tweak the code. Users should never need to change
this.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Samuel Li]

There are three major options when SG capability is enabled,
1) Allocate everything when possible from GTT memory
2) Allocate everything when possible from VRAM
3) Allow both VRAM/GTT to be available
Each has its own pros/cons, and it has not been decided which direction is going to be used for all PCs. So it seems beneficial to provide all the options.

Sam

On 2018-03-05 05:17 AM, Michel Dänzer wrote:
> On 2018-03-03 12:25 AM, Samuel Li wrote:
>> It's enabled by default. -1 is auto, to allow both vram and gtt
>> memory be available, for testing purpose only.
> 
> Do we really need a module parameter for this? There's already too many
> of them. The driver should know which to use in which cases, and
> developers can easily tweak the code. Users should never need to change
> this.
> 
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Samuel Li]

>>       domain = amdgpu_display_framebuffer_domains(adev);
>> +    if (domain == AMDGPU_GEM_DOMAIN_GTT) {
>> +        DRM_INFO("Scatter gather display: enabled\n");
>> +    } else if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>> +        DRM_INFO("Scatter gather display: auto\n");
>> +    }
> 
> Dito and printing that here is not a good idea as far as I can see.
> 
> Christian.


The intention here is to print out when fb is created. You have other suggestions?

Sam


On 2018-03-03 08:41 AM, Christian König wrote:
> Am 03.03.2018 um 00:25 schrieb Samuel Li:
>> It's enabled by default. -1 is auto, to allow both vram and gtt
>> memory be available, for testing purpose only.
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h         | 1 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++--
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c     | 4 ++++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c      | 5 +++++
>>   4 files changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 292c7e7..6b0ee34 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -129,6 +129,7 @@ extern int amdgpu_lbpw;
>>   extern int amdgpu_compute_multipipe;
>>   extern int amdgpu_gpu_recovery;
>>   extern int amdgpu_emu_mode;
>> +extern int amdgpu_sg_display;
>>     #ifdef CONFIG_DRM_AMDGPU_SI
>>   extern int amdgpu_si_support;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> index 5495b29..dfa11b1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> @@ -513,8 +513,13 @@ uint32_t amdgpu_display_framebuffer_domains(struct amdgpu_device *adev)
>>   #if defined(CONFIG_DRM_AMD_DC)
>>       if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type < CHIP_RAVEN &&
>>           adev->flags & AMD_IS_APU &&
>> -        amdgpu_device_asic_has_dc_support(adev->asic_type))
>> -        domain |= AMDGPU_GEM_DOMAIN_GTT;
>> +        amdgpu_device_asic_has_dc_support(adev->asic_type)) {
>> +        if (amdgpu_sg_display == 1) {
>> +            domain = AMDGPU_GEM_DOMAIN_GTT;
>> +        } else if (amdgpu_sg_display == -1) {
>> +            domain |= AMDGPU_GEM_DOMAIN_GTT;
>> +        }
> 
> Coding style, that if shouldn't use "{" and "}".
> 
>> +    }
>>   #endif
>>         return domain;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index e670936..f0ada24 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -132,6 +132,7 @@ int amdgpu_lbpw = -1;
>>   int amdgpu_compute_multipipe = -1;
>>   int amdgpu_gpu_recovery = -1; /* auto */
>>   int amdgpu_emu_mode = 0;
>> +int amdgpu_sg_display = 1;
>>     MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in megabytes");
>>   module_param_named(vramlimit, amdgpu_vram_limit, int, 0600);
>> @@ -290,6 +291,9 @@ module_param_named(gpu_recovery, amdgpu_gpu_recovery, int, 0444);
>>   MODULE_PARM_DESC(emu_mode, "Emulation mode, (1 = enable, 0 = disable)");
>>   module_param_named(emu_mode, amdgpu_emu_mode, int, 0444);
>>   +MODULE_PARM_DESC(sg_display, "Enable scatter gather display, (1 = enable, 0 = disable, -1 = auto");
>> +module_param_named(sg_display, amdgpu_sg_display, int, 0444);
>> +
>>   #ifdef CONFIG_DRM_AMDGPU_SI
>>     #if defined(CONFIG_DRM_RADEON) || defined(CONFIG_DRM_RADEON_MODULE)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>> index 1206301..10f1f4f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>> @@ -138,6 +138,11 @@ static int amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
>>       mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
>>                             fb_tiled);
>>       domain = amdgpu_display_framebuffer_domains(adev);
>> +    if (domain == AMDGPU_GEM_DOMAIN_GTT) {
>> +        DRM_INFO("Scatter gather display: enabled\n");
>> +    } else if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>> +        DRM_INFO("Scatter gather display: auto\n");
>> +    }
> 
> Dito and printing that here is not a good idea as far as I can see.
> 
> Christian.
> 
>>         height = ALIGN(mode_cmd->height, 8);
>>       size = mode_cmd->pitches[0] * height;
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Alex Deucher]

On Tue, Mar 6, 2018 at 11:49 AM, Samuel Li <samuel.li@amd.com> wrote:
>>>       domain = amdgpu_display_framebuffer_domains(adev);
>>> +    if (domain == AMDGPU_GEM_DOMAIN_GTT) {
>>> +        DRM_INFO("Scatter gather display: enabled\n");
>>> +    } else if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>>> +        DRM_INFO("Scatter gather display: auto\n");
>>> +    }
>>
>> Dito and printing that here is not a good idea as far as I can see.
>>
>> Christian.
>
>
> The intention here is to print out when fb is created. You have other suggestions?

Make it DRM_DEBUG?  otherwise you'll spam the logs.

>
> Sam
>
>
> On 2018-03-03 08:41 AM, Christian König wrote:
>> Am 03.03.2018 um 00:25 schrieb Samuel Li:
>>> It's enabled by default. -1 is auto, to allow both vram and gtt
>>> memory be available, for testing purpose only.
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h         | 1 +
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++--
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c     | 4 ++++
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c      | 5 +++++
>>>   4 files changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> index 292c7e7..6b0ee34 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> @@ -129,6 +129,7 @@ extern int amdgpu_lbpw;
>>>   extern int amdgpu_compute_multipipe;
>>>   extern int amdgpu_gpu_recovery;
>>>   extern int amdgpu_emu_mode;
>>> +extern int amdgpu_sg_display;
>>>     #ifdef CONFIG_DRM_AMDGPU_SI
>>>   extern int amdgpu_si_support;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>> index 5495b29..dfa11b1 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>> @@ -513,8 +513,13 @@ uint32_t amdgpu_display_framebuffer_domains(struct amdgpu_device *adev)
>>>   #if defined(CONFIG_DRM_AMD_DC)
>>>       if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type < CHIP_RAVEN &&
>>>           adev->flags & AMD_IS_APU &&
>>> -        amdgpu_device_asic_has_dc_support(adev->asic_type))
>>> -        domain |= AMDGPU_GEM_DOMAIN_GTT;
>>> +        amdgpu_device_asic_has_dc_support(adev->asic_type)) {
>>> +        if (amdgpu_sg_display == 1) {
>>> +            domain = AMDGPU_GEM_DOMAIN_GTT;
>>> +        } else if (amdgpu_sg_display == -1) {
>>> +            domain |= AMDGPU_GEM_DOMAIN_GTT;
>>> +        }
>>
>> Coding style, that if shouldn't use "{" and "}".
>>
>>> +    }
>>>   #endif
>>>         return domain;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> index e670936..f0ada24 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> @@ -132,6 +132,7 @@ int amdgpu_lbpw = -1;
>>>   int amdgpu_compute_multipipe = -1;
>>>   int amdgpu_gpu_recovery = -1; /* auto */
>>>   int amdgpu_emu_mode = 0;
>>> +int amdgpu_sg_display = 1;
>>>     MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in megabytes");
>>>   module_param_named(vramlimit, amdgpu_vram_limit, int, 0600);
>>> @@ -290,6 +291,9 @@ module_param_named(gpu_recovery, amdgpu_gpu_recovery, int, 0444);
>>>   MODULE_PARM_DESC(emu_mode, "Emulation mode, (1 = enable, 0 = disable)");
>>>   module_param_named(emu_mode, amdgpu_emu_mode, int, 0444);
>>>   +MODULE_PARM_DESC(sg_display, "Enable scatter gather display, (1 = enable, 0 = disable, -1 = auto");
>>> +module_param_named(sg_display, amdgpu_sg_display, int, 0444);
>>> +
>>>   #ifdef CONFIG_DRM_AMDGPU_SI
>>>     #if defined(CONFIG_DRM_RADEON) || defined(CONFIG_DRM_RADEON_MODULE)
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>> index 1206301..10f1f4f 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>> @@ -138,6 +138,11 @@ static int amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
>>>       mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
>>>                             fb_tiled);
>>>       domain = amdgpu_display_framebuffer_domains(adev);
>>> +    if (domain == AMDGPU_GEM_DOMAIN_GTT) {
>>> +        DRM_INFO("Scatter gather display: enabled\n");
>>> +    } else if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>>> +        DRM_INFO("Scatter gather display: auto\n");
>>> +    }
>>
>> Dito and printing that here is not a good idea as far as I can see.
>>
>> Christian.
>>
>>>         height = ALIGN(mode_cmd->height, 8);
>>>       size = mode_cmd->pitches[0] * height;
>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Li, Samuel]

This information is kind of important, and it only prints once typically when module is loaded. 

Regards,
Samuel Li


> -----Original Message-----
> From: Alex Deucher [mailto:alexdeucher@gmail.com]
> Sent: Tuesday, March 06, 2018 12:04 PM
> To: Li, Samuel <Samuel.Li@amd.com>
> Cc: Koenig, Christian <Christian.Koenig@amd.com>; amd-gfx list <amd-
> gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
> 
> On Tue, Mar 6, 2018 at 11:49 AM, Samuel Li <samuel.li@amd.com> wrote:
> >>>       domain = amdgpu_display_framebuffer_domains(adev);
> >>> +    if (domain == AMDGPU_GEM_DOMAIN_GTT) {
> >>> +        DRM_INFO("Scatter gather display: enabled\n");
> >>> +    } else if (domain & AMDGPU_GEM_DOMAIN_GTT) {
> >>> +        DRM_INFO("Scatter gather display: auto\n");
> >>> +    }
> >>
> >> Dito and printing that here is not a good idea as far as I can see.
> >>
> >> Christian.
> >
> >
> > The intention here is to print out when fb is created. You have other
> suggestions?
> 
> Make it DRM_DEBUG?  otherwise you'll spam the logs.
> 
> >
> > Sam
> >
> >
> > On 2018-03-03 08:41 AM, Christian König wrote:
> >> Am 03.03.2018 um 00:25 schrieb Samuel Li:
> >>> It's enabled by default. -1 is auto, to allow both vram and gtt
> >>> memory be available, for testing purpose only.
> >>> ---
> >>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h         | 1 +
> >>>   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++--
> >>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c     | 4 ++++
> >>>   drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c      | 5 +++++
> >>>   4 files changed, 17 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> index 292c7e7..6b0ee34 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> @@ -129,6 +129,7 @@ extern int amdgpu_lbpw;
> >>>   extern int amdgpu_compute_multipipe;
> >>>   extern int amdgpu_gpu_recovery;
> >>>   extern int amdgpu_emu_mode;
> >>> +extern int amdgpu_sg_display;
> >>>     #ifdef CONFIG_DRM_AMDGPU_SI
> >>>   extern int amdgpu_si_support;
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> >>> index 5495b29..dfa11b1 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> >>> @@ -513,8 +513,13 @@ uint32_t
> amdgpu_display_framebuffer_domains(struct amdgpu_device *adev)
> >>>   #if defined(CONFIG_DRM_AMD_DC)
> >>>       if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type <
> CHIP_RAVEN &&
> >>>           adev->flags & AMD_IS_APU &&
> >>> -        amdgpu_device_asic_has_dc_support(adev->asic_type))
> >>> -        domain |= AMDGPU_GEM_DOMAIN_GTT;
> >>> +        amdgpu_device_asic_has_dc_support(adev->asic_type)) {
> >>> +        if (amdgpu_sg_display == 1) {
> >>> +            domain = AMDGPU_GEM_DOMAIN_GTT;
> >>> +        } else if (amdgpu_sg_display == -1) {
> >>> +            domain |= AMDGPU_GEM_DOMAIN_GTT;
> >>> +        }
> >>
> >> Coding style, that if shouldn't use "{" and "}".
> >>
> >>> +    }
> >>>   #endif
> >>>         return domain;
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >>> index e670936..f0ada24 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >>> @@ -132,6 +132,7 @@ int amdgpu_lbpw = -1;
> >>>   int amdgpu_compute_multipipe = -1;
> >>>   int amdgpu_gpu_recovery = -1; /* auto */
> >>>   int amdgpu_emu_mode = 0;
> >>> +int amdgpu_sg_display = 1;
> >>>     MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in
> megabytes");
> >>>   module_param_named(vramlimit, amdgpu_vram_limit, int, 0600); @@
> >>> -290,6 +291,9 @@ module_param_named(gpu_recovery,
> amdgpu_gpu_recovery, int, 0444);
> >>>   MODULE_PARM_DESC(emu_mode, "Emulation mode, (1 = enable, 0 =
> disable)");
> >>>   module_param_named(emu_mode, amdgpu_emu_mode, int, 0444);
> >>>   +MODULE_PARM_DESC(sg_display, "Enable scatter gather display, (1 =
> >>> enable, 0 = disable, -1 = auto");
> >>> +module_param_named(sg_display, amdgpu_sg_display, int, 0444);
> >>> +
> >>>   #ifdef CONFIG_DRM_AMDGPU_SI
> >>>     #if defined(CONFIG_DRM_RADEON) ||
> >>> defined(CONFIG_DRM_RADEON_MODULE) diff --git
> >>> a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> >>> index 1206301..10f1f4f 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> >>> @@ -138,6 +138,11 @@ static int
> amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
> >>>       mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd-
> >width, cpp,
> >>>                             fb_tiled);
> >>>       domain = amdgpu_display_framebuffer_domains(adev);
> >>> +    if (domain == AMDGPU_GEM_DOMAIN_GTT) {
> >>> +        DRM_INFO("Scatter gather display: enabled\n");
> >>> +    } else if (domain & AMDGPU_GEM_DOMAIN_GTT) {
> >>> +        DRM_INFO("Scatter gather display: auto\n");
> >>> +    }
> >>
> >> Dito and printing that here is not a good idea as far as I can see.
> >>
> >> Christian.
> >>
> >>>         height = ALIGN(mode_cmd->height, 8);
> >>>       size = mode_cmd->pitches[0] * height;
> >>
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

And exactly that's the problematical assumption.

This doesn't print only when the module is loaded, but rather when a 
framebuffer object is created.

And depending on the use case that can even be many many times per second.

Please remove that printing, addition to that I agree with Michel that 
the module parameter is overkill.

Regards,
Christian.

Am 06.03.2018 um 18:09 schrieb Li, Samuel:
> This information is kind of important, and it only prints once typically when module is loaded.
>
> Regards,
> Samuel Li
>
>
>> -----Original Message-----
>> From: Alex Deucher [mailto:alexdeucher@gmail.com]
>> Sent: Tuesday, March 06, 2018 12:04 PM
>> To: Li, Samuel <Samuel.Li@amd.com>
>> Cc: Koenig, Christian <Christian.Koenig@amd.com>; amd-gfx list <amd-
>> gfx@lists.freedesktop.org>
>> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>>
>> On Tue, Mar 6, 2018 at 11:49 AM, Samuel Li <samuel.li@amd.com> wrote:
>>>>>        domain = amdgpu_display_framebuffer_domains(adev);
>>>>> +    if (domain == AMDGPU_GEM_DOMAIN_GTT) {
>>>>> +        DRM_INFO("Scatter gather display: enabled\n");
>>>>> +    } else if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>>>>> +        DRM_INFO("Scatter gather display: auto\n");
>>>>> +    }
>>>> Dito and printing that here is not a good idea as far as I can see.
>>>>
>>>> Christian.
>>>
>>> The intention here is to print out when fb is created. You have other
>> suggestions?
>>
>> Make it DRM_DEBUG?  otherwise you'll spam the logs.
>>
>>> Sam
>>>
>>>
>>> On 2018-03-03 08:41 AM, Christian König wrote:
>>>> Am 03.03.2018 um 00:25 schrieb Samuel Li:
>>>>> It's enabled by default. -1 is auto, to allow both vram and gtt
>>>>> memory be available, for testing purpose only.
>>>>> ---
>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu.h         | 1 +
>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++++++--
>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c     | 4 ++++
>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c      | 5 +++++
>>>>>    4 files changed, 17 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> index 292c7e7..6b0ee34 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> @@ -129,6 +129,7 @@ extern int amdgpu_lbpw;
>>>>>    extern int amdgpu_compute_multipipe;
>>>>>    extern int amdgpu_gpu_recovery;
>>>>>    extern int amdgpu_emu_mode;
>>>>> +extern int amdgpu_sg_display;
>>>>>      #ifdef CONFIG_DRM_AMDGPU_SI
>>>>>    extern int amdgpu_si_support;
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>>>> index 5495b29..dfa11b1 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>>>> @@ -513,8 +513,13 @@ uint32_t
>> amdgpu_display_framebuffer_domains(struct amdgpu_device *adev)
>>>>>    #if defined(CONFIG_DRM_AMD_DC)
>>>>>        if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type <
>> CHIP_RAVEN &&
>>>>>            adev->flags & AMD_IS_APU &&
>>>>> -        amdgpu_device_asic_has_dc_support(adev->asic_type))
>>>>> -        domain |= AMDGPU_GEM_DOMAIN_GTT;
>>>>> +        amdgpu_device_asic_has_dc_support(adev->asic_type)) {
>>>>> +        if (amdgpu_sg_display == 1) {
>>>>> +            domain = AMDGPU_GEM_DOMAIN_GTT;
>>>>> +        } else if (amdgpu_sg_display == -1) {
>>>>> +            domain |= AMDGPU_GEM_DOMAIN_GTT;
>>>>> +        }
>>>> Coding style, that if shouldn't use "{" and "}".
>>>>
>>>>> +    }
>>>>>    #endif
>>>>>          return domain;
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>>> index e670936..f0ada24 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>>> @@ -132,6 +132,7 @@ int amdgpu_lbpw = -1;
>>>>>    int amdgpu_compute_multipipe = -1;
>>>>>    int amdgpu_gpu_recovery = -1; /* auto */
>>>>>    int amdgpu_emu_mode = 0;
>>>>> +int amdgpu_sg_display = 1;
>>>>>      MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in
>> megabytes");
>>>>>    module_param_named(vramlimit, amdgpu_vram_limit, int, 0600); @@
>>>>> -290,6 +291,9 @@ module_param_named(gpu_recovery,
>> amdgpu_gpu_recovery, int, 0444);
>>>>>    MODULE_PARM_DESC(emu_mode, "Emulation mode, (1 = enable, 0 =
>> disable)");
>>>>>    module_param_named(emu_mode, amdgpu_emu_mode, int, 0444);
>>>>>    +MODULE_PARM_DESC(sg_display, "Enable scatter gather display, (1 =
>>>>> enable, 0 = disable, -1 = auto");
>>>>> +module_param_named(sg_display, amdgpu_sg_display, int, 0444);
>>>>> +
>>>>>    #ifdef CONFIG_DRM_AMDGPU_SI
>>>>>      #if defined(CONFIG_DRM_RADEON) ||
>>>>> defined(CONFIG_DRM_RADEON_MODULE) diff --git
>>>>> a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>>>> index 1206301..10f1f4f 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>>>> @@ -138,6 +138,11 @@ static int
>> amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
>>>>>        mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd-
>>> width, cpp,
>>>>>                              fb_tiled);
>>>>>        domain = amdgpu_display_framebuffer_domains(adev);
>>>>> +    if (domain == AMDGPU_GEM_DOMAIN_GTT) {
>>>>> +        DRM_INFO("Scatter gather display: enabled\n");
>>>>> +    } else if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>>>>> +        DRM_INFO("Scatter gather display: auto\n");
>>>>> +    }
>>>> Dito and printing that here is not a good idea as far as I can see.
>>>>
>>>> Christian.
>>>>
>>>>>          height = ALIGN(mode_cmd->height, 8);
>>>>>        size = mode_cmd->pitches[0] * height;
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Li, Samuel]

> And exactly that's the problematical assumption.
Not assumption, I tested. You have any typical use case that it might become a problem?

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

Regards,
Samuel Li


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

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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

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

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

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

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

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

Regards,
Christian.

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

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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Michel Dänzer]

On 2018-03-06 06:44 PM, Christian König wrote:
> Am 06.03.2018 um 18:22 schrieb Li, Samuel:
>>
>>> addition to that I agree with Michel that the module parameter is
>>> overkill.
>> That I already explained. Currently SG display feature needs to
>> provide options for all kinds of use cases. All amd drivers so far
>> provides options, and the default configuration is actually to
>> allocate everything from GTT when possible.
> 
> That isn't job of the kernel to have this parameter. Saying that we
> should probably add a DDX and/or environment option to control that.

Why would we need that? It's the kernel driver's job to handle this as
needed.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

Am 06.03.2018 um 18:51 schrieb Michel Dänzer:
> On 2018-03-06 06:44 PM, Christian König wrote:
>> Am 06.03.2018 um 18:22 schrieb Li, Samuel:
>>>> addition to that I agree with Michel that the module parameter is
>>>> overkill.
>>> That I already explained. Currently SG display feature needs to
>>> provide options for all kinds of use cases. All amd drivers so far
>>> provides options, and the default configuration is actually to
>>> allocate everything from GTT when possible.
>> That isn't job of the kernel to have this parameter. Saying that we
>> should probably add a DDX and/or environment option to control that.
> Why would we need that? It's the kernel driver's job to handle this as
> needed.

Buffer placement is specified by the DDX/Mesa, when we override that in 
the kernel we just force unnecessary buffer moves.

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

RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Li, Samuel]

> Buffer placement is specified by the DDX/Mesa, when we override that in the kernel we just force unnecessary buffer moves.
Since scan out buffer is moved to GTT now, DDX/Mesa might need to make some changes for this too.

Regards,
Samuel Li


> -----Original Message-----
> From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com]
> Sent: Tuesday, March 06, 2018 1:02 PM
> To: Michel Dänzer <michel@daenzer.net>; Koenig, Christian
> <Christian.Koenig@amd.com>; Li, Samuel <Samuel.Li@amd.com>; Alex
> Deucher <alexdeucher@gmail.com>
> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
> 
> Am 06.03.2018 um 18:51 schrieb Michel Dänzer:
> > On 2018-03-06 06:44 PM, Christian König wrote:
> >> Am 06.03.2018 um 18:22 schrieb Li, Samuel:
> >>>> addition to that I agree with Michel that the module parameter is
> >>>> overkill.
> >>> That I already explained. Currently SG display feature needs to
> >>> provide options for all kinds of use cases. All amd drivers so far
> >>> provides options, and the default configuration is actually to
> >>> allocate everything from GTT when possible.
> >> That isn't job of the kernel to have this parameter. Saying that we
> >> should probably add a DDX and/or environment option to control that.
> > Why would we need that? It's the kernel driver's job to handle this as
> > needed.
> 
> Buffer placement is specified by the DDX/Mesa, when we override that in
> the kernel we just force unnecessary buffer moves.
> 
> Regards,
> Christian.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Michel Dänzer]

On 2018-03-06 07:02 PM, Christian König wrote:
> Am 06.03.2018 um 18:51 schrieb Michel Dänzer:
>> On 2018-03-06 06:44 PM, Christian König wrote:
>>> Am 06.03.2018 um 18:22 schrieb Li, Samuel:
>>>>> addition to that I agree with Michel that the module parameter is
>>>>> overkill.
>>>> That I already explained. Currently SG display feature needs to
>>>> provide options for all kinds of use cases. All amd drivers so far
>>>> provides options, and the default configuration is actually to
>>>> allocate everything from GTT when possible.
>>> That isn't job of the kernel to have this parameter. Saying that we
>>> should probably add a DDX and/or environment option to control that.
>> Why would we need that? It's the kernel driver's job to handle this as
>> needed.
> 
> Buffer placement is specified by the DDX/Mesa, when we override that in
> the kernel we just force unnecessary buffer moves.

Userspace just needs a way to know which domains can/should be used for
scanout buffers, e.g. via an info ioctl.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

Am 06.03.2018 um 19:15 schrieb Michel Dänzer:
> On 2018-03-06 07:02 PM, Christian König wrote:
>> Am 06.03.2018 um 18:51 schrieb Michel Dänzer:
>>> On 2018-03-06 06:44 PM, Christian König wrote:
>>>> Am 06.03.2018 um 18:22 schrieb Li, Samuel:
>>>>>> addition to that I agree with Michel that the module parameter is
>>>>>> overkill.
>>>>> That I already explained. Currently SG display feature needs to
>>>>> provide options for all kinds of use cases. All amd drivers so far
>>>>> provides options, and the default configuration is actually to
>>>>> allocate everything from GTT when possible.
>>>> That isn't job of the kernel to have this parameter. Saying that we
>>>> should probably add a DDX and/or environment option to control that.
>>> Why would we need that? It's the kernel driver's job to handle this as
>>> needed.
>> Buffer placement is specified by the DDX/Mesa, when we override that in
>> the kernel we just force unnecessary buffer moves.
> Userspace just needs a way to know which domains can/should be used for
> scanout buffers, e.g. via an info ioctl.

Yeah, but why shouldn't they then make the decision where to place it as 
well?

See when we enforce a certain placement in the kernel we will just get 
an unnecessary buffer move with old user space components.

In other words the kernel just needs to advertise that the hw/sw 
combination allows scanout from GTT as well and then an updated 
userspace can actually make use of that placement or decide to not use 
it in certain situations. E.g. the last time I tested it placing things 
into GTT still resulted in quite a performance penalty for rendering.

The major use case I can currently see are A+A laptop, cause there it 
can avoid the additional buffer move from GTT to VRAM for the APU.

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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Michel Dänzer]

On 2018-03-06 07:23 PM, Christian König wrote:
> Am 06.03.2018 um 19:15 schrieb Michel Dänzer:
>> On 2018-03-06 07:02 PM, Christian König wrote:
>>> Am 06.03.2018 um 18:51 schrieb Michel Dänzer:
>>>> On 2018-03-06 06:44 PM, Christian König wrote:
>>>>> Am 06.03.2018 um 18:22 schrieb Li, Samuel:
>>>>>>> addition to that I agree with Michel that the module parameter is
>>>>>>> overkill.
>>>>>> That I already explained. Currently SG display feature needs to
>>>>>> provide options for all kinds of use cases. All amd drivers so far
>>>>>> provides options, and the default configuration is actually to
>>>>>> allocate everything from GTT when possible.
>>>>> That isn't job of the kernel to have this parameter. Saying that we
>>>>> should probably add a DDX and/or environment option to control that.
>>>> Why would we need that? It's the kernel driver's job to handle this as
>>>> needed.
>>> Buffer placement is specified by the DDX/Mesa, when we override that in
>>> the kernel we just force unnecessary buffer moves.
>> Userspace just needs a way to know which domains can/should be used for
>> scanout buffers, e.g. via an info ioctl.
> 
> Yeah, but why shouldn't they then make the decision where to place it as
> well?
> 
> See when we enforce a certain placement in the kernel we will just get
> an unnecessary buffer move with old user space components.
> 
> In other words the kernel just needs to advertise that the hw/sw
> combination allows scanout from GTT as well and then an updated
> userspace can actually make use of that placement or decide to not use
> it in certain situations.

I think we'll need to always pin BOs to GTT for scanout in some cases,
because some hardware has issues when transitioning between scanout from
VRAM and GTT.

How about:

Userspace can query from the kernel which domain(s) can be used for
scanout: only VRAM / VRAM or GTT / only GTT. Userspace sets the allowed
domains correspondingly for scanout BOs.

If you can think of a scenario where this won't work, please
specifically describe it and how you propose addressing it.


> E.g. the last time I tested it placing things into GTT still resulted
> in quite a performance penalty for rendering.

FWIW, I think the penalty is most likely IOMMU related. Last time I
tested, I couldn't measure a big difference with IOMMU disabled.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

Am 07.03.2018 um 09:42 schrieb Michel Dänzer:
> On 2018-03-06 07:23 PM, Christian König wrote:
>> Am 06.03.2018 um 19:15 schrieb Michel Dänzer:
>>> On 2018-03-06 07:02 PM, Christian König wrote:
>>>> Am 06.03.2018 um 18:51 schrieb Michel Dänzer:
>>>>> On 2018-03-06 06:44 PM, Christian König wrote:
>>>>>> Am 06.03.2018 um 18:22 schrieb Li, Samuel:
>>>>>>>> addition to that I agree with Michel that the module parameter is
>>>>>>>> overkill.
>>>>>>> That I already explained. Currently SG display feature needs to
>>>>>>> provide options for all kinds of use cases. All amd drivers so far
>>>>>>> provides options, and the default configuration is actually to
>>>>>>> allocate everything from GTT when possible.
>>>>>> That isn't job of the kernel to have this parameter. Saying that we
>>>>>> should probably add a DDX and/or environment option to control that.
>>>>> Why would we need that? It's the kernel driver's job to handle this as
>>>>> needed.
>>>> Buffer placement is specified by the DDX/Mesa, when we override that in
>>>> the kernel we just force unnecessary buffer moves.
>>> Userspace just needs a way to know which domains can/should be used for
>>> scanout buffers, e.g. via an info ioctl.
>> Yeah, but why shouldn't they then make the decision where to place it as
>> well?
>>
>> See when we enforce a certain placement in the kernel we will just get
>> an unnecessary buffer move with old user space components.
>>
>> In other words the kernel just needs to advertise that the hw/sw
>> combination allows scanout from GTT as well and then an updated
>> userspace can actually make use of that placement or decide to not use
>> it in certain situations.
> I think we'll need to always pin BOs to GTT for scanout in some cases,
> because some hardware has issues when transitioning between scanout from
> VRAM and GTT.
>
> How about:
>
> Userspace can query from the kernel which domain(s) can be used for
> scanout: only VRAM / VRAM or GTT / only GTT. Userspace sets the allowed
> domains correspondingly for scanout BOs.

Well that sounds close to what I had in mind as well, we just can't 
specify only GTT when we want to keep backward compatibility with 
existing user space.

>
> If you can think of a scenario where this won't work, please
> specifically describe it and how you propose addressing it.
>
>
>> E.g. the last time I tested it placing things into GTT still resulted
>> in quite a performance penalty for rendering.
> FWIW, I think the penalty is most likely IOMMU related. Last time I
> tested, I couldn't measure a big difference with IOMMU disabled.

No, the penalty I'm talking about came from the ping/pong we did with 
the scanout buffers.

See when I tested this the DDX and Mesa where unmodified, so both still 
assumed VRAM as placement for scanout BOs, but the kernel forced scanout 
BOs into GTT for testing.

So what happened was that on scanout we moved the VRAM BO to GTT and 
after unpinning it on the first command submission which used the BO we 
moved it back to VRAM again.

So as long as we don't want a severe performance penalty when you 
combine old userspace with new kernel using a kernel parameter to force 
scanout from GTT is not possible.

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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Michel Dänzer]

On 2018-03-07 10:47 AM, Christian König wrote:
> 
> See when I tested this the DDX and Mesa where unmodified, so both still
> assumed VRAM as placement for scanout BOs, but the kernel forced scanout
> BOs into GTT for testing.
> 
> So what happened was that on scanout we moved the VRAM BO to GTT and
> after unpinning it on the first command submission which used the BO we
> moved it back to VRAM again.
> 
> So as long as we don't want a severe performance penalty when you
> combine old userspace with new kernel using a kernel parameter to force
> scanout from GTT is not possible.

That means we either need to come up with a different workaround for
hardware issues transitioning between scanout from VRAM and GTT, or we
can't scan out from GTT in that case.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

Am 07.03.2018 um 10:53 schrieb Michel Dänzer:
> On 2018-03-07 10:47 AM, Christian König wrote:
>> See when I tested this the DDX and Mesa where unmodified, so both still
>> assumed VRAM as placement for scanout BOs, but the kernel forced scanout
>> BOs into GTT for testing.
>>
>> So what happened was that on scanout we moved the VRAM BO to GTT and
>> after unpinning it on the first command submission which used the BO we
>> moved it back to VRAM again.
>>
>> So as long as we don't want a severe performance penalty when you
>> combine old userspace with new kernel using a kernel parameter to force
>> scanout from GTT is not possible.
> That means we either need to come up with a different workaround for
> hardware issues transitioning between scanout from VRAM and GTT, or we
> can't scan out from GTT in that case.

What exactly was the problem with the transition from VRAM to GTT?

I mean what we can rather easily do is check where the existing BO is 
pinned and then always pin into the same domain on page flip.

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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Michel Dänzer]

On 2018-03-07 11:04 AM, Christian König wrote:
> Am 07.03.2018 um 10:53 schrieb Michel Dänzer:
>> On 2018-03-07 10:47 AM, Christian König wrote:
>>> See when I tested this the DDX and Mesa where unmodified, so both still
>>> assumed VRAM as placement for scanout BOs, but the kernel forced scanout
>>> BOs into GTT for testing.
>>>
>>> So what happened was that on scanout we moved the VRAM BO to GTT and
>>> after unpinning it on the first command submission which used the BO we
>>> moved it back to VRAM again.
>>>
>>> So as long as we don't want a severe performance penalty when you
>>> combine old userspace with new kernel using a kernel parameter to force
>>> scanout from GTT is not possible.
>> That means we either need to come up with a different workaround for
>> hardware issues transitioning between scanout from VRAM and GTT, or we
>> can't scan out from GTT in that case.
> 
> What exactly was the problem with the transition from VRAM to GTT?
> 
> I mean what we can rather easily do is check where the existing BO is
> pinned and then always pin into the same domain on page flip.

Yeah, something like that could work. In which case, all that's needed
is a way for userspace to know that GTT scanout can work, right? With
that information, it can manage the domains of scanout BOs as it wants.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Samuel Li]

Why so complicated? If old user space compatibility is required, just use sg_display=0.

Sam


On 2018-03-07 05:12 AM, Michel Dänzer wrote:
> On 2018-03-07 11:04 AM, Christian König wrote:
>> Am 07.03.2018 um 10:53 schrieb Michel Dänzer:
>>> On 2018-03-07 10:47 AM, Christian König wrote:
>>>> See when I tested this the DDX and Mesa where unmodified, so both still
>>>> assumed VRAM as placement for scanout BOs, but the kernel forced scanout
>>>> BOs into GTT for testing.
>>>>
>>>> So what happened was that on scanout we moved the VRAM BO to GTT and
>>>> after unpinning it on the first command submission which used the BO we
>>>> moved it back to VRAM again.
>>>>
>>>> So as long as we don't want a severe performance penalty when you
>>>> combine old userspace with new kernel using a kernel parameter to force
>>>> scanout from GTT is not possible.
>>> That means we either need to come up with a different workaround for
>>> hardware issues transitioning between scanout from VRAM and GTT, or we
>>> can't scan out from GTT in that case.
>>
>> What exactly was the problem with the transition from VRAM to GTT?
>>
>> I mean what we can rather easily do is check where the existing BO is
>> pinned and then always pin into the same domain on page flip.
> 
> Yeah, something like that could work. In which case, all that's needed
> is a way for userspace to know that GTT scanout can work, right? With
> that information, it can manage the domains of scanout BOs as it wants.
> 
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Alex Deucher]

On Wed, Mar 7, 2018 at 12:29 PM, Samuel Li <samuel.li@amd.com> wrote:
>
> Why so complicated? If old user space compatibility is required, just use sg_display=0.

It will always just work in that case and we can adjust for the
optimal scenario by default in all cases without requiring the user to
set misc parameters to tune their setup that may break in the future
or hide bugs because a user sets it and forgets it.

Alex

>
> Sam
>
>
> On 2018-03-07 05:12 AM, Michel Dänzer wrote:
>> On 2018-03-07 11:04 AM, Christian König wrote:
>>> Am 07.03.2018 um 10:53 schrieb Michel Dänzer:
>>>> On 2018-03-07 10:47 AM, Christian König wrote:
>>>>> See when I tested this the DDX and Mesa where unmodified, so both still
>>>>> assumed VRAM as placement for scanout BOs, but the kernel forced scanout
>>>>> BOs into GTT for testing.
>>>>>
>>>>> So what happened was that on scanout we moved the VRAM BO to GTT and
>>>>> after unpinning it on the first command submission which used the BO we
>>>>> moved it back to VRAM again.
>>>>>
>>>>> So as long as we don't want a severe performance penalty when you
>>>>> combine old userspace with new kernel using a kernel parameter to force
>>>>> scanout from GTT is not possible.
>>>> That means we either need to come up with a different workaround for
>>>> hardware issues transitioning between scanout from VRAM and GTT, or we
>>>> can't scan out from GTT in that case.
>>>
>>> What exactly was the problem with the transition from VRAM to GTT?
>>>
>>> I mean what we can rather easily do is check where the existing BO is
>>> pinned and then always pin into the same domain on page flip.
>>
>> Yeah, something like that could work. In which case, all that's needed
>> is a way for userspace to know that GTT scanout can work, right? With
>> that information, it can manage the domains of scanout BOs as it wants.
>>
>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Michel Dänzer]

On 2018-03-07 06:38 PM, Alex Deucher wrote:
> On Wed, Mar 7, 2018 at 12:29 PM, Samuel Li <samuel.li@amd.com> wrote:
>>
>> Why so complicated? If old user space compatibility is required, just use sg_display=0.
> 
> It will always just work in that case and we can adjust for the
> optimal scenario by default in all cases without requiring the user to
> set misc parameters to tune their setup that may break in the future
> or hide bugs because a user sets it and forgets it.

Right. I don't agree it's all that complicated anyway, we do this sort
of thing all the time, it also makes our lives easier down the road.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Samuel Li]

I think it's not useful though. Think about that, SG display feature is intended to use as less VRAM as possible. Will someone want a display buffer sometimes VRAM, sometimes GTT?
Hardly a case to me, and I think it's a waste of effort. That also might explain no driver does that now.

Sam



On 2018-03-07 12:50 PM, Michel Dänzer wrote:
> On 2018-03-07 06:38 PM, Alex Deucher wrote:
>> On Wed, Mar 7, 2018 at 12:29 PM, Samuel Li <samuel.li@amd.com> wrote:
>>>
>>> Why so complicated? If old user space compatibility is required, just use sg_display=0.
>>
>> It will always just work in that case and we can adjust for the
>> optimal scenario by default in all cases without requiring the user to
>> set misc parameters to tune their setup that may break in the future
>> or hide bugs because a user sets it and forgets it.
> 
> Right. I don't agree it's all that complicated anyway, we do this sort
> of thing all the time, it also makes our lives easier down the road.
> 
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Alex Deucher]

On Wed, Mar 7, 2018 at 1:18 PM, Samuel Li <samuel.li@amd.com> wrote:
> I think it's not useful though. Think about that, SG display feature is intended to use as less VRAM as possible. Will someone want a display buffer sometimes VRAM, sometimes GTT?
> Hardly a case to me, and I think it's a waste of effort. That also might explain no driver does that now.

You might want different strategies depending on how much VRAM you
have.  If you have a bunch and it performs better, prefer it.  If you
have limited VRAM, you might want to prefer GTT.  You might also want
to prefer VRAM, but also allow buffers to fall back to GTT if
necessary.  This would make the logic dynamic and all in one place.
The kernel can advertise what asics support scanout from system ram
and then UMDs can just query that to choose placements rather than
adding hardcoded logic for specific asics.

Alex

>
> Sam
>
>
>
> On 2018-03-07 12:50 PM, Michel Dänzer wrote:
>> On 2018-03-07 06:38 PM, Alex Deucher wrote:
>>> On Wed, Mar 7, 2018 at 12:29 PM, Samuel Li <samuel.li@amd.com> wrote:
>>>>
>>>> Why so complicated? If old user space compatibility is required, just use sg_display=0.
>>>
>>> It will always just work in that case and we can adjust for the
>>> optimal scenario by default in all cases without requiring the user to
>>> set misc parameters to tune their setup that may break in the future
>>> or hide bugs because a user sets it and forgets it.
>>
>> Right. I don't agree it's all that complicated anyway, we do this sort
>> of thing all the time, it also makes our lives easier down the road.
>>
>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Samuel Li]

> You might also want to prefer VRAM, but also allow buffers to fall back to GTT if
> necessary.
For display buffer, this case seems not really attractive. When display buffer changes between GTT and VRAM dynamically, our driver needs to adpat too, which is hard to see the benefits and not really worth the effort.

Sam



On 2018-03-07 01:27 PM, Alex Deucher wrote:
> On Wed, Mar 7, 2018 at 1:18 PM, Samuel Li <samuel.li@amd.com> wrote:
>> I think it's not useful though. Think about that, SG display feature is intended to use as less VRAM as possible. Will someone want a display buffer sometimes VRAM, sometimes GTT?
>> Hardly a case to me, and I think it's a waste of effort. That also might explain no driver does that now.
> 
> You might want different strategies depending on how much VRAM you
> have.  If you have a bunch and it performs better, prefer it.  If you
> have limited VRAM, you might want to prefer GTT.  You might also want
> to prefer VRAM, but also allow buffers to fall back to GTT if
> necessary.  This would make the logic dynamic and all in one place.
> The kernel can advertise what asics support scanout from system ram
> and then UMDs can just query that to choose placements rather than
> adding hardcoded logic for specific asics.
> 
> Alex
> 
>>
>> Sam
>>
>>
>>
>> On 2018-03-07 12:50 PM, Michel Dänzer wrote:
>>> On 2018-03-07 06:38 PM, Alex Deucher wrote:
>>>> On Wed, Mar 7, 2018 at 12:29 PM, Samuel Li <samuel.li@amd.com> wrote:
>>>>>
>>>>> Why so complicated? If old user space compatibility is required, just use sg_display=0.
>>>>
>>>> It will always just work in that case and we can adjust for the
>>>> optimal scenario by default in all cases without requiring the user to
>>>> set misc parameters to tune their setup that may break in the future
>>>> or hide bugs because a user sets it and forgets it.
>>>
>>> Right. I don't agree it's all that complicated anyway, we do this sort
>>> of thing all the time, it also makes our lives easier down the road.
>>>
>>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Li, Samuel]

How about that, in my previous patches, it actually allows three scenarios,
1) VRAM as display buffer
2) GTT as display buffer
3) Mixed GTT/display buffer, as you have asked.

I think the first step is to make the 2nd scenario work at optimal level. After that, if anyone wants to work the 3rd scenario, I have no objection at all.

Regards,
Samuel Li


> -----Original Message-----
> From: Samuel Li [mailto:samuel.li@amd.com]
> Sent: Wednesday, March 07, 2018 1:54 PM
> To: Alex Deucher <alexdeucher@gmail.com>
> Cc: Michel Dänzer <michel@daenzer.net>; Koenig, Christian
> <Christian.Koenig@amd.com>; amd-gfx list <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
> 
> > You might also want to prefer VRAM, but also allow buffers to fall
> > back to GTT if necessary.
> For display buffer, this case seems not really attractive. When display buffer
> changes between GTT and VRAM dynamically, our driver needs to adpat too,
> which is hard to see the benefits and not really worth the effort.
> 
> Sam
> 
> 
> 
> On 2018-03-07 01:27 PM, Alex Deucher wrote:
> > On Wed, Mar 7, 2018 at 1:18 PM, Samuel Li <samuel.li@amd.com> wrote:
> >> I think it's not useful though. Think about that, SG display feature is
> intended to use as less VRAM as possible. Will someone want a display
> buffer sometimes VRAM, sometimes GTT?
> >> Hardly a case to me, and I think it's a waste of effort. That also might
> explain no driver does that now.
> >
> > You might want different strategies depending on how much VRAM you
> > have.  If you have a bunch and it performs better, prefer it.  If you
> > have limited VRAM, you might want to prefer GTT.  You might also want
> > to prefer VRAM, but also allow buffers to fall back to GTT if
> > necessary.  This would make the logic dynamic and all in one place.
> > The kernel can advertise what asics support scanout from system ram
> > and then UMDs can just query that to choose placements rather than
> > adding hardcoded logic for specific asics.
> >
> > Alex
> >
> >>
> >> Sam
> >>
> >>
> >>
> >> On 2018-03-07 12:50 PM, Michel Dänzer wrote:
> >>> On 2018-03-07 06:38 PM, Alex Deucher wrote:
> >>>> On Wed, Mar 7, 2018 at 12:29 PM, Samuel Li <samuel.li@amd.com>
> wrote:
> >>>>>
> >>>>> Why so complicated? If old user space compatibility is required, just
> use sg_display=0.
> >>>>
> >>>> It will always just work in that case and we can adjust for the
> >>>> optimal scenario by default in all cases without requiring the user
> >>>> to set misc parameters to tune their setup that may break in the
> >>>> future or hide bugs because a user sets it and forgets it.
> >>>
> >>> Right. I don't agree it's all that complicated anyway, we do this
> >>> sort of thing all the time, it also makes our lives easier down the road.
> >>>
> >>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Alex Deucher]

On Wed, Mar 7, 2018 at 2:02 PM, Li, Samuel <Samuel.Li@amd.com> wrote:
> How about that, in my previous patches, it actually allows three scenarios,
> 1) VRAM as display buffer
> 2) GTT as display buffer
> 3) Mixed GTT/display buffer, as you have asked.
>
> I think the first step is to make the 2nd scenario work at optimal level. After that, if anyone wants to work the 3rd scenario, I have no objection at all.

Regardless of which scenarios we need to support, I think we also need
to really plumb this through to mesa however since user space is who
ultimately requests the location.  Overriding it in the kernel gets
tricky and can lead to ping-ponging as others have noted.  Better to
have user space know what chips support it or not and request display
buffers in GTT or VRAM from the start.

Alex

>
> Regards,
> Samuel Li
>
>
>> -----Original Message-----
>> From: Samuel Li [mailto:samuel.li@amd.com]
>> Sent: Wednesday, March 07, 2018 1:54 PM
>> To: Alex Deucher <alexdeucher@gmail.com>
>> Cc: Michel Dänzer <michel@daenzer.net>; Koenig, Christian
>> <Christian.Koenig@amd.com>; amd-gfx list <amd-gfx@lists.freedesktop.org>
>> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>>
>> > You might also want to prefer VRAM, but also allow buffers to fall
>> > back to GTT if necessary.
>> For display buffer, this case seems not really attractive. When display buffer
>> changes between GTT and VRAM dynamically, our driver needs to adpat too,
>> which is hard to see the benefits and not really worth the effort.
>>
>> Sam
>>
>>
>>
>> On 2018-03-07 01:27 PM, Alex Deucher wrote:
>> > On Wed, Mar 7, 2018 at 1:18 PM, Samuel Li <samuel.li@amd.com> wrote:
>> >> I think it's not useful though. Think about that, SG display feature is
>> intended to use as less VRAM as possible. Will someone want a display
>> buffer sometimes VRAM, sometimes GTT?
>> >> Hardly a case to me, and I think it's a waste of effort. That also might
>> explain no driver does that now.
>> >
>> > You might want different strategies depending on how much VRAM you
>> > have.  If you have a bunch and it performs better, prefer it.  If you
>> > have limited VRAM, you might want to prefer GTT.  You might also want
>> > to prefer VRAM, but also allow buffers to fall back to GTT if
>> > necessary.  This would make the logic dynamic and all in one place.
>> > The kernel can advertise what asics support scanout from system ram
>> > and then UMDs can just query that to choose placements rather than
>> > adding hardcoded logic for specific asics.
>> >
>> > Alex
>> >
>> >>
>> >> Sam
>> >>
>> >>
>> >>
>> >> On 2018-03-07 12:50 PM, Michel Dänzer wrote:
>> >>> On 2018-03-07 06:38 PM, Alex Deucher wrote:
>> >>>> On Wed, Mar 7, 2018 at 12:29 PM, Samuel Li <samuel.li@amd.com>
>> wrote:
>> >>>>>
>> >>>>> Why so complicated? If old user space compatibility is required, just
>> use sg_display=0.
>> >>>>
>> >>>> It will always just work in that case and we can adjust for the
>> >>>> optimal scenario by default in all cases without requiring the user
>> >>>> to set misc parameters to tune their setup that may break in the
>> >>>> future or hide bugs because a user sets it and forgets it.
>> >>>
>> >>> Right. I don't agree it's all that complicated anyway, we do this
>> >>> sort of thing all the time, it also makes our lives easier down the road.
>> >>>
>> >>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Michel Dänzer]

On 2018-03-07 10:47 AM, Christian König wrote:
> Am 07.03.2018 um 09:42 schrieb Michel Dänzer:
>> On 2018-03-06 07:23 PM, Christian König wrote:
>>
>>> E.g. the last time I tested it placing things into GTT still resulted
>>> in quite a performance penalty for rendering.
>> FWIW, I think the penalty is most likely IOMMU related. Last time I
>> tested, I couldn't measure a big difference with IOMMU disabled.
> 
> No, the penalty I'm talking about came from the ping/pong we did with
> the scanout buffers.
> 
> See when I tested this the DDX and Mesa where unmodified, so both still
> assumed VRAM as placement for scanout BOs, but the kernel forced scanout
> BOs into GTT for testing.
> 
> So what happened was that on scanout we moved the VRAM BO to GTT and
> after unpinning it on the first command submission which used the BO we
> moved it back to VRAM again.

In the meantime, I've had the same idea as Marek: Can't the kernel
driver simply change the BO's preferred domain to GTT when scanning out
from it? Then it won't move back to VRAM.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

Am 08.03.2018 um 09:35 schrieb Michel Dänzer:
> On 2018-03-07 10:47 AM, Christian König wrote:
>> Am 07.03.2018 um 09:42 schrieb Michel Dänzer:
>>> On 2018-03-06 07:23 PM, Christian König wrote:
>>>
>>>> E.g. the last time I tested it placing things into GTT still resulted
>>>> in quite a performance penalty for rendering.
>>> FWIW, I think the penalty is most likely IOMMU related. Last time I
>>> tested, I couldn't measure a big difference with IOMMU disabled.
>> No, the penalty I'm talking about came from the ping/pong we did with
>> the scanout buffers.
>>
>> See when I tested this the DDX and Mesa where unmodified, so both still
>> assumed VRAM as placement for scanout BOs, but the kernel forced scanout
>> BOs into GTT for testing.
>>
>> So what happened was that on scanout we moved the VRAM BO to GTT and
>> after unpinning it on the first command submission which used the BO we
>> moved it back to VRAM again.
> In the meantime, I've had the same idea as Marek: Can't the kernel
> driver simply change the BO's preferred domain to GTT when scanning out
> from it? Then it won't move back to VRAM.

Yes, I've considered this as well.

But I think making the decision in Mesa is the cleaner approach.

E.g. so far we only override the placement decision of userspace for two 
reasons:
1. We where running out of memory in VRAM.
2. We have a hardware restriction which makes VRAM usage mandatory.

And even then we never adjust the placement permanently, we just 
temporary moved the buffer where it was needed and moved it back after 
the operation completed.

Additional to that Mesa might want to set even more flags and/or changes 
it's behavior. E.g. use a tilling mode which both importer and export in 
an A+A laptop understands etc...

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

RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Li, Samuel]

I agree with Marek/Michel: since kernel sets the domain before scanning out, it shall update the preferred domain here too.

Regards,
Samuel Li


> -----Original Message-----
> From: Koenig, Christian
> Sent: Thursday, March 08, 2018 4:07 AM
> To: Michel Dänzer <michel@daenzer.net>; Li, Samuel
> <Samuel.Li@amd.com>; Alex Deucher <alexdeucher@gmail.com>
> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
> 
> Am 08.03.2018 um 09:35 schrieb Michel Dänzer:
> > On 2018-03-07 10:47 AM, Christian König wrote:
> >> Am 07.03.2018 um 09:42 schrieb Michel Dänzer:
> >>> On 2018-03-06 07:23 PM, Christian König wrote:
> >>>
> >>>> E.g. the last time I tested it placing things into GTT still
> >>>> resulted in quite a performance penalty for rendering.
> >>> FWIW, I think the penalty is most likely IOMMU related. Last time I
> >>> tested, I couldn't measure a big difference with IOMMU disabled.
> >> No, the penalty I'm talking about came from the ping/pong we did with
> >> the scanout buffers.
> >>
> >> See when I tested this the DDX and Mesa where unmodified, so both
> >> still assumed VRAM as placement for scanout BOs, but the kernel
> >> forced scanout BOs into GTT for testing.
> >>
> >> So what happened was that on scanout we moved the VRAM BO to GTT
> and
> >> after unpinning it on the first command submission which used the BO
> >> we moved it back to VRAM again.
> > In the meantime, I've had the same idea as Marek: Can't the kernel
> > driver simply change the BO's preferred domain to GTT when scanning
> > out from it? Then it won't move back to VRAM.
> 
> Yes, I've considered this as well.
> 
> But I think making the decision in Mesa is the cleaner approach.
> 
> E.g. so far we only override the placement decision of userspace for two
> reasons:
> 1. We where running out of memory in VRAM.
> 2. We have a hardware restriction which makes VRAM usage mandatory.
> 
> And even then we never adjust the placement permanently, we just
> temporary moved the buffer where it was needed and moved it back after
> the operation completed.
> 
> Additional to that Mesa might want to set even more flags and/or changes
> it's behavior. E.g. use a tilling mode which both importer and export in an
> A+A laptop understands etc...
> 
> Regards,
> Christian.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

I think that the consensus with Alex and me is that we should avoid 
exactly that.

Overriding the preferred domain in the kernel is a no-go for that patch 
set, so please implement the discussed changes in Mesa.

Regards,
Christian.

Am 19.03.2018 um 20:22 schrieb Li, Samuel:
> I agree with Marek/Michel: since kernel sets the domain before scanning out, it shall update the preferred domain here too.
>
> Regards,
> Samuel Li
>
>
>> -----Original Message-----
>> From: Koenig, Christian
>> Sent: Thursday, March 08, 2018 4:07 AM
>> To: Michel Dänzer <michel@daenzer.net>; Li, Samuel
>> <Samuel.Li@amd.com>; Alex Deucher <alexdeucher@gmail.com>
>> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
>> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>>
>> Am 08.03.2018 um 09:35 schrieb Michel Dänzer:
>>> On 2018-03-07 10:47 AM, Christian König wrote:
>>>> Am 07.03.2018 um 09:42 schrieb Michel Dänzer:
>>>>> On 2018-03-06 07:23 PM, Christian König wrote:
>>>>>
>>>>>> E.g. the last time I tested it placing things into GTT still
>>>>>> resulted in quite a performance penalty for rendering.
>>>>> FWIW, I think the penalty is most likely IOMMU related. Last time I
>>>>> tested, I couldn't measure a big difference with IOMMU disabled.
>>>> No, the penalty I'm talking about came from the ping/pong we did with
>>>> the scanout buffers.
>>>>
>>>> See when I tested this the DDX and Mesa where unmodified, so both
>>>> still assumed VRAM as placement for scanout BOs, but the kernel
>>>> forced scanout BOs into GTT for testing.
>>>>
>>>> So what happened was that on scanout we moved the VRAM BO to GTT
>> and
>>>> after unpinning it on the first command submission which used the BO
>>>> we moved it back to VRAM again.
>>> In the meantime, I've had the same idea as Marek: Can't the kernel
>>> driver simply change the BO's preferred domain to GTT when scanning
>>> out from it? Then it won't move back to VRAM.
>> Yes, I've considered this as well.
>>
>> But I think making the decision in Mesa is the cleaner approach.
>>
>> E.g. so far we only override the placement decision of userspace for two
>> reasons:
>> 1. We where running out of memory in VRAM.
>> 2. We have a hardware restriction which makes VRAM usage mandatory.
>>
>> And even then we never adjust the placement permanently, we just
>> temporary moved the buffer where it was needed and moved it back after
>> the operation completed.
>>
>> Additional to that Mesa might want to set even more flags and/or changes
>> it's behavior. E.g. use a tilling mode which both importer and export in an
>> A+A laptop understands etc...
>>
>> Regards,
>> Christian.

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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Marek Olšák]

[-- Attachment #1.1: Type: text/plain, Size: 3527 bytes --]

On Mon, Mar 19, 2018 at 3:27 PM, Christian König <christian.koenig@amd.com>
wrote:

> I think that the consensus with Alex and me is that we should avoid
> exactly that.
>
> Overriding the preferred domain in the kernel is a no-go for that patch
> set, so please implement the discussed changes in Mesa.
>

I don't see how Mesa can make a smarter decision than the kernel. If you
overwrite the preferred domain of the buffer in the kernel, there will be
no ping-ponging between domains. Mesa never changes the initial preferred
domain.

Marek



>
> Regards,
> Christian.
>
>
> Am 19.03.2018 um 20:22 schrieb Li, Samuel:
>
>> I agree with Marek/Michel: since kernel sets the domain before scanning
>> out, it shall update the preferred domain here too.
>>
>> Regards,
>> Samuel Li
>>
>>
>> -----Original Message-----
>>> From: Koenig, Christian
>>> Sent: Thursday, March 08, 2018 4:07 AM
>>> To: Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org>; Li, Samuel
>>> <Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Cc: amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
>>> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display
>>> support
>>>
>>> Am 08.03.2018 um 09:35 schrieb Michel Dänzer:
>>>
>>>> On 2018-03-07 10:47 AM, Christian König wrote:
>>>>
>>>>> Am 07.03.2018 um 09:42 schrieb Michel Dänzer:
>>>>>
>>>>>> On 2018-03-06 07:23 PM, Christian König wrote:
>>>>>>
>>>>>> E.g. the last time I tested it placing things into GTT still
>>>>>>> resulted in quite a performance penalty for rendering.
>>>>>>>
>>>>>> FWIW, I think the penalty is most likely IOMMU related. Last time I
>>>>>> tested, I couldn't measure a big difference with IOMMU disabled.
>>>>>>
>>>>> No, the penalty I'm talking about came from the ping/pong we did with
>>>>> the scanout buffers.
>>>>>
>>>>> See when I tested this the DDX and Mesa where unmodified, so both
>>>>> still assumed VRAM as placement for scanout BOs, but the kernel
>>>>> forced scanout BOs into GTT for testing.
>>>>>
>>>>> So what happened was that on scanout we moved the VRAM BO to GTT
>>>>>
>>>> and
>>>
>>>> after unpinning it on the first command submission which used the BO
>>>>> we moved it back to VRAM again.
>>>>>
>>>> In the meantime, I've had the same idea as Marek: Can't the kernel
>>>> driver simply change the BO's preferred domain to GTT when scanning
>>>> out from it? Then it won't move back to VRAM.
>>>>
>>> Yes, I've considered this as well.
>>>
>>> But I think making the decision in Mesa is the cleaner approach.
>>>
>>> E.g. so far we only override the placement decision of userspace for two
>>> reasons:
>>> 1. We where running out of memory in VRAM.
>>> 2. We have a hardware restriction which makes VRAM usage mandatory.
>>>
>>> And even then we never adjust the placement permanently, we just
>>> temporary moved the buffer where it was needed and moved it back after
>>> the operation completed.
>>>
>>> Additional to that Mesa might want to set even more flags and/or changes
>>> it's behavior. E.g. use a tilling mode which both importer and export in
>>> an
>>> A+A laptop understands etc...
>>>
>>> Regards,
>>> Christian.
>>>
>>
> _______________________________________________
> 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: 5568 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 5094 bytes --]

Am 19.03.2018 um 20:39 schrieb Marek Olšák:
> On Mon, Mar 19, 2018 at 3:27 PM, Christian König 
> <christian.koenig-5C7GfCeVMHo@public.gmane.org <mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>> wrote:
>
>     I think that the consensus with Alex and me is that we should
>     avoid exactly that.
>
>     Overriding the preferred domain in the kernel is a no-go for that
>     patch set, so please implement the discussed changes in Mesa.
>
>
> I don't see how Mesa can make a smarter decision than the kernel. If 
> you overwrite the preferred domain of the buffer in the kernel, there 
> will be no ping-ponging between domains. Mesa never changes the 
> initial preferred domain.

Yeah, but it can set the initial domain based on what it knows how the 
buffer will be used.

E.g. when scanout from GTT is supported we would like to always set the 
initial domain as GTT instead of VRAM.

Christian.

>
> Marek
>
>
>     Regards,
>     Christian.
>
>
>     Am 19.03.2018 um 20:22 schrieb Li, Samuel:
>
>         I agree with Marek/Michel: since kernel sets the domain before
>         scanning out, it shall update the preferred domain here too.
>
>         Regards,
>         Samuel Li
>
>
>             -----Original Message-----
>             From: Koenig, Christian
>             Sent: Thursday, March 08, 2018 4:07 AM
>             To: Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org
>             <mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>>; Li, Samuel
>             <Samuel.Li-5C7GfCeVMHo@public.gmane.org <mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>; Alex
>             Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org <mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>
>             Cc: amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>             <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>>
>             Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather
>             display support
>
>             Am 08.03.2018 um 09:35 schrieb Michel Dänzer:
>
>                 On 2018-03-07 10:47 AM, Christian König wrote:
>
>                     Am 07.03.2018 um 09:42 schrieb Michel Dänzer:
>
>                         On 2018-03-06 07:23 PM, Christian König wrote:
>
>                             E.g. the last time I tested it placing
>                             things into GTT still
>                             resulted in quite a performance penalty
>                             for rendering.
>
>                         FWIW, I think the penalty is most likely IOMMU
>                         related. Last time I
>                         tested, I couldn't measure a big difference
>                         with IOMMU disabled.
>
>                     No, the penalty I'm talking about came from the
>                     ping/pong we did with
>                     the scanout buffers.
>
>                     See when I tested this the DDX and Mesa where
>                     unmodified, so both
>                     still assumed VRAM as placement for scanout BOs,
>                     but the kernel
>                     forced scanout BOs into GTT for testing.
>
>                     So what happened was that on scanout we moved the
>                     VRAM BO to GTT
>
>             and
>
>                     after unpinning it on the first command submission
>                     which used the BO
>                     we moved it back to VRAM again.
>
>                 In the meantime, I've had the same idea as Marek:
>                 Can't the kernel
>                 driver simply change the BO's preferred domain to GTT
>                 when scanning
>                 out from it? Then it won't move back to VRAM.
>
>             Yes, I've considered this as well.
>
>             But I think making the decision in Mesa is the cleaner
>             approach.
>
>             E.g. so far we only override the placement decision of
>             userspace for two
>             reasons:
>             1. We where running out of memory in VRAM.
>             2. We have a hardware restriction which makes VRAM usage
>             mandatory.
>
>             And even then we never adjust the placement permanently,
>             we just
>             temporary moved the buffer where it was needed and moved
>             it back after
>             the operation completed.
>
>             Additional to that Mesa might want to set even more flags
>             and/or changes
>             it's behavior. E.g. use a tilling mode which both importer
>             and export in an
>             A+A laptop understands etc...
>
>             Regards,
>             Christian.
>
>
>     _______________________________________________
>     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: 10279 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

RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Li, Samuel]

[-- Attachment #1.1: Type: text/plain, Size: 3777 bytes --]

Agreed.

>I think that the consensus with Alex and me is that we should avoid exactly that.
Christian, Alex’s concern is about ping-pong, not about the preferred domain.

Regards,
Samuel Li

From: Marek Olšák [mailto:maraeo@gmail.com]
Sent: Monday, March 19, 2018 3:39 PM
To: Koenig, Christian <Christian.Koenig@amd.com>
Cc: Li, Samuel <Samuel.Li@amd.com>; Michel Dänzer <michel@daenzer.net>; Alex Deucher <alexdeucher@gmail.com>; amd-gfx list <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

On Mon, Mar 19, 2018 at 3:27 PM, Christian König <christian.koenig@amd.com<mailto:christian.koenig@amd.com>> wrote:
I think that the consensus with Alex and me is that we should avoid exactly that.

Overriding the preferred domain in the kernel is a no-go for that patch set, so please implement the discussed changes in Mesa.

I don't see how Mesa can make a smarter decision than the kernel. If you overwrite the preferred domain of the buffer in the kernel, there will be no ping-ponging between domains. Mesa never changes the initial preferred domain.

Marek



Regards,
Christian.


Am 19.03.2018 um 20:22 schrieb Li, Samuel:
I agree with Marek/Michel: since kernel sets the domain before scanning out, it shall update the preferred domain here too.

Regards,
Samuel Li

-----Original Message-----
From: Koenig, Christian
Sent: Thursday, March 08, 2018 4:07 AM
To: Michel Dänzer <michel@daenzer.net<mailto:michel@daenzer.net>>; Li, Samuel
<Samuel.Li@amd.com<mailto:Samuel.Li@amd.com>>; Alex Deucher <alexdeucher@gmail.com<mailto:alexdeucher@gmail.com>>
Cc: amd-gfx list <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

Am 08.03.2018 um 09:35 schrieb Michel Dänzer:
On 2018-03-07 10:47 AM, Christian König wrote:
Am 07.03.2018 um 09:42 schrieb Michel Dänzer:
On 2018-03-06 07:23 PM, Christian König wrote:
E.g. the last time I tested it placing things into GTT still
resulted in quite a performance penalty for rendering.
FWIW, I think the penalty is most likely IOMMU related. Last time I
tested, I couldn't measure a big difference with IOMMU disabled.
No, the penalty I'm talking about came from the ping/pong we did with
the scanout buffers.

See when I tested this the DDX and Mesa where unmodified, so both
still assumed VRAM as placement for scanout BOs, but the kernel
forced scanout BOs into GTT for testing.

So what happened was that on scanout we moved the VRAM BO to GTT
and
after unpinning it on the first command submission which used the BO
we moved it back to VRAM again.
In the meantime, I've had the same idea as Marek: Can't the kernel
driver simply change the BO's preferred domain to GTT when scanning
out from it? Then it won't move back to VRAM.
Yes, I've considered this as well.

But I think making the decision in Mesa is the cleaner approach.

E.g. so far we only override the placement decision of userspace for two
reasons:
1. We where running out of memory in VRAM.
2. We have a hardware restriction which makes VRAM usage mandatory.

And even then we never adjust the placement permanently, we just
temporary moved the buffer where it was needed and moved it back after
the operation completed.

Additional to that Mesa might want to set even more flags and/or changes
it's behavior. E.g. use a tilling mode which both importer and export in an
A+A laptop understands etc...

Regards,
Christian.

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


[-- Attachment #1.2: Type: text/html, Size: 9898 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 6415 bytes --]

Quoting Alex:
> Regardless of which scenarios we need to support, I think we also need
> to really plumb this through to mesa however since user space is who
> ultimately requests the location.  Overriding it in the kernel gets
> tricky and can lead to ping-ponging as others have noted.  Better to
> have user space know what chips support it or not and request display
> buffers in GTT or VRAM from the start.
And I completely agree with Alex here. So overriding the domain in the 
kernel is a serious NAK from my side as well.

Please implement the necessary bits in Mesa, shouldn't be more than a 
few lines of code anyway.

Regards,
Christian.

Am 19.03.2018 um 20:42 schrieb Li, Samuel:
>
> Agreed.
>
> >I think that the consensus with Alex and me is that we should avoid 
> exactly that.
> Christian, Alex’s concern is about ping-pong, not about the preferred 
> domain.
>
> Regards,
>
> Samuel Li
>
> *From:*Marek Olšák [mailto:maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> *Sent:* Monday, March 19, 2018 3:39 PM
> *To:* Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>
> *Cc:* Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Michel Dänzer 
> <michel-otUistvHUpPR7s880joybQ@public.gmane.org>; Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; amd-gfx 
> list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> *Subject:* Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display 
> support
>
> On Mon, Mar 19, 2018 at 3:27 PM, Christian König 
> <christian.koenig-5C7GfCeVMHo@public.gmane.org <mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>> wrote:
>
>     I think that the consensus with Alex and me is that we should
>     avoid exactly that.
>
>     Overriding the preferred domain in the kernel is a no-go for that
>     patch set, so please implement the discussed changes in Mesa.
>
> I don't see how Mesa can make a smarter decision than the kernel. If 
> you overwrite the preferred domain of the buffer in the kernel, there 
> will be no ping-ponging between domains. Mesa never changes the 
> initial preferred domain.
>
> Marek
>
>
>     Regards,
>     Christian.
>
>
>
>     Am 19.03.2018 um 20:22 schrieb Li, Samuel:
>
>         I agree with Marek/Michel: since kernel sets the domain before
>         scanning out, it shall update the preferred domain here too.
>
>         Regards,
>         Samuel Li
>
>             -----Original Message-----
>             From: Koenig, Christian
>             Sent: Thursday, March 08, 2018 4:07 AM
>             To: Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org
>             <mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>>; Li, Samuel
>             <Samuel.Li-5C7GfCeVMHo@public.gmane.org <mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>; Alex
>             Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org <mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>
>             Cc: amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>             <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>>
>             Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather
>             display support
>
>             Am 08.03.2018 um 09:35 schrieb Michel Dänzer:
>
>                 On 2018-03-07 10:47 AM, Christian König wrote:
>
>                     Am 07.03.2018 um 09:42 schrieb Michel Dänzer:
>
>                         On 2018-03-06 07:23 PM, Christian König wrote:
>
>                             E.g. the last time I tested it placing
>                             things into GTT still
>                             resulted in quite a performance penalty
>                             for rendering.
>
>                         FWIW, I think the penalty is most likely IOMMU
>                         related. Last time I
>                         tested, I couldn't measure a big difference
>                         with IOMMU disabled.
>
>                     No, the penalty I'm talking about came from the
>                     ping/pong we did with
>                     the scanout buffers.
>
>                     See when I tested this the DDX and Mesa where
>                     unmodified, so both
>                     still assumed VRAM as placement for scanout BOs,
>                     but the kernel
>                     forced scanout BOs into GTT for testing.
>
>                     So what happened was that on scanout we moved the
>                     VRAM BO to GTT
>
>             and
>
>                     after unpinning it on the first command submission
>                     which used the BO
>                     we moved it back to VRAM again.
>
>                 In the meantime, I've had the same idea as Marek:
>                 Can't the kernel
>                 driver simply change the BO's preferred domain to GTT
>                 when scanning
>                 out from it? Then it won't move back to VRAM.
>
>             Yes, I've considered this as well.
>
>             But I think making the decision in Mesa is the cleaner
>             approach.
>
>             E.g. so far we only override the placement decision of
>             userspace for two
>             reasons:
>             1. We where running out of memory in VRAM.
>             2. We have a hardware restriction which makes VRAM usage
>             mandatory.
>
>             And even then we never adjust the placement permanently,
>             we just
>             temporary moved the buffer where it was needed and moved
>             it back after
>             the operation completed.
>
>             Additional to that Mesa might want to set even more flags
>             and/or changes
>             it's behavior. E.g. use a tilling mode which both importer
>             and export in an
>             A+A laptop understands etc...
>
>             Regards,
>             Christian.
>
>
>     _______________________________________________
>     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
>
>
>
> _______________________________________________
> 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: 18050 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

RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Li, Samuel]

[-- Attachment #1.1: Type: text/plain, Size: 5878 bytes --]

Christian,

You misunderstood Alex’s comments,


>Regardless of which scenarios we need to support, I think we also need

>to really plumb this through to mesa however since user space is who

>ultimately requests the location.  Overriding it in the kernel gets

>tricky and can lead to ping-ponging as others have noted.  Better to

Here Alex mentioned the scenarios is 1)VRAM, 2)GTT, 3)VRAM/GTT.
His concern is this might cause ping-pong, not about preferred domain. Since preferred domain can solve the ping-pong issue, it shall address his concern here.

Regards,
Samuel Li

From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com]
Sent: Monday, March 19, 2018 3:45 PM
To: Li, Samuel <Samuel.Li@amd.com>; Marek Olšák <maraeo@gmail.com>; Koenig, Christian <Christian.Koenig@amd.com>
Cc: Alex Deucher <alexdeucher@gmail.com>; Michel Dänzer <michel@daenzer.net>; amd-gfx list <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

Quoting Alex:


Regardless of which scenarios we need to support, I think we also need

to really plumb this through to mesa however since user space is who

ultimately requests the location.  Overriding it in the kernel gets

tricky and can lead to ping-ponging as others have noted.  Better to

have user space know what chips support it or not and request display

buffers in GTT or VRAM from the start.
And I completely agree with Alex here. So overriding the domain in the kernel is a serious NAK from my side as well.

Please implement the necessary bits in Mesa, shouldn't be more than a few lines of code anyway.

Regards,
Christian.

Am 19.03.2018 um 20:42 schrieb Li, Samuel:
Agreed.

>I think that the consensus with Alex and me is that we should avoid exactly that.
Christian, Alex’s concern is about ping-pong, not about the preferred domain.


Regards,
Samuel Li

From: Marek Olšák [mailto:maraeo@gmail.com]
Sent: Monday, March 19, 2018 3:39 PM
To: Koenig, Christian <Christian.Koenig@amd.com><mailto:Christian.Koenig@amd.com>
Cc: Li, Samuel <Samuel.Li@amd.com><mailto:Samuel.Li@amd.com>; Michel Dänzer <michel@daenzer.net><mailto:michel@daenzer.net>; Alex Deucher <alexdeucher@gmail.com><mailto:alexdeucher@gmail.com>; amd-gfx list <amd-gfx@lists.freedesktop.org><mailto:amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

On Mon, Mar 19, 2018 at 3:27 PM, Christian König <christian.koenig@amd.com<mailto:christian.koenig@amd.com>> wrote:
I think that the consensus with Alex and me is that we should avoid exactly that.

Overriding the preferred domain in the kernel is a no-go for that patch set, so please implement the discussed changes in Mesa.

I don't see how Mesa can make a smarter decision than the kernel. If you overwrite the preferred domain of the buffer in the kernel, there will be no ping-ponging between domains. Mesa never changes the initial preferred domain.

Marek



Regards,
Christian.


Am 19.03.2018 um 20:22 schrieb Li, Samuel:
I agree with Marek/Michel: since kernel sets the domain before scanning out, it shall update the preferred domain here too.

Regards,
Samuel Li


-----Original Message-----
From: Koenig, Christian
Sent: Thursday, March 08, 2018 4:07 AM
To: Michel Dänzer <michel@daenzer.net<mailto:michel@daenzer.net>>; Li, Samuel
<Samuel.Li@amd.com<mailto:Samuel.Li@amd.com>>; Alex Deucher <alexdeucher@gmail.com<mailto:alexdeucher@gmail.com>>
Cc: amd-gfx list <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

Am 08.03.2018 um 09:35 schrieb Michel Dänzer:
On 2018-03-07 10:47 AM, Christian König wrote:
Am 07.03.2018 um 09:42 schrieb Michel Dänzer:
On 2018-03-06 07:23 PM, Christian König wrote:
E.g. the last time I tested it placing things into GTT still
resulted in quite a performance penalty for rendering.
FWIW, I think the penalty is most likely IOMMU related. Last time I
tested, I couldn't measure a big difference with IOMMU disabled.
No, the penalty I'm talking about came from the ping/pong we did with
the scanout buffers.

See when I tested this the DDX and Mesa where unmodified, so both
still assumed VRAM as placement for scanout BOs, but the kernel
forced scanout BOs into GTT for testing.

So what happened was that on scanout we moved the VRAM BO to GTT
and
after unpinning it on the first command submission which used the BO
we moved it back to VRAM again.
In the meantime, I've had the same idea as Marek: Can't the kernel
driver simply change the BO's preferred domain to GTT when scanning
out from it? Then it won't move back to VRAM.
Yes, I've considered this as well.

But I think making the decision in Mesa is the cleaner approach.

E.g. so far we only override the placement decision of userspace for two
reasons:
1. We where running out of memory in VRAM.
2. We have a hardware restriction which makes VRAM usage mandatory.

And even then we never adjust the placement permanently, we just
temporary moved the buffer where it was needed and moved it back after
the operation completed.

Additional to that Mesa might want to set even more flags and/or changes
it's behavior. E.g. use a tilling mode which both importer and export in an
A+A laptop understands etc...

Regards,
Christian.

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





_______________________________________________

amd-gfx mailing list

amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>

https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[-- Attachment #1.2: Type: text/html, Size: 15734 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Deucher, Alexander]

[-- Attachment #1.1: Type: text/plain, Size: 7203 bytes --]

My personal preference is still to plumb this through to mesa rather than forcing it in the kernel.


Alex

________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org>
Sent: Monday, March 19, 2018 3:50:34 PM
To: Koenig, Christian; Marek Olšák
Cc: Alex Deucher; Michel Dänzer; amd-gfx list
Subject: RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support


Christian,



You misunderstood Alex’s comments,



>Regardless of which scenarios we need to support, I think we also need

>to really plumb this through to mesa however since user space is who

>ultimately requests the location.  Overriding it in the kernel gets

>tricky and can lead to ping-ponging as others have noted.  Better to



Here Alex mentioned the scenarios is 1)VRAM, 2)GTT, 3)VRAM/GTT.

His concern is this might cause ping-pong, not about preferred domain. Since preferred domain can solve the ping-pong issue, it shall address his concern here.



Regards,

Samuel Li



From: Christian König [mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
Sent: Monday, March 19, 2018 3:45 PM
To: Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Marek Olšák <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>
Cc: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Michel Dänzer <michel@daenzer.net>; amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support



Quoting Alex:


Regardless of which scenarios we need to support, I think we also need

to really plumb this through to mesa however since user space is who

ultimately requests the location.  Overriding it in the kernel gets

tricky and can lead to ping-ponging as others have noted.  Better to

have user space know what chips support it or not and request display

buffers in GTT or VRAM from the start.

And I completely agree with Alex here. So overriding the domain in the kernel is a serious NAK from my side as well.

Please implement the necessary bits in Mesa, shouldn't be more than a few lines of code anyway.

Regards,
Christian.

Am 19.03.2018 um 20:42 schrieb Li, Samuel:

Agreed.



>I think that the consensus with Alex and me is that we should avoid exactly that.
Christian, Alex’s concern is about ping-pong, not about the preferred domain.



Regards,

Samuel Li



From: Marek Olšák [mailto:maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
Sent: Monday, March 19, 2018 3:39 PM
To: Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org><mailto:Christian.Koenig@amd.com>
Cc: Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org><mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org><mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>; Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org><mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; amd-gfx list <amd-gfx-PD4FTy7X32k@public.gmane.orgeedesktop.org><mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support



On Mon, Mar 19, 2018 at 3:27 PM, Christian König <christian.koenig-urvtwAKJhsc@public.gmane.orgm<mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>> wrote:

I think that the consensus with Alex and me is that we should avoid exactly that.

Overriding the preferred domain in the kernel is a no-go for that patch set, so please implement the discussed changes in Mesa.



I don't see how Mesa can make a smarter decision than the kernel. If you overwrite the preferred domain of the buffer in the kernel, there will be no ping-ponging between domains. Mesa never changes the initial preferred domain.



Marek





Regards,
Christian.


Am 19.03.2018 um 20:22 schrieb Li, Samuel:

I agree with Marek/Michel: since kernel sets the domain before scanning out, it shall update the preferred domain here too.

Regards,
Samuel Li



-----Original Message-----
From: Koenig, Christian
Sent: Thursday, March 08, 2018 4:07 AM
To: Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org<mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>>; Li, Samuel
<Samuel.Li-5C7GfCeVMHo@public.gmane.org<mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>; Alex Deucher <alexdeucher@gmail.com<mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>
Cc: amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lMiVNPc3mojA@public.gmane.orgsktop.org>>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

Am 08.03.2018 um 09:35 schrieb Michel Dänzer:

On 2018-03-07 10:47 AM, Christian König wrote:

Am 07.03.2018 um 09:42 schrieb Michel Dänzer:

On 2018-03-06 07:23 PM, Christian König wrote:

E.g. the last time I tested it placing things into GTT still
resulted in quite a performance penalty for rendering.

FWIW, I think the penalty is most likely IOMMU related. Last time I
tested, I couldn't measure a big difference with IOMMU disabled.

No, the penalty I'm talking about came from the ping/pong we did with
the scanout buffers.

See when I tested this the DDX and Mesa where unmodified, so both
still assumed VRAM as placement for scanout BOs, but the kernel
forced scanout BOs into GTT for testing.

So what happened was that on scanout we moved the VRAM BO to GTT

and

after unpinning it on the first command submission which used the BO
we moved it back to VRAM again.

In the meantime, I've had the same idea as Marek: Can't the kernel
driver simply change the BO's preferred domain to GTT when scanning
out from it? Then it won't move back to VRAM.

Yes, I've considered this as well.

But I think making the decision in Mesa is the cleaner approach.

E.g. so far we only override the placement decision of userspace for two
reasons:
1. We where running out of memory in VRAM.
2. We have a hardware restriction which makes VRAM usage mandatory.

And even then we never adjust the placement permanently, we just
temporary moved the buffer where it was needed and moved it back after
the operation completed.

Additional to that Mesa might want to set even more flags and/or changes
it's behavior. E.g. use a tilling mode which both importer and export in an
A+A laptop understands etc...

Regards,
Christian.

_______________________________________________
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






_______________________________________________

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



[-- Attachment #1.2: Type: text/html, Size: 16022 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

RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Li, Samuel]

[-- Attachment #1.1: Type: text/plain, Size: 8408 bytes --]

Alex,

I assume you are talking the three scenarios here, 1)VRAM, 2)GTT, 3)VRAM/GTT.
But kernel will need the decision too(amdgpufb). I think it shall be better to do it in kernel, instead of different clients(mesa, ddx, kernel ...)

Regards,
Samuel Li

From: Deucher, Alexander
Sent: Monday, March 19, 2018 3:54 PM
To: Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Koenig, Christian <Christian.Koenig@amd.com>; Marek Olšák <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Michel Dänzer <michel@daenzer.net>; amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support


My personal preference is still to plumb this through to mesa rather than forcing it in the kernel.



Alex

________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org<mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>
Sent: Monday, March 19, 2018 3:50:34 PM
To: Koenig, Christian; Marek Olšák
Cc: Alex Deucher; Michel Dänzer; amd-gfx list
Subject: RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support


Christian,



You misunderstood Alex's comments,



>Regardless of which scenarios we need to support, I think we also need

>to really plumb this through to mesa however since user space is who

>ultimately requests the location.  Overriding it in the kernel gets

>tricky and can lead to ping-ponging as others have noted.  Better to



Here Alex mentioned the scenarios is 1)VRAM, 2)GTT, 3)VRAM/GTT.

His concern is this might cause ping-pong, not about preferred domain. Since preferred domain can solve the ping-pong issue, it shall address his concern here.



Regards,

Samuel Li



From: Christian König [mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
Sent: Monday, March 19, 2018 3:45 PM
To: Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org<mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>; Marek Olšák <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>; Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org<mailto:Christian.Koenig-5C7GfCeVMHo@public.gmane.org>>
Cc: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>; Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org<mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>>; amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support



Quoting Alex:

Regardless of which scenarios we need to support, I think we also need

to really plumb this through to mesa however since user space is who

ultimately requests the location.  Overriding it in the kernel gets

tricky and can lead to ping-ponging as others have noted.  Better to

have user space know what chips support it or not and request display

buffers in GTT or VRAM from the start.

And I completely agree with Alex here. So overriding the domain in the kernel is a serious NAK from my side as well.

Please implement the necessary bits in Mesa, shouldn't be more than a few lines of code anyway.

Regards,
Christian.

Am 19.03.2018 um 20:42 schrieb Li, Samuel:

Agreed.



>I think that the consensus with Alex and me is that we should avoid exactly that.
Christian, Alex's concern is about ping-pong, not about the preferred domain.


Regards,

Samuel Li



From: Marek Olšák [mailto:maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
Sent: Monday, March 19, 2018 3:39 PM
To: Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org><mailto:Christian.Koenig@amd.com>
Cc: Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org><mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org><mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>; Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org><mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; amd-gfx list <amd-gfx-PD4FTy7X32k@public.gmane.orgeedesktop.org><mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support



On Mon, Mar 19, 2018 at 3:27 PM, Christian König <christian.koenig-urvtwAKJhsc@public.gmane.orgm<mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>> wrote:

I think that the consensus with Alex and me is that we should avoid exactly that.

Overriding the preferred domain in the kernel is a no-go for that patch set, so please implement the discussed changes in Mesa.



I don't see how Mesa can make a smarter decision than the kernel. If you overwrite the preferred domain of the buffer in the kernel, there will be no ping-ponging between domains. Mesa never changes the initial preferred domain.



Marek





Regards,
Christian.


Am 19.03.2018 um 20:22 schrieb Li, Samuel:

I agree with Marek/Michel: since kernel sets the domain before scanning out, it shall update the preferred domain here too.

Regards,
Samuel Li


-----Original Message-----
From: Koenig, Christian
Sent: Thursday, March 08, 2018 4:07 AM
To: Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org<mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>>; Li, Samuel
<Samuel.Li-5C7GfCeVMHo@public.gmane.org<mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>; Alex Deucher <alexdeucher@gmail.com<mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>
Cc: amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lMiVNPc3mojA@public.gmane.orgsktop.org>>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

Am 08.03.2018 um 09:35 schrieb Michel Dänzer:

On 2018-03-07 10:47 AM, Christian König wrote:

Am 07.03.2018 um 09:42 schrieb Michel Dänzer:

On 2018-03-06 07:23 PM, Christian König wrote:

E.g. the last time I tested it placing things into GTT still
resulted in quite a performance penalty for rendering.

FWIW, I think the penalty is most likely IOMMU related. Last time I
tested, I couldn't measure a big difference with IOMMU disabled.

No, the penalty I'm talking about came from the ping/pong we did with
the scanout buffers.

See when I tested this the DDX and Mesa where unmodified, so both
still assumed VRAM as placement for scanout BOs, but the kernel
forced scanout BOs into GTT for testing.

So what happened was that on scanout we moved the VRAM BO to GTT

and

after unpinning it on the first command submission which used the BO
we moved it back to VRAM again.

In the meantime, I've had the same idea as Marek: Can't the kernel
driver simply change the BO's preferred domain to GTT when scanning
out from it? Then it won't move back to VRAM.

Yes, I've considered this as well.

But I think making the decision in Mesa is the cleaner approach.

E.g. so far we only override the placement decision of userspace for two
reasons:
1. We where running out of memory in VRAM.
2. We have a hardware restriction which makes VRAM usage mandatory.

And even then we never adjust the placement permanently, we just
temporary moved the buffer where it was needed and moved it back after
the operation completed.

Additional to that Mesa might want to set even more flags and/or changes
it's behavior. E.g. use a tilling mode which both importer and export in an
A+A laptop understands etc...

Regards,
Christian.

_______________________________________________
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





_______________________________________________

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



[-- Attachment #1.2: Type: text/html, Size: 23286 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 10930 bytes --]

Hi Sam,

no that is seriously a no-go. The placement decision is done my 
userspace, not the kernel.

The only time the kernel overrides that decision is a) hardware 
requirements (for example old UVD on AGP systems) and b) memory pressure.

We are have actually pushed quite hard on avoiding and removing places 
where the kernel overrides the placement decision of userspace, so I 
don't think we should add any new ones.

Regards,
Christian.

Am 19.03.2018 um 20:58 schrieb Li, Samuel:
>
> Alex,
>
> I assume you are talking the three scenarios here, 1)VRAM, 2)GTT, 
> 3)VRAM/GTT.
>
> But kernel will need the decision too(amdgpufb). I think it shall be 
> better to do it in kernel, instead of different clients(mesa, ddx, 
> kernel …)
>
> Regards,
>
> Samuel Li
>
> *From:*Deucher, Alexander
> *Sent:* Monday, March 19, 2018 3:54 PM
> *To:* Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Koenig, Christian 
> <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>; Marek Olšák <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> *Cc:* Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Michel Dänzer 
> <michel-otUistvHUpPR7s880joybQ@public.gmane.org>; amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> *Subject:* Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display 
> support
>
> My personal preference is still to plumb this through to mesa rather 
> than forcing it in the kernel.
>
> Alex
>
> ------------------------------------------------------------------------
>
> *From:*amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org 
> <mailto:amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>> on behalf of Li, 
> Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org <mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>
> *Sent:* Monday, March 19, 2018 3:50:34 PM
> *To:* Koenig, Christian; Marek Olšák
> *Cc:* Alex Deucher; Michel Dänzer; amd-gfx list
> *Subject:* RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display 
> support
>
> Christian,
>
> You misunderstood Alex’s comments,
>
> >Regardless of which scenarios we need to support, I think we also need
> >to really plumb this through to mesa however since user space is who
> >ultimately requests the location.  Overriding it in the kernel gets
> >tricky and can lead to ping-ponging as others have noted.  Better to
>
> Here Alex mentioned the scenarios is 1)VRAM, 2)GTT, 3)VRAM/GTT.
>
> His concern is this might cause ping-pong, not about preferred domain. 
> Since preferred domain can solve the ping-pong issue, it shall address 
> his concern here.
>
> Regards,
>
> Samuel Li
>
> *From:*Christian König [mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> *Sent:* Monday, March 19, 2018 3:45 PM
> *To:* Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org <mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>; Marek 
> Olšák <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org <mailto:maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>; Koenig, Christian 
> <Christian.Koenig-5C7GfCeVMHo@public.gmane.org <mailto:Christian.Koenig-5C7GfCeVMHo@public.gmane.org>>
> *Cc:* Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org 
> <mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>; Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org 
> <mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>>; amd-gfx list 
> <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>>
> *Subject:* Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display 
> support
>
> Quoting Alex:
>
>     Regardless of which scenarios we need to support, I think we also need
>
>     to really plumb this through to mesa however since user space is who
>
>     ultimately requests the location.  Overriding it in the kernel gets
>
>     tricky and can lead to ping-ponging as others have noted.  Better to
>
>     have user space know what chips support it or not and request display
>
>     buffers in GTT or VRAM from the start.
>
> And I completely agree with Alex here. So overriding the domain in the 
> kernel is a serious NAK from my side as well.
>
> Please implement the necessary bits in Mesa, shouldn't be more than a 
> few lines of code anyway.
>
> Regards,
> Christian.
>
> Am 19.03.2018 um 20:42 schrieb Li, Samuel:
>
>     Agreed.
>
>     >I think that the consensus with Alex and me is that we should
>     avoid exactly that.
>     Christian, Alex’s concern is about ping-pong, not about the
>     preferred domain.
>
>     Regards,
>
>     Samuel Li
>
>     *From:*Marek Olšák [mailto:maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
>     *Sent:* Monday, March 19, 2018 3:39 PM
>     *To:* Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>
>     <mailto:Christian.Koenig-5C7GfCeVMHo@public.gmane.org>
>     *Cc:* Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org> <mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>;
>     Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org> <mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>;
>     Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>     <mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; amd-gfx list
>     <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
>     *Subject:* Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather
>     display support
>
>     On Mon, Mar 19, 2018 at 3:27 PM, Christian König
>     <christian.koenig-5C7GfCeVMHo@public.gmane.org <mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>> wrote:
>
>         I think that the consensus with Alex and me is that we should
>         avoid exactly that.
>
>         Overriding the preferred domain in the kernel is a no-go for
>         that patch set, so please implement the discussed changes in Mesa.
>
>     I don't see how Mesa can make a smarter decision than the kernel.
>     If you overwrite the preferred domain of the buffer in the kernel,
>     there will be no ping-ponging between domains. Mesa never changes
>     the initial preferred domain.
>
>     Marek
>
>
>         Regards,
>         Christian.
>
>
>
>         Am 19.03.2018 um 20:22 schrieb Li, Samuel:
>
>             I agree with Marek/Michel: since kernel sets the domain
>             before scanning out, it shall update the preferred domain
>             here too.
>
>             Regards,
>             Samuel Li
>
>                 -----Original Message-----
>                 From: Koenig, Christian
>                 Sent: Thursday, March 08, 2018 4:07 AM
>                 To: Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org
>                 <mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>>; Li, Samuel
>                 <Samuel.Li-5C7GfCeVMHo@public.gmane.org <mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>; Alex
>                 Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
>                 <mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>
>                 Cc: amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>                 <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>>
>                 Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter
>                 gather display support
>
>                 Am 08.03.2018 um 09:35 schrieb Michel Dänzer:
>
>                     On 2018-03-07 10:47 AM, Christian König wrote:
>
>                         Am 07.03.2018 um 09:42 schrieb Michel Dänzer:
>
>                             On 2018-03-06 07:23 PM, Christian König wrote:
>
>                                 E.g. the last time I tested it placing
>                                 things into GTT still
>                                 resulted in quite a performance
>                                 penalty for rendering.
>
>                             FWIW, I think the penalty is most likely
>                             IOMMU related. Last time I
>                             tested, I couldn't measure a big
>                             difference with IOMMU disabled.
>
>                         No, the penalty I'm talking about came from
>                         the ping/pong we did with
>                         the scanout buffers.
>
>                         See when I tested this the DDX and Mesa where
>                         unmodified, so both
>                         still assumed VRAM as placement for scanout
>                         BOs, but the kernel
>                         forced scanout BOs into GTT for testing.
>
>                         So what happened was that on scanout we moved
>                         the VRAM BO to GTT
>
>                 and
>
>                         after unpinning it on the first command
>                         submission which used the BO
>                         we moved it back to VRAM again.
>
>                     In the meantime, I've had the same idea as Marek:
>                     Can't the kernel
>                     driver simply change the BO's preferred domain to
>                     GTT when scanning
>                     out from it? Then it won't move back to VRAM.
>
>                 Yes, I've considered this as well.
>
>                 But I think making the decision in Mesa is the cleaner
>                 approach.
>
>                 E.g. so far we only override the placement decision of
>                 userspace for two
>                 reasons:
>                 1. We where running out of memory in VRAM.
>                 2. We have a hardware restriction which makes VRAM
>                 usage mandatory.
>
>                 And even then we never adjust the placement
>                 permanently, we just
>                 temporary moved the buffer where it was needed and
>                 moved it back after
>                 the operation completed.
>
>                 Additional to that Mesa might want to set even more
>                 flags and/or changes
>                 it's behavior. E.g. use a tilling mode which both
>                 importer and export in an
>                 A+A laptop understands etc...
>
>                 Regards,
>                 Christian.
>
>
>         _______________________________________________
>         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
>
>
>
>     _______________________________________________
>
>     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
>


[-- Attachment #1.2: Type: text/html, Size: 38395 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Deucher, Alexander]

[-- Attachment #1.1: Type: text/plain, Size: 9656 bytes --]

I'm not sure what you mean by the 3 scenarios.  Generally userspace selects what domains it wants a buffer to be in, vram, gtt, or both (don't care).  I'd rather not have the kernel second guess the UMDs if we can help it.  I'd rather leave the kernel for cases where we have to force things due to hw bugs, or hw restrictions, etc.  If we force all display buffers to be in gtt in the kernel, we have not excluded the possibility of ever setting displays anywhere else without a kernel update.  E.g., to my earlier point, there may be cases where it is advantageous to put display buffers in vram even if s/g display is supported.  That was the point I was trying to make about user mode selecting the domain (vram of gtt or vram|gtt).  Say you have a board with 2 GB of ram and 1 GB is carved out for "vram".  In that case, it would make sense to put the buffer in vram because otherwise you are wasting a comparatively scarce resource.


Alex

________________________________
From: Li, Samuel
Sent: Monday, March 19, 2018 3:58:52 PM
To: Deucher, Alexander; Koenig, Christian; Marek Olšák
Cc: Alex Deucher; Michel Dänzer; amd-gfx list
Subject: RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support


Alex,



I assume you are talking the three scenarios here, 1)VRAM, 2)GTT, 3)VRAM/GTT.

But kernel will need the decision too(amdgpufb). I think it shall be better to do it in kernel, instead of different clients(mesa, ddx, kernel …)



Regards,

Samuel Li



From: Deucher, Alexander
Sent: Monday, March 19, 2018 3:54 PM
To: Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Koenig, Christian <Christian.Koenig@amd.com>; Marek Olšák <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Michel Dänzer <michel@daenzer.net>; amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support



My personal preference is still to plumb this through to mesa rather than forcing it in the kernel.



Alex

________________________________

From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org<mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>
Sent: Monday, March 19, 2018 3:50:34 PM
To: Koenig, Christian; Marek Olšák
Cc: Alex Deucher; Michel Dänzer; amd-gfx list
Subject: RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support



Christian,



You misunderstood Alex’s comments,



>Regardless of which scenarios we need to support, I think we also need

>to really plumb this through to mesa however since user space is who

>ultimately requests the location.  Overriding it in the kernel gets

>tricky and can lead to ping-ponging as others have noted.  Better to



Here Alex mentioned the scenarios is 1)VRAM, 2)GTT, 3)VRAM/GTT.

His concern is this might cause ping-pong, not about preferred domain. Since preferred domain can solve the ping-pong issue, it shall address his concern here.



Regards,

Samuel Li



From: Christian König [mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
Sent: Monday, March 19, 2018 3:45 PM
To: Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org<mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>; Marek Olšák <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>; Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org<mailto:Christian.Koenig-5C7GfCeVMHo@public.gmane.org>>
Cc: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>; Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org<mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>>; amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support



Quoting Alex:

Regardless of which scenarios we need to support, I think we also need

to really plumb this through to mesa however since user space is who

ultimately requests the location.  Overriding it in the kernel gets

tricky and can lead to ping-ponging as others have noted.  Better to

have user space know what chips support it or not and request display

buffers in GTT or VRAM from the start.

And I completely agree with Alex here. So overriding the domain in the kernel is a serious NAK from my side as well.

Please implement the necessary bits in Mesa, shouldn't be more than a few lines of code anyway.

Regards,
Christian.

Am 19.03.2018 um 20:42 schrieb Li, Samuel:

Agreed.



>I think that the consensus with Alex and me is that we should avoid exactly that.
Christian, Alex’s concern is about ping-pong, not about the preferred domain.


Regards,

Samuel Li



From: Marek Olšák [mailto:maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
Sent: Monday, March 19, 2018 3:39 PM
To: Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org><mailto:Christian.Koenig@amd.com>
Cc: Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org><mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org><mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>; Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org><mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; amd-gfx list <amd-gfx-PD4FTy7X32k@public.gmane.orgeedesktop.org><mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support



On Mon, Mar 19, 2018 at 3:27 PM, Christian König <christian.koenig-urvtwAKJhsc@public.gmane.orgm<mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>> wrote:

I think that the consensus with Alex and me is that we should avoid exactly that.

Overriding the preferred domain in the kernel is a no-go for that patch set, so please implement the discussed changes in Mesa.



I don't see how Mesa can make a smarter decision than the kernel. If you overwrite the preferred domain of the buffer in the kernel, there will be no ping-ponging between domains. Mesa never changes the initial preferred domain.



Marek





Regards,
Christian.


Am 19.03.2018 um 20:22 schrieb Li, Samuel:

I agree with Marek/Michel: since kernel sets the domain before scanning out, it shall update the preferred domain here too.

Regards,
Samuel Li


-----Original Message-----
From: Koenig, Christian
Sent: Thursday, March 08, 2018 4:07 AM
To: Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org<mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>>; Li, Samuel
<Samuel.Li-5C7GfCeVMHo@public.gmane.org<mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>; Alex Deucher <alexdeucher@gmail.com<mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>
Cc: amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lMiVNPc3mojA@public.gmane.orgsktop.org>>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

Am 08.03.2018 um 09:35 schrieb Michel Dänzer:

On 2018-03-07 10:47 AM, Christian König wrote:

Am 07.03.2018 um 09:42 schrieb Michel Dänzer:

On 2018-03-06 07:23 PM, Christian König wrote:

E.g. the last time I tested it placing things into GTT still
resulted in quite a performance penalty for rendering.

FWIW, I think the penalty is most likely IOMMU related. Last time I
tested, I couldn't measure a big difference with IOMMU disabled.

No, the penalty I'm talking about came from the ping/pong we did with
the scanout buffers.

See when I tested this the DDX and Mesa where unmodified, so both
still assumed VRAM as placement for scanout BOs, but the kernel
forced scanout BOs into GTT for testing.

So what happened was that on scanout we moved the VRAM BO to GTT

and

after unpinning it on the first command submission which used the BO
we moved it back to VRAM again.

In the meantime, I've had the same idea as Marek: Can't the kernel
driver simply change the BO's preferred domain to GTT when scanning
out from it? Then it won't move back to VRAM.

Yes, I've considered this as well.

But I think making the decision in Mesa is the cleaner approach.

E.g. so far we only override the placement decision of userspace for two
reasons:
1. We where running out of memory in VRAM.
2. We have a hardware restriction which makes VRAM usage mandatory.

And even then we never adjust the placement permanently, we just
temporary moved the buffer where it was needed and moved it back after
the operation completed.

Additional to that Mesa might want to set even more flags and/or changes
it's behavior. E.g. use a tilling mode which both importer and export in an
A+A laptop understands etc...

Regards,
Christian.

_______________________________________________
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





_______________________________________________

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



[-- Attachment #1.2: Type: text/html, Size: 23345 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

RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Deucher, Alexander]

[-- Attachment #1.1: Type: text/plain, Size: 10386 bytes --]

s/not/now/.  I meant to say, "we have NOW excluded the possibility of ever setting displays anywhere else without a kernel update".

Alex

From: amd-gfx [mailto:amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org] On Behalf Of Deucher, Alexander
Sent: Monday, March 19, 2018 4:13 PM
To: Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Koenig, Christian <Christian.Koenig@amd.com>; Marek Olšák <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Michel Dänzer <michel@daenzer.net>; amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support


I'm not sure what you mean by the 3 scenarios.  Generally userspace selects what domains it wants a buffer to be in, vram, gtt, or both (don't care).  I'd rather not have the kernel second guess the UMDs if we can help it.  I'd rather leave the kernel for cases where we have to force things due to hw bugs, or hw restrictions, etc.  If we force all display buffers to be in gtt in the kernel, we have not excluded the possibility of ever setting displays anywhere else without a kernel update.  E.g., to my earlier point, there may be cases where it is advantageous to put display buffers in vram even if s/g display is supported.  That was the point I was trying to make about user mode selecting the domain (vram of gtt or vram|gtt).  Say you have a board with 2 GB of ram and 1 GB is carved out for "vram".  In that case, it would make sense to put the buffer in vram because otherwise you are wasting a comparatively scarce resource.



Alex

________________________________
From: Li, Samuel
Sent: Monday, March 19, 2018 3:58:52 PM
To: Deucher, Alexander; Koenig, Christian; Marek Olšák
Cc: Alex Deucher; Michel Dänzer; amd-gfx list
Subject: RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support


Alex,



I assume you are talking the three scenarios here, 1)VRAM, 2)GTT, 3)VRAM/GTT.

But kernel will need the decision too(amdgpufb). I think it shall be better to do it in kernel, instead of different clients(mesa, ddx, kernel ...)



Regards,

Samuel Li



From: Deucher, Alexander
Sent: Monday, March 19, 2018 3:54 PM
To: Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Koenig, Christian <Christian.Koenig@amd.com>; Marek Olšák <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Michel Dänzer <michel@daenzer.net>; amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support



My personal preference is still to plumb this through to mesa rather than forcing it in the kernel.



Alex

________________________________

From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org<mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>
Sent: Monday, March 19, 2018 3:50:34 PM
To: Koenig, Christian; Marek Olšák
Cc: Alex Deucher; Michel Dänzer; amd-gfx list
Subject: RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support



Christian,



You misunderstood Alex's comments,



>Regardless of which scenarios we need to support, I think we also need

>to really plumb this through to mesa however since user space is who

>ultimately requests the location.  Overriding it in the kernel gets

>tricky and can lead to ping-ponging as others have noted.  Better to



Here Alex mentioned the scenarios is 1)VRAM, 2)GTT, 3)VRAM/GTT.

His concern is this might cause ping-pong, not about preferred domain. Since preferred domain can solve the ping-pong issue, it shall address his concern here.



Regards,

Samuel Li



From: Christian König [mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
Sent: Monday, March 19, 2018 3:45 PM
To: Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org<mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>; Marek Olšák <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>; Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org<mailto:Christian.Koenig-5C7GfCeVMHo@public.gmane.org>>
Cc: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>; Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org<mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>>; amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support



Quoting Alex:

Regardless of which scenarios we need to support, I think we also need

to really plumb this through to mesa however since user space is who

ultimately requests the location.  Overriding it in the kernel gets

tricky and can lead to ping-ponging as others have noted.  Better to

have user space know what chips support it or not and request display

buffers in GTT or VRAM from the start.

And I completely agree with Alex here. So overriding the domain in the kernel is a serious NAK from my side as well.

Please implement the necessary bits in Mesa, shouldn't be more than a few lines of code anyway.

Regards,
Christian.

Am 19.03.2018 um 20:42 schrieb Li, Samuel:

Agreed.



>I think that the consensus with Alex and me is that we should avoid exactly that.
Christian, Alex's concern is about ping-pong, not about the preferred domain.

Regards,

Samuel Li



From: Marek Olšák [mailto:maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
Sent: Monday, March 19, 2018 3:39 PM
To: Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org><mailto:Christian.Koenig@amd.com>
Cc: Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org><mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org><mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>; Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org><mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; amd-gfx list <amd-gfx-PD4FTy7X32k@public.gmane.orgeedesktop.org><mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support



On Mon, Mar 19, 2018 at 3:27 PM, Christian König <christian.koenig-urvtwAKJhsc@public.gmane.orgm<mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>> wrote:

I think that the consensus with Alex and me is that we should avoid exactly that.

Overriding the preferred domain in the kernel is a no-go for that patch set, so please implement the discussed changes in Mesa.



I don't see how Mesa can make a smarter decision than the kernel. If you overwrite the preferred domain of the buffer in the kernel, there will be no ping-ponging between domains. Mesa never changes the initial preferred domain.



Marek





Regards,
Christian.


Am 19.03.2018 um 20:22 schrieb Li, Samuel:

I agree with Marek/Michel: since kernel sets the domain before scanning out, it shall update the preferred domain here too.

Regards,
Samuel Li

-----Original Message-----
From: Koenig, Christian
Sent: Thursday, March 08, 2018 4:07 AM
To: Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org<mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>>; Li, Samuel
<Samuel.Li-5C7GfCeVMHo@public.gmane.org<mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>; Alex Deucher <alexdeucher@gmail.com<mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>
Cc: amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lMiVNPc3mojA@public.gmane.orgsktop.org>>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

Am 08.03.2018 um 09:35 schrieb Michel Dänzer:

On 2018-03-07 10:47 AM, Christian König wrote:

Am 07.03.2018 um 09:42 schrieb Michel Dänzer:

On 2018-03-06 07:23 PM, Christian König wrote:

E.g. the last time I tested it placing things into GTT still
resulted in quite a performance penalty for rendering.

FWIW, I think the penalty is most likely IOMMU related. Last time I
tested, I couldn't measure a big difference with IOMMU disabled.

No, the penalty I'm talking about came from the ping/pong we did with
the scanout buffers.

See when I tested this the DDX and Mesa where unmodified, so both
still assumed VRAM as placement for scanout BOs, but the kernel
forced scanout BOs into GTT for testing.

So what happened was that on scanout we moved the VRAM BO to GTT

and

after unpinning it on the first command submission which used the BO
we moved it back to VRAM again.

In the meantime, I've had the same idea as Marek: Can't the kernel
driver simply change the BO's preferred domain to GTT when scanning
out from it? Then it won't move back to VRAM.

Yes, I've considered this as well.

But I think making the decision in Mesa is the cleaner approach.

E.g. so far we only override the placement decision of userspace for two
reasons:
1. We where running out of memory in VRAM.
2. We have a hardware restriction which makes VRAM usage mandatory.

And even then we never adjust the placement permanently, we just
temporary moved the buffer where it was needed and moved it back after
the operation completed.

Additional to that Mesa might want to set even more flags and/or changes
it's behavior. E.g. use a tilling mode which both importer and export in an
A+A laptop understands etc...

Regards,
Christian.

_______________________________________________
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





_______________________________________________

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



[-- Attachment #1.2: Type: text/html, Size: 27077 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Marek Olšák]

[-- Attachment #1.1: Type: text/plain, Size: 10786 bytes --]

When Mesa wants a buffer in VRAM, it always sets VRAM. It relies on BO move
throttling to prevent unnecessary BO moves.

My questions are:
- what should Mesa do differently for tiny VRAM?
- what is a tiny VRAM?
- if VRAM is tiny, which allocations should we put there?

Marek

On Mon, Mar 19, 2018 at 4:15 PM, Deucher, Alexander <
Alexander.Deucher-5C7GfCeVMHo@public.gmane.org> wrote:

> s/not/now/.  I meant to say, “we have NOW excluded the possibility of
> ever setting displays anywhere else without a kernel update”.
>
>
>
> Alex
>
>
>
> *From:* amd-gfx [mailto:amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org] *On Behalf
> Of *Deucher, Alexander
> *Sent:* Monday, March 19, 2018 4:13 PM
>
> *To:* Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Koenig, Christian <
> Christian.Koenig-5C7GfCeVMHo@public.gmane.org>; Marek Olšák <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> *Cc:* Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Michel Dänzer <
> michel-otUistvHUpPR7s880joybQ@public.gmane.org>; amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> *Subject:* Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display
> support
>
>
>
> I'm not sure what you mean by the 3 scenarios.  Generally userspace
> selects what domains it wants a buffer to be in, vram, gtt, or both (don't
> care).  I'd rather not have the kernel second guess the UMDs if we can help
> it.  I'd rather leave the kernel for cases where we have to force things
> due to hw bugs, or hw restrictions, etc.  If we force all display buffers
> to be in gtt in the kernel, we have not excluded the possibility of ever
> setting displays anywhere else without a kernel update.  E.g., to my
> earlier point, there may be cases where it is advantageous to put display
> buffers in vram even if s/g display is supported.  That was the point I was
> trying to make about user mode selecting the domain (vram of gtt or
> vram|gtt).  Say you have a board with 2 GB of ram and 1 GB is carved out
> for "vram".  In that case, it would make sense to put the buffer in vram
> because otherwise you are wasting a comparatively scarce resource.
>
>
>
> Alex
> ------------------------------
>
> *From:* Li, Samuel
> *Sent:* Monday, March 19, 2018 3:58:52 PM
> *To:* Deucher, Alexander; Koenig, Christian; Marek Olšák
> *Cc:* Alex Deucher; Michel Dänzer; amd-gfx list
> *Subject:* RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display
> support
>
>
>
> Alex,
>
>
>
> I assume you are talking the three scenarios here, 1)VRAM, 2)GTT,
> 3)VRAM/GTT.
>
> But kernel will need the decision too(amdgpufb). I think it shall be
> better to do it in kernel, instead of different clients(mesa, ddx, kernel …)
>
>
>
> Regards,
>
> Samuel Li
>
>
>
> *From:* Deucher, Alexander
> *Sent:* Monday, March 19, 2018 3:54 PM
> *To:* Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Koenig, Christian <
> Christian.Koenig-5C7GfCeVMHo@public.gmane.org>; Marek Olšák <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> *Cc:* Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Michel Dänzer <
> michel-otUistvHUpPR7s880joybQ@public.gmane.org>; amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> *Subject:* Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display
> support
>
>
>
> My personal preference is still to plumb this through to mesa rather than
> forcing it in the kernel.
>
>
>
> Alex
> ------------------------------
>
> *From:* amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Li,
> Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org>
> *Sent:* Monday, March 19, 2018 3:50:34 PM
> *To:* Koenig, Christian; Marek Olšák
> *Cc:* Alex Deucher; Michel Dänzer; amd-gfx list
> *Subject:* RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display
> support
>
>
>
> Christian,
>
>
>
> You misunderstood Alex’s comments,
>
>
>
> >Regardless of which scenarios we need to support, I think we also need
>
> >to really plumb this through to mesa however since user space is who
>
> >ultimately requests the location.  Overriding it in the kernel gets
>
> >tricky and can lead to ping-ponging as others have noted.  Better to
>
>
>
> Here Alex mentioned the scenarios is 1)VRAM, 2)GTT, 3)VRAM/GTT.
>
> His concern is this might cause ping-pong, not about preferred domain.
> Since preferred domain can solve the ping-pong issue, it shall address his
> concern here.
>
>
>
> Regards,
>
> Samuel Li
>
>
>
> *From:* Christian König [mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
> <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
> *Sent:* Monday, March 19, 2018 3:45 PM
> *To:* Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Marek Olšák <maraeo@gmail.com>;
> Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>
> *Cc:* Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Michel Dänzer <
> michel-otUistvHUpPR7s880joybQ@public.gmane.org>; amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> *Subject:* Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display
> support
>
>
>
> Quoting Alex:
>
> Regardless of which scenarios we need to support, I think we also need
>
> to really plumb this through to mesa however since user space is who
>
> ultimately requests the location.  Overriding it in the kernel gets
>
> tricky and can lead to ping-ponging as others have noted.  Better to
>
> have user space know what chips support it or not and request display
>
> buffers in GTT or VRAM from the start.
>
> And I completely agree with Alex here. So overriding the domain in the
> kernel is a serious NAK from my side as well.
>
> Please implement the necessary bits in Mesa, shouldn't be more than a few
> lines of code anyway.
>
> Regards,
> Christian.
>
> Am 19.03.2018 um 20:42 schrieb Li, Samuel:
>
> Agreed.
>
>
>
> >I think that the consensus with Alex and me is that we should avoid
> exactly that.
> Christian, Alex’s concern is about ping-pong, not about the preferred
> domain.
>
> Regards,
>
> Samuel Li
>
>
>
> *From:* Marek Olšák [mailto:maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
> *Sent:* Monday, March 19, 2018 3:39 PM
> *To:* Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>
> <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>
> *Cc:* Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org> <Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Michel Dänzer
> <michel-otUistvHUpPR7s880joybQ@public.gmane.org> <michel-otUistvHUpPR7s880joybQ@public.gmane.org>; Alex Deucher
> <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; amd-gfx list
> <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> *Subject:* Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display
> support
>
>
>
> On Mon, Mar 19, 2018 at 3:27 PM, Christian König <christian.koenig@amd.com>
> wrote:
>
> I think that the consensus with Alex and me is that we should avoid
> exactly that.
>
> Overriding the preferred domain in the kernel is a no-go for that patch
> set, so please implement the discussed changes in Mesa.
>
>
>
> I don't see how Mesa can make a smarter decision than the kernel. If you
> overwrite the preferred domain of the buffer in the kernel, there will be
> no ping-ponging between domains. Mesa never changes the initial preferred
> domain.
>
>
>
> Marek
>
>
>
>
>
>
> Regards,
> Christian.
>
>
>
> Am 19.03.2018 um 20:22 schrieb Li, Samuel:
>
> I agree with Marek/Michel: since kernel sets the domain before scanning
> out, it shall update the preferred domain here too.
>
> Regards,
> Samuel Li
>
> -----Original Message-----
> From: Koenig, Christian
> Sent: Thursday, March 08, 2018 4:07 AM
> To: Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org>; Li, Samuel
> <Samuel.Li-5C7GfCeVMHo@public.gmane.org>; Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>
> Am 08.03.2018 um 09:35 schrieb Michel Dänzer:
>
> On 2018-03-07 10:47 AM, Christian König wrote:
>
> Am 07.03.2018 um 09:42 schrieb Michel Dänzer:
>
> On 2018-03-06 07:23 PM, Christian König wrote:
>
> E.g. the last time I tested it placing things into GTT still
> resulted in quite a performance penalty for rendering.
>
> FWIW, I think the penalty is most likely IOMMU related. Last time I
> tested, I couldn't measure a big difference with IOMMU disabled.
>
> No, the penalty I'm talking about came from the ping/pong we did with
> the scanout buffers.
>
> See when I tested this the DDX and Mesa where unmodified, so both
> still assumed VRAM as placement for scanout BOs, but the kernel
> forced scanout BOs into GTT for testing.
>
> So what happened was that on scanout we moved the VRAM BO to GTT
>
> and
>
> after unpinning it on the first command submission which used the BO
> we moved it back to VRAM again.
>
> In the meantime, I've had the same idea as Marek: Can't the kernel
> driver simply change the BO's preferred domain to GTT when scanning
> out from it? Then it won't move back to VRAM.
>
> Yes, I've considered this as well.
>
> But I think making the decision in Mesa is the cleaner approach.
>
> E.g. so far we only override the placement decision of userspace for two
> reasons:
> 1. We where running out of memory in VRAM.
> 2. We have a hardware restriction which makes VRAM usage mandatory.
>
> And even then we never adjust the placement permanently, we just
> temporary moved the buffer where it was needed and moved it back after
> the operation completed.
>
> Additional to that Mesa might want to set even more flags and/or changes
> it's behavior. E.g. use a tilling mode which both importer and export in an
> A+A laptop understands etc...
>
> Regards,
> Christian.
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>
>
>
> _______________________________________________
>
> 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: 27682 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

RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Li, Samuel]

[-- Attachment #1.1: Type: text/plain, Size: 10928 bytes --]

>to my earlier point, there may be cases where it is advantageous to put display buffers in vram even if s/g display is supported
Agreed. That is also why the patch has the options to let user select where to put display buffers.
As whether to put the option in Mesa or kernel, it seems the difference is not much. Also, since amdgpufb can request even without mesa, kernel might be a better choice. In addition, putting in the kernel can save client’s duplicate work(mesa, ogl, vulkan, 2d, kernel…)

Regards,
Samuel Li

From: Marek Olšák [mailto:maraeo@gmail.com]
Sent: Monday, March 19, 2018 4:27 PM
To: Deucher, Alexander <Alexander.Deucher@amd.com>
Cc: Li, Samuel <Samuel.Li@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Alex Deucher <alexdeucher@gmail.com>; Michel Dänzer <michel@daenzer.net>; amd-gfx list <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

When Mesa wants a buffer in VRAM, it always sets VRAM. It relies on BO move throttling to prevent unnecessary BO moves.

My questions are:
- what should Mesa do differently for tiny VRAM?
- what is a tiny VRAM?
- if VRAM is tiny, which allocations should we put there?

Marek

On Mon, Mar 19, 2018 at 4:15 PM, Deucher, Alexander <Alexander.Deucher@amd.com<mailto:Alexander.Deucher@amd.com>> wrote:
s/not/now/.  I meant to say, “we have NOW excluded the possibility of ever setting displays anywhere else without a kernel update”.

Alex

From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>] On Behalf Of Deucher, Alexander
Sent: Monday, March 19, 2018 4:13 PM

To: Li, Samuel <Samuel.Li@amd.com<mailto:Samuel.Li@amd.com>>; Koenig, Christian <Christian.Koenig@amd.com<mailto:Christian.Koenig@amd.com>>; Marek Olšák <maraeo@gmail.com<mailto:maraeo@gmail.com>>
Cc: Alex Deucher <alexdeucher@gmail.com<mailto:alexdeucher@gmail.com>>; Michel Dänzer <michel@daenzer.net<mailto:michel@daenzer.net>>; amd-gfx list <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support


I'm not sure what you mean by the 3 scenarios.  Generally userspace selects what domains it wants a buffer to be in, vram, gtt, or both (don't care).  I'd rather not have the kernel second guess the UMDs if we can help it.  I'd rather leave the kernel for cases where we have to force things due to hw bugs, or hw restrictions, etc.  If we force all display buffers to be in gtt in the kernel, we have not excluded the possibility of ever setting displays anywhere else without a kernel update.  E.g., to my earlier point, there may be cases where it is advantageous to put display buffers in vram even if s/g display is supported.  That was the point I was trying to make about user mode selecting the domain (vram of gtt or vram|gtt).  Say you have a board with 2 GB of ram and 1 GB is carved out for "vram".  In that case, it would make sense to put the buffer in vram because otherwise you are wasting a comparatively scarce resource.



Alex

________________________________
From: Li, Samuel
Sent: Monday, March 19, 2018 3:58:52 PM
To: Deucher, Alexander; Koenig, Christian; Marek Olšák
Cc: Alex Deucher; Michel Dänzer; amd-gfx list
Subject: RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support


Alex,



I assume you are talking the three scenarios here, 1)VRAM, 2)GTT, 3)VRAM/GTT.

But kernel will need the decision too(amdgpufb). I think it shall be better to do it in kernel, instead of different clients(mesa, ddx, kernel …)



Regards,

Samuel Li



From: Deucher, Alexander
Sent: Monday, March 19, 2018 3:54 PM
To: Li, Samuel <Samuel.Li@amd.com<mailto:Samuel.Li@amd.com>>; Koenig, Christian <Christian.Koenig@amd.com<mailto:Christian.Koenig@amd.com>>; Marek Olšák <maraeo@gmail.com<mailto:maraeo@gmail.com>>
Cc: Alex Deucher <alexdeucher@gmail.com<mailto:alexdeucher@gmail.com>>; Michel Dänzer <michel@daenzer.net<mailto:michel@daenzer.net>>; amd-gfx list <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support



My personal preference is still to plumb this through to mesa rather than forcing it in the kernel.



Alex

________________________________

From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Li, Samuel <Samuel.Li@amd.com<mailto:Samuel.Li@amd.com>>
Sent: Monday, March 19, 2018 3:50:34 PM
To: Koenig, Christian; Marek Olšák
Cc: Alex Deucher; Michel Dänzer; amd-gfx list
Subject: RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support



Christian,



You misunderstood Alex’s comments,



>Regardless of which scenarios we need to support, I think we also need

>to really plumb this through to mesa however since user space is who

>ultimately requests the location.  Overriding it in the kernel gets

>tricky and can lead to ping-ponging as others have noted.  Better to



Here Alex mentioned the scenarios is 1)VRAM, 2)GTT, 3)VRAM/GTT.

His concern is this might cause ping-pong, not about preferred domain. Since preferred domain can solve the ping-pong issue, it shall address his concern here.



Regards,

Samuel Li



From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com]
Sent: Monday, March 19, 2018 3:45 PM
To: Li, Samuel <Samuel.Li@amd.com<mailto:Samuel.Li@amd.com>>; Marek Olšák <maraeo@gmail.com<mailto:maraeo@gmail.com>>; Koenig, Christian <Christian.Koenig@amd.com<mailto:Christian.Koenig@amd.com>>
Cc: Alex Deucher <alexdeucher@gmail.com<mailto:alexdeucher@gmail.com>>; Michel Dänzer <michel@daenzer.net<mailto:michel@daenzer.net>>; amd-gfx list <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support



Quoting Alex:

Regardless of which scenarios we need to support, I think we also need

to really plumb this through to mesa however since user space is who

ultimately requests the location.  Overriding it in the kernel gets

tricky and can lead to ping-ponging as others have noted.  Better to

have user space know what chips support it or not and request display

buffers in GTT or VRAM from the start.

And I completely agree with Alex here. So overriding the domain in the kernel is a serious NAK from my side as well.

Please implement the necessary bits in Mesa, shouldn't be more than a few lines of code anyway.

Regards,
Christian.

Am 19.03.2018 um 20:42 schrieb Li, Samuel:

Agreed.



>I think that the consensus with Alex and me is that we should avoid exactly that.
Christian, Alex’s concern is about ping-pong, not about the preferred domain.

Regards,

Samuel Li



From: Marek Olšák [mailto:maraeo@gmail.com]
Sent: Monday, March 19, 2018 3:39 PM
To: Koenig, Christian <Christian.Koenig@amd.com><mailto:Christian.Koenig@amd.com>
Cc: Li, Samuel <Samuel.Li@amd.com><mailto:Samuel.Li@amd.com>; Michel Dänzer <michel@daenzer.net><mailto:michel@daenzer.net>; Alex Deucher <alexdeucher@gmail.com><mailto:alexdeucher@gmail.com>; amd-gfx list <amd-gfx@lists.freedesktop.org><mailto:amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support



On Mon, Mar 19, 2018 at 3:27 PM, Christian König <christian.koenig@amd.com<mailto:christian.koenig@amd.com>> wrote:

I think that the consensus with Alex and me is that we should avoid exactly that.

Overriding the preferred domain in the kernel is a no-go for that patch set, so please implement the discussed changes in Mesa.



I don't see how Mesa can make a smarter decision than the kernel. If you overwrite the preferred domain of the buffer in the kernel, there will be no ping-ponging between domains. Mesa never changes the initial preferred domain.



Marek





Regards,
Christian.


Am 19.03.2018 um 20:22 schrieb Li, Samuel:

I agree with Marek/Michel: since kernel sets the domain before scanning out, it shall update the preferred domain here too.

Regards,
Samuel Li

-----Original Message-----
From: Koenig, Christian
Sent: Thursday, March 08, 2018 4:07 AM
To: Michel Dänzer <michel@daenzer.net<mailto:michel@daenzer.net>>; Li, Samuel
<Samuel.Li@amd.com<mailto:Samuel.Li@amd.com>>; Alex Deucher <alexdeucher@gmail.com<mailto:alexdeucher@gmail.com>>
Cc: amd-gfx list <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

Am 08.03.2018 um 09:35 schrieb Michel Dänzer:

On 2018-03-07 10:47 AM, Christian König wrote:

Am 07.03.2018 um 09:42 schrieb Michel Dänzer:

On 2018-03-06 07:23 PM, Christian König wrote:

E.g. the last time I tested it placing things into GTT still
resulted in quite a performance penalty for rendering.

FWIW, I think the penalty is most likely IOMMU related. Last time I
tested, I couldn't measure a big difference with IOMMU disabled.

No, the penalty I'm talking about came from the ping/pong we did with
the scanout buffers.

See when I tested this the DDX and Mesa where unmodified, so both
still assumed VRAM as placement for scanout BOs, but the kernel
forced scanout BOs into GTT for testing.

So what happened was that on scanout we moved the VRAM BO to GTT

and

after unpinning it on the first command submission which used the BO
we moved it back to VRAM again.

In the meantime, I've had the same idea as Marek: Can't the kernel
driver simply change the BO's preferred domain to GTT when scanning
out from it? Then it won't move back to VRAM.

Yes, I've considered this as well.

But I think making the decision in Mesa is the cleaner approach.

E.g. so far we only override the placement decision of userspace for two
reasons:
1. We where running out of memory in VRAM.
2. We have a hardware restriction which makes VRAM usage mandatory.

And even then we never adjust the placement permanently, we just
temporary moved the buffer where it was needed and moved it back after
the operation completed.

Additional to that Mesa might want to set even more flags and/or changes
it's behavior. E.g. use a tilling mode which both importer and export in an
A+A laptop understands etc...

Regards,
Christian.

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





_______________________________________________

amd-gfx mailing list

amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>

https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[-- Attachment #1.2: Type: text/html, Size: 31980 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Alex Deucher]

On Mon, Mar 19, 2018 at 4:26 PM, Marek Olšák <maraeo@gmail.com> wrote:
> When Mesa wants a buffer in VRAM, it always sets VRAM. It relies on BO move
> throttling to prevent unnecessary BO moves.
>
> My questions are:
> - what should Mesa do differently for tiny VRAM?
> - what is a tiny VRAM?
> - if VRAM is tiny, which allocations should we put there?

Right now, all mesa would have to do is query the kernel to see if the
device supports s/g display.  If it does, it can do whatever makes
sense in the long term once we've had more time to benchmark, etc.,
but to start off with, I'd say just make it GTT or VRAM|GTT.
Otherwise we have the kernel second guessing mesa and I'd like to
avoid that.  I want the kernel to respect that userspace asks for as
much as possible.

Alex

>
> Marek
>
> On Mon, Mar 19, 2018 at 4:15 PM, Deucher, Alexander
> <Alexander.Deucher@amd.com> wrote:
>>
>> s/not/now/.  I meant to say, “we have NOW excluded the possibility of ever
>> setting displays anywhere else without a kernel update”.
>>
>>
>>
>> Alex
>>
>>
>>
>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of
>> Deucher, Alexander
>> Sent: Monday, March 19, 2018 4:13 PM
>>
>>
>> To: Li, Samuel <Samuel.Li@amd.com>; Koenig, Christian
>> <Christian.Koenig@amd.com>; Marek Olšák <maraeo@gmail.com>
>> Cc: Alex Deucher <alexdeucher@gmail.com>; Michel Dänzer
>> <michel@daenzer.net>; amd-gfx list <amd-gfx@lists.freedesktop.org>
>> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>>
>>
>>
>> I'm not sure what you mean by the 3 scenarios.  Generally userspace
>> selects what domains it wants a buffer to be in, vram, gtt, or both (don't
>> care).  I'd rather not have the kernel second guess the UMDs if we can help
>> it.  I'd rather leave the kernel for cases where we have to force things due
>> to hw bugs, or hw restrictions, etc.  If we force all display buffers to be
>> in gtt in the kernel, we have not excluded the possibility of ever setting
>> displays anywhere else without a kernel update.  E.g., to my earlier point,
>> there may be cases where it is advantageous to put display buffers in vram
>> even if s/g display is supported.  That was the point I was trying to make
>> about user mode selecting the domain (vram of gtt or vram|gtt).  Say you
>> have a board with 2 GB of ram and 1 GB is carved out for "vram".  In that
>> case, it would make sense to put the buffer in vram because otherwise you
>> are wasting a comparatively scarce resource.
>>
>>
>>
>> Alex
>>
>> ________________________________
>>
>> From: Li, Samuel
>> Sent: Monday, March 19, 2018 3:58:52 PM
>> To: Deucher, Alexander; Koenig, Christian; Marek Olšák
>> Cc: Alex Deucher; Michel Dänzer; amd-gfx list
>> Subject: RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>>
>>
>>
>> Alex,
>>
>>
>>
>> I assume you are talking the three scenarios here, 1)VRAM, 2)GTT,
>> 3)VRAM/GTT.
>>
>> But kernel will need the decision too(amdgpufb). I think it shall be
>> better to do it in kernel, instead of different clients(mesa, ddx, kernel …)
>>
>>
>>
>> Regards,
>>
>> Samuel Li
>>
>>
>>
>> From: Deucher, Alexander
>> Sent: Monday, March 19, 2018 3:54 PM
>> To: Li, Samuel <Samuel.Li@amd.com>; Koenig, Christian
>> <Christian.Koenig@amd.com>; Marek Olšák <maraeo@gmail.com>
>> Cc: Alex Deucher <alexdeucher@gmail.com>; Michel Dänzer
>> <michel@daenzer.net>; amd-gfx list <amd-gfx@lists.freedesktop.org>
>> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>>
>>
>>
>> My personal preference is still to plumb this through to mesa rather than
>> forcing it in the kernel.
>>
>>
>>
>> Alex
>>
>> ________________________________
>>
>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Li,
>> Samuel <Samuel.Li@amd.com>
>> Sent: Monday, March 19, 2018 3:50:34 PM
>> To: Koenig, Christian; Marek Olšák
>> Cc: Alex Deucher; Michel Dänzer; amd-gfx list
>> Subject: RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>>
>>
>>
>> Christian,
>>
>>
>>
>> You misunderstood Alex’s comments,
>>
>>
>>
>> >Regardless of which scenarios we need to support, I think we also need
>>
>> >to really plumb this through to mesa however since user space is who
>>
>> >ultimately requests the location.  Overriding it in the kernel gets
>>
>> >tricky and can lead to ping-ponging as others have noted.  Better to
>>
>>
>>
>> Here Alex mentioned the scenarios is 1)VRAM, 2)GTT, 3)VRAM/GTT.
>>
>> His concern is this might cause ping-pong, not about preferred domain.
>> Since preferred domain can solve the ping-pong issue, it shall address his
>> concern here.
>>
>>
>>
>> Regards,
>>
>> Samuel Li
>>
>>
>>
>> From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com]
>> Sent: Monday, March 19, 2018 3:45 PM
>> To: Li, Samuel <Samuel.Li@amd.com>; Marek Olšák <maraeo@gmail.com>;
>> Koenig, Christian <Christian.Koenig@amd.com>
>> Cc: Alex Deucher <alexdeucher@gmail.com>; Michel Dänzer
>> <michel@daenzer.net>; amd-gfx list <amd-gfx@lists.freedesktop.org>
>> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>>
>>
>>
>> Quoting Alex:
>>
>> Regardless of which scenarios we need to support, I think we also need
>>
>> to really plumb this through to mesa however since user space is who
>>
>> ultimately requests the location.  Overriding it in the kernel gets
>>
>> tricky and can lead to ping-ponging as others have noted.  Better to
>>
>> have user space know what chips support it or not and request display
>>
>> buffers in GTT or VRAM from the start.
>>
>> And I completely agree with Alex here. So overriding the domain in the
>> kernel is a serious NAK from my side as well.
>>
>> Please implement the necessary bits in Mesa, shouldn't be more than a few
>> lines of code anyway.
>>
>> Regards,
>> Christian.
>>
>> Am 19.03.2018 um 20:42 schrieb Li, Samuel:
>>
>> Agreed.
>>
>>
>>
>> >I think that the consensus with Alex and me is that we should avoid
>> > exactly that.
>> Christian, Alex’s concern is about ping-pong, not about the preferred
>> domain.
>>
>> Regards,
>>
>> Samuel Li
>>
>>
>>
>> From: Marek Olšák [mailto:maraeo@gmail.com]
>> Sent: Monday, March 19, 2018 3:39 PM
>> To: Koenig, Christian <Christian.Koenig@amd.com>
>> Cc: Li, Samuel <Samuel.Li@amd.com>; Michel Dänzer <michel@daenzer.net>;
>> Alex Deucher <alexdeucher@gmail.com>; amd-gfx list
>> <amd-gfx@lists.freedesktop.org>
>> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>>
>>
>>
>> On Mon, Mar 19, 2018 at 3:27 PM, Christian König
>> <christian.koenig@amd.com> wrote:
>>
>> I think that the consensus with Alex and me is that we should avoid
>> exactly that.
>>
>> Overriding the preferred domain in the kernel is a no-go for that patch
>> set, so please implement the discussed changes in Mesa.
>>
>>
>>
>> I don't see how Mesa can make a smarter decision than the kernel. If you
>> overwrite the preferred domain of the buffer in the kernel, there will be no
>> ping-ponging between domains. Mesa never changes the initial preferred
>> domain.
>>
>>
>>
>> Marek
>>
>>
>>
>>
>>
>>
>> Regards,
>> Christian.
>>
>>
>>
>> Am 19.03.2018 um 20:22 schrieb Li, Samuel:
>>
>> I agree with Marek/Michel: since kernel sets the domain before scanning
>> out, it shall update the preferred domain here too.
>>
>> Regards,
>> Samuel Li
>>
>> -----Original Message-----
>> From: Koenig, Christian
>> Sent: Thursday, March 08, 2018 4:07 AM
>> To: Michel Dänzer <michel@daenzer.net>; Li, Samuel
>> <Samuel.Li@amd.com>; Alex Deucher <alexdeucher@gmail.com>
>> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
>> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>>
>> Am 08.03.2018 um 09:35 schrieb Michel Dänzer:
>>
>> On 2018-03-07 10:47 AM, Christian König wrote:
>>
>> Am 07.03.2018 um 09:42 schrieb Michel Dänzer:
>>
>> On 2018-03-06 07:23 PM, Christian König wrote:
>>
>> E.g. the last time I tested it placing things into GTT still
>> resulted in quite a performance penalty for rendering.
>>
>> FWIW, I think the penalty is most likely IOMMU related. Last time I
>> tested, I couldn't measure a big difference with IOMMU disabled.
>>
>> No, the penalty I'm talking about came from the ping/pong we did with
>> the scanout buffers.
>>
>> See when I tested this the DDX and Mesa where unmodified, so both
>> still assumed VRAM as placement for scanout BOs, but the kernel
>> forced scanout BOs into GTT for testing.
>>
>> So what happened was that on scanout we moved the VRAM BO to GTT
>>
>> and
>>
>> after unpinning it on the first command submission which used the BO
>> we moved it back to VRAM again.
>>
>> In the meantime, I've had the same idea as Marek: Can't the kernel
>> driver simply change the BO's preferred domain to GTT when scanning
>> out from it? Then it won't move back to VRAM.
>>
>> Yes, I've considered this as well.
>>
>> But I think making the decision in Mesa is the cleaner approach.
>>
>> E.g. so far we only override the placement decision of userspace for two
>> reasons:
>> 1. We where running out of memory in VRAM.
>> 2. We have a hardware restriction which makes VRAM usage mandatory.
>>
>> And even then we never adjust the placement permanently, we just
>> temporary moved the buffer where it was needed and moved it back after
>> the operation completed.
>>
>> Additional to that Mesa might want to set even more flags and/or changes
>> it's behavior. E.g. use a tilling mode which both importer and export in
>> an
>> A+A laptop understands etc...
>>
>> Regards,
>> Christian.
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>>
>>
>>
>>
>> _______________________________________________
>>
>> amd-gfx mailing list
>>
>> amd-gfx@lists.freedesktop.org
>>
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>>
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Marek Olšák]

[-- Attachment #1.1: Type: text/plain, Size: 994 bytes --]

On Mon, Mar 19, 2018 at 4:53 PM, Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Mon, Mar 19, 2018 at 4:26 PM, Marek Olšák <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > When Mesa wants a buffer in VRAM, it always sets VRAM. It relies on BO
> move
> > throttling to prevent unnecessary BO moves.
> >
> > My questions are:
> > - what should Mesa do differently for tiny VRAM?
> > - what is a tiny VRAM?
> > - if VRAM is tiny, which allocations should we put there?
>
> Right now, all mesa would have to do is query the kernel to see if the
> device supports s/g display.  If it does, it can do whatever makes
> sense in the long term once we've had more time to benchmark, etc.,
> but to start off with, I'd say just make it GTT or VRAM|GTT.
> Otherwise we have the kernel second guessing mesa and I'd like to
> avoid that.  I want the kernel to respect that userspace asks for as
> much as possible.
>

Sounds good.

Marek

[-- Attachment #1.2: Type: text/html, Size: 1486 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Alex Deucher]

On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel <Samuel.Li@amd.com> wrote:
>>to my earlier point, there may be cases where it is advantageous to put
>> display buffers in vram even if s/g display is supported
>
> Agreed. That is also why the patch has the options to let user select where
> to put display buffers.
>
> As whether to put the option in Mesa or kernel, it seems the difference is
> not much. Also, since amdgpufb can request even without mesa, kernel might
> be a better choice. In addition, putting in the kernel can save client’s
> duplicate work(mesa, ogl, vulkan, 2d, kernel…)

Why do we even expose different memory pools to the UMDs in the first
place ;)  Each pool has performance characteristics that may be
relevant for a particular work load.  Only the UMDs really know the
finer points of those workloads. In general, you don't want the kernel
dictating policy if you can avoid it.  The kernel exposes
functionality and userspace sets the policy.  With the location set in
userspace, each app/user can have whatever policy makes sense for
their use case all at the same time without needing to tweak their
kernel for every use case.

Alex

>
>
>
> Regards,
>
> Samuel Li
>
>
>
> From: Marek Olšák [mailto:maraeo@gmail.com]
> Sent: Monday, March 19, 2018 4:27 PM
> To: Deucher, Alexander <Alexander.Deucher@amd.com>
> Cc: Li, Samuel <Samuel.Li@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Alex Deucher <alexdeucher@gmail.com>; Michel
> Dänzer <michel@daenzer.net>; amd-gfx list <amd-gfx@lists.freedesktop.org>
>
>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>
>
>
> When Mesa wants a buffer in VRAM, it always sets VRAM. It relies on BO move
> throttling to prevent unnecessary BO moves.
>
>
>
> My questions are:
>
> - what should Mesa do differently for tiny VRAM?
>
> - what is a tiny VRAM?
>
> - if VRAM is tiny, which allocations should we put there?
>
>
>
> Marek
>
>
>
> On Mon, Mar 19, 2018 at 4:15 PM, Deucher, Alexander
> <Alexander.Deucher@amd.com> wrote:
>
> s/not/now/.  I meant to say, “we have NOW excluded the possibility of ever
> setting displays anywhere else without a kernel update”.
>
>
>
> Alex
>
>
>
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Deucher, Alexander
> Sent: Monday, March 19, 2018 4:13 PM
>
>
> To: Li, Samuel <Samuel.Li@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Marek Olšák <maraeo@gmail.com>
> Cc: Alex Deucher <alexdeucher@gmail.com>; Michel Dänzer
> <michel@daenzer.net>; amd-gfx list <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>
>
>
> I'm not sure what you mean by the 3 scenarios.  Generally userspace selects
> what domains it wants a buffer to be in, vram, gtt, or both (don't care).
> I'd rather not have the kernel second guess the UMDs if we can help it.  I'd
> rather leave the kernel for cases where we have to force things due to hw
> bugs, or hw restrictions, etc.  If we force all display buffers to be in gtt
> in the kernel, we have not excluded the possibility of ever setting displays
> anywhere else without a kernel update.  E.g., to my earlier point, there may
> be cases where it is advantageous to put display buffers in vram even if s/g
> display is supported.  That was the point I was trying to make about user
> mode selecting the domain (vram of gtt or vram|gtt).  Say you have a board
> with 2 GB of ram and 1 GB is carved out for "vram".  In that case, it would
> make sense to put the buffer in vram because otherwise you are wasting a
> comparatively scarce resource.
>
>
>
> Alex
>
> ________________________________
>
> From: Li, Samuel
> Sent: Monday, March 19, 2018 3:58:52 PM
> To: Deucher, Alexander; Koenig, Christian; Marek Olšák
> Cc: Alex Deucher; Michel Dänzer; amd-gfx list
> Subject: RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>
>
>
> Alex,
>
>
>
> I assume you are talking the three scenarios here, 1)VRAM, 2)GTT,
> 3)VRAM/GTT.
>
> But kernel will need the decision too(amdgpufb). I think it shall be better
> to do it in kernel, instead of different clients(mesa, ddx, kernel …)
>
>
>
> Regards,
>
> Samuel Li
>
>
>
> From: Deucher, Alexander
> Sent: Monday, March 19, 2018 3:54 PM
> To: Li, Samuel <Samuel.Li@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Marek Olšák <maraeo@gmail.com>
> Cc: Alex Deucher <alexdeucher@gmail.com>; Michel Dänzer
> <michel@daenzer.net>; amd-gfx list <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>
>
>
> My personal preference is still to plumb this through to mesa rather than
> forcing it in the kernel.
>
>
>
> Alex
>
> ________________________________
>
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Li,
> Samuel <Samuel.Li@amd.com>
> Sent: Monday, March 19, 2018 3:50:34 PM
> To: Koenig, Christian; Marek Olšák
> Cc: Alex Deucher; Michel Dänzer; amd-gfx list
> Subject: RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>
>
>
> Christian,
>
>
>
> You misunderstood Alex’s comments,
>
>
>
>>Regardless of which scenarios we need to support, I think we also need
>
>>to really plumb this through to mesa however since user space is who
>
>>ultimately requests the location.  Overriding it in the kernel gets
>
>>tricky and can lead to ping-ponging as others have noted.  Better to
>
>
>
> Here Alex mentioned the scenarios is 1)VRAM, 2)GTT, 3)VRAM/GTT.
>
> His concern is this might cause ping-pong, not about preferred domain. Since
> preferred domain can solve the ping-pong issue, it shall address his concern
> here.
>
>
>
> Regards,
>
> Samuel Li
>
>
>
> From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com]
> Sent: Monday, March 19, 2018 3:45 PM
> To: Li, Samuel <Samuel.Li@amd.com>; Marek Olšák <maraeo@gmail.com>; Koenig,
> Christian <Christian.Koenig@amd.com>
> Cc: Alex Deucher <alexdeucher@gmail.com>; Michel Dänzer
> <michel@daenzer.net>; amd-gfx list <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>
>
>
> Quoting Alex:
>
> Regardless of which scenarios we need to support, I think we also need
>
> to really plumb this through to mesa however since user space is who
>
> ultimately requests the location.  Overriding it in the kernel gets
>
> tricky and can lead to ping-ponging as others have noted.  Better to
>
> have user space know what chips support it or not and request display
>
> buffers in GTT or VRAM from the start.
>
> And I completely agree with Alex here. So overriding the domain in the
> kernel is a serious NAK from my side as well.
>
> Please implement the necessary bits in Mesa, shouldn't be more than a few
> lines of code anyway.
>
> Regards,
> Christian.
>
> Am 19.03.2018 um 20:42 schrieb Li, Samuel:
>
> Agreed.
>
>
>
>>I think that the consensus with Alex and me is that we should avoid exactly
>> that.
> Christian, Alex’s concern is about ping-pong, not about the preferred
> domain.
>
> Regards,
>
> Samuel Li
>
>
>
> From: Marek Olšák [mailto:maraeo@gmail.com]
> Sent: Monday, March 19, 2018 3:39 PM
> To: Koenig, Christian <Christian.Koenig@amd.com>
> Cc: Li, Samuel <Samuel.Li@amd.com>; Michel Dänzer <michel@daenzer.net>; Alex
> Deucher <alexdeucher@gmail.com>; amd-gfx list
> <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>
>
>
> On Mon, Mar 19, 2018 at 3:27 PM, Christian König <christian.koenig@amd.com>
> wrote:
>
> I think that the consensus with Alex and me is that we should avoid exactly
> that.
>
> Overriding the preferred domain in the kernel is a no-go for that patch set,
> so please implement the discussed changes in Mesa.
>
>
>
> I don't see how Mesa can make a smarter decision than the kernel. If you
> overwrite the preferred domain of the buffer in the kernel, there will be no
> ping-ponging between domains. Mesa never changes the initial preferred
> domain.
>
>
>
> Marek
>
>
>
>
>
>
> Regards,
> Christian.
>
>
>
> Am 19.03.2018 um 20:22 schrieb Li, Samuel:
>
> I agree with Marek/Michel: since kernel sets the domain before scanning out,
> it shall update the preferred domain here too.
>
> Regards,
> Samuel Li
>
> -----Original Message-----
> From: Koenig, Christian
> Sent: Thursday, March 08, 2018 4:07 AM
> To: Michel Dänzer <michel@daenzer.net>; Li, Samuel
> <Samuel.Li@amd.com>; Alex Deucher <alexdeucher@gmail.com>
> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
>
> Am 08.03.2018 um 09:35 schrieb Michel Dänzer:
>
> On 2018-03-07 10:47 AM, Christian König wrote:
>
> Am 07.03.2018 um 09:42 schrieb Michel Dänzer:
>
> On 2018-03-06 07:23 PM, Christian König wrote:
>
> E.g. the last time I tested it placing things into GTT still
> resulted in quite a performance penalty for rendering.
>
> FWIW, I think the penalty is most likely IOMMU related. Last time I
> tested, I couldn't measure a big difference with IOMMU disabled.
>
> No, the penalty I'm talking about came from the ping/pong we did with
> the scanout buffers.
>
> See when I tested this the DDX and Mesa where unmodified, so both
> still assumed VRAM as placement for scanout BOs, but the kernel
> forced scanout BOs into GTT for testing.
>
> So what happened was that on scanout we moved the VRAM BO to GTT
>
> and
>
> after unpinning it on the first command submission which used the BO
> we moved it back to VRAM again.
>
> In the meantime, I've had the same idea as Marek: Can't the kernel
> driver simply change the BO's preferred domain to GTT when scanning
> out from it? Then it won't move back to VRAM.
>
> Yes, I've considered this as well.
>
> But I think making the decision in Mesa is the cleaner approach.
>
> E.g. so far we only override the placement decision of userspace for two
> reasons:
> 1. We where running out of memory in VRAM.
> 2. We have a hardware restriction which makes VRAM usage mandatory.
>
> And even then we never adjust the placement permanently, we just
> temporary moved the buffer where it was needed and moved it back after
> the operation completed.
>
> Additional to that Mesa might want to set even more flags and/or changes
> it's behavior. E.g. use a tilling mode which both importer and export in an
> A+A laptop understands etc...
>
> Regards,
> Christian.
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Marek Olšák]

[-- Attachment #1.1: Type: text/plain, Size: 1793 bytes --]

In theory, Mesa doesn't have to do anything. It can continue setting VRAM
and if the kernel has to put a display buffer into GTT, it doesn't matter
(for Mesa). Whether the VRAM placement is really used is largely determined
by BO priorities.

The way I understand scather/gather is that it only allows the GTT
placement. It doesn't force the GTT placement. Mesa also doesn't force the
GTT placement.

Marek

On Mon, Mar 19, 2018 at 5:12 PM, Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org> wrote:
> >>to my earlier point, there may be cases where it is advantageous to put
> >> display buffers in vram even if s/g display is supported
> >
> > Agreed. That is also why the patch has the options to let user select
> where
> > to put display buffers.
> >
> > As whether to put the option in Mesa or kernel, it seems the difference
> is
> > not much. Also, since amdgpufb can request even without mesa, kernel
> might
> > be a better choice. In addition, putting in the kernel can save client’s
> > duplicate work(mesa, ogl, vulkan, 2d, kernel…)
>
> Why do we even expose different memory pools to the UMDs in the first
> place ;)  Each pool has performance characteristics that may be
> relevant for a particular work load.  Only the UMDs really know the
> finer points of those workloads. In general, you don't want the kernel
> dictating policy if you can avoid it.  The kernel exposes
> functionality and userspace sets the policy.  With the location set in
> userspace, each app/user can have whatever policy makes sense for
> their use case all at the same time without needing to tweak their
> kernel for every use case.
>
> Alex
>

[-- Attachment #1.2: Type: text/html, Size: 2571 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 2556 bytes --]

Yes, exactly. And if I remember correctly Mesa used to always set GTT as 
fallback on APUs, correct?

The problem seems to be that this fallback isn't set for displayable BOs.

So what needs to be done is to just enable this fallback for displayable 
BOs as well if the kernel can handle it.

Christian.

Am 20.03.2018 um 00:01 schrieb Marek Olšák:
> In theory, Mesa doesn't have to do anything. It can continue setting 
> VRAM and if the kernel has to put a display buffer into GTT, it 
> doesn't matter (for Mesa). Whether the VRAM placement is really used 
> is largely determined by BO priorities.
>
> The way I understand scather/gather is that it only allows the GTT 
> placement. It doesn't force the GTT placement. Mesa also doesn't force 
> the GTT placement.
>
> Marek
>
> On Mon, Mar 19, 2018 at 5:12 PM, Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org 
> <mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
>
>     On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org
>     <mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>> wrote:
>     >>to my earlier point, there may be cases where it is advantageous
>     to put
>     >> display buffers in vram even if s/g display is supported
>     >
>     > Agreed. That is also why the patch has the options to let user
>     select where
>     > to put display buffers.
>     >
>     > As whether to put the option in Mesa or kernel, it seems the
>     difference is
>     > not much. Also, since amdgpufb can request even without mesa,
>     kernel might
>     > be a better choice. In addition, putting in the kernel can save
>     client’s
>     > duplicate work(mesa, ogl, vulkan, 2d, kernel…)
>
>     Why do we even expose different memory pools to the UMDs in the first
>     place ;)  Each pool has performance characteristics that may be
>     relevant for a particular work load.  Only the UMDs really know the
>     finer points of those workloads. In general, you don't want the kernel
>     dictating policy if you can avoid it.  The kernel exposes
>     functionality and userspace sets the policy.  With the location set in
>     userspace, each app/user can have whatever policy makes sense for
>     their use case all at the same time without needing to tweak their
>     kernel for every use case.
>
>     Alex
>
>
>
> _______________________________________________
> 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: 4869 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Marek Olšák]

[-- Attachment #1.1: Type: text/plain, Size: 2771 bytes --]

On Tue, Mar 20, 2018 at 9:55 AM, Christian König <
ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Yes, exactly. And if I remember correctly Mesa used to always set GTT as
> fallback on APUs, correct?
>

"used to" is the key part. Mesa doesn't force GTT on APUs anymore. It
expects that the combination of BO priorities and BO move throttling will
result in optimal BO placements over time.

Marek


>
> The problem seems to be that this fallback isn't set for displayable BOs.
>
> So what needs to be done is to just enable this fallback for displayable
> BOs as well if the kernel can handle it.
>
> Christian.
>
>
> Am 20.03.2018 um 00:01 schrieb Marek Olšák:
>
> In theory, Mesa doesn't have to do anything. It can continue setting VRAM
> and if the kernel has to put a display buffer into GTT, it doesn't matter
> (for Mesa). Whether the VRAM placement is really used is largely determined
> by BO priorities.
>
> The way I understand scather/gather is that it only allows the GTT
> placement. It doesn't force the GTT placement. Mesa also doesn't force the
> GTT placement.
>
> Marek
>
> On Mon, Mar 19, 2018 at 5:12 PM, Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> wrote:
>
>> On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org> wrote:
>> >>to my earlier point, there may be cases where it is advantageous to put
>> >> display buffers in vram even if s/g display is supported
>> >
>> > Agreed. That is also why the patch has the options to let user select
>> where
>> > to put display buffers.
>> >
>> > As whether to put the option in Mesa or kernel, it seems the difference
>> is
>> > not much. Also, since amdgpufb can request even without mesa, kernel
>> might
>> > be a better choice. In addition, putting in the kernel can save client’s
>> > duplicate work(mesa, ogl, vulkan, 2d, kernel…)
>>
>> Why do we even expose different memory pools to the UMDs in the first
>> place ;)  Each pool has performance characteristics that may be
>> relevant for a particular work load.  Only the UMDs really know the
>> finer points of those workloads. In general, you don't want the kernel
>> dictating policy if you can avoid it.  The kernel exposes
>> functionality and userspace sets the policy.  With the location set in
>> userspace, each app/user can have whatever policy makes sense for
>> their use case all at the same time without needing to tweak their
>> kernel for every use case.
>>
>> Alex
>>
>
>
> _______________________________________________
> amd-gfx mailing listamd-gfx-PD4FTy7X32lNgt0PjOBp934avgP/u3fG0wdF1cv0I5s@public.gmane.org://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 6005 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 3994 bytes --]

I don't think that is a good idea.

Ideally GTT should now have the same performance as VRAM on APUs and we 
should use VRAM only for things where we absolutely have to and to 
actually use up the otherwise unused VRAM.

Can you run some tests with all BOs forced to GTT and see if there is 
any performance regression?

Christian.

Am 20.03.2018 um 15:51 schrieb Marek Olšák:
> On Tue, Mar 20, 2018 at 9:55 AM, Christian König 
> <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org 
> <mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
>
>     Yes, exactly. And if I remember correctly Mesa used to always set
>     GTT as fallback on APUs, correct?
>
>
> "used to" is the key part. Mesa doesn't force GTT on APUs anymore. It 
> expects that the combination of BO priorities and BO move throttling 
> will result in optimal BO placements over time.
>
> Marek
>
>
>     The problem seems to be that this fallback isn't set for
>     displayable BOs.
>
>     So what needs to be done is to just enable this fallback for
>     displayable BOs as well if the kernel can handle it.
>
>     Christian.
>
>
>     Am 20.03.2018 um 00:01 schrieb Marek Olšák:
>>     In theory, Mesa doesn't have to do anything. It can continue
>>     setting VRAM and if the kernel has to put a display buffer into
>>     GTT, it doesn't matter (for Mesa). Whether the VRAM placement is
>>     really used is largely determined by BO priorities.
>>
>>     The way I understand scather/gather is that it only allows the
>>     GTT placement. It doesn't force the GTT placement. Mesa also
>>     doesn't force the GTT placement.
>>
>>     Marek
>>
>>     On Mon, Mar 19, 2018 at 5:12 PM, Alex Deucher
>>     <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org <mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
>>
>>         On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel
>>         <Samuel.Li-5C7GfCeVMHo@public.gmane.org <mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>> wrote:
>>         >>to my earlier point, there may be cases where it is
>>         advantageous to put
>>         >> display buffers in vram even if s/g display is supported
>>         >
>>         > Agreed. That is also why the patch has the options to let
>>         user select where
>>         > to put display buffers.
>>         >
>>         > As whether to put the option in Mesa or kernel, it seems
>>         the difference is
>>         > not much. Also, since amdgpufb can request even without
>>         mesa, kernel might
>>         > be a better choice. In addition, putting in the kernel can
>>         save client’s
>>         > duplicate work(mesa, ogl, vulkan, 2d, kernel…)
>>
>>         Why do we even expose different memory pools to the UMDs in
>>         the first
>>         place ;)  Each pool has performance characteristics that may be
>>         relevant for a particular work load.  Only the UMDs really
>>         know the
>>         finer points of those workloads. In general, you don't want
>>         the kernel
>>         dictating policy if you can avoid it.  The kernel exposes
>>         functionality and userspace sets the policy.  With the
>>         location set in
>>         userspace, each app/user can have whatever policy makes sense for
>>         their use case all at the same time without needing to tweak
>>         their
>>         kernel for every use case.
>>
>>         Alex
>>
>>
>>
>>     _______________________________________________
>>     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>
>
>
>
>
> _______________________________________________
> 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: 9796 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Deucher, Alexander]

[-- Attachment #1.1: Type: text/plain, Size: 4096 bytes --]

I think we can also have the case of systems with similar amounts of carve out and system ram.  E.g., on a system with 4 GB of system memory with 1 GB carved out for vram.  It would be a big waste not to use VRAM.  We'll probably need a heuristic at some point.


Alex

________________________________
From: Christian König <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Sent: Tuesday, March 20, 2018 2:32:49 PM
To: Marek Olšák; Koenig, Christian
Cc: Alex Deucher; Deucher, Alexander; Michel Dänzer; Li, Samuel; amd-gfx list
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

I don't think that is a good idea.

Ideally GTT should now have the same performance as VRAM on APUs and we should use VRAM only for things where we absolutely have to and to actually use up the otherwise unused VRAM.

Can you run some tests with all BOs forced to GTT and see if there is any performance regression?

Christian.

Am 20.03.2018 um 15:51 schrieb Marek Olšák:
On Tue, Mar 20, 2018 at 9:55 AM, Christian König <ckoenig.leichtzumerken@gmail.com<mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
Yes, exactly. And if I remember correctly Mesa used to always set GTT as fallback on APUs, correct?

"used to" is the key part. Mesa doesn't force GTT on APUs anymore. It expects that the combination of BO priorities and BO move throttling will result in optimal BO placements over time.

Marek


The problem seems to be that this fallback isn't set for displayable BOs.

So what needs to be done is to just enable this fallback for displayable BOs as well if the kernel can handle it.

Christian.


Am 20.03.2018 um 00:01 schrieb Marek Olšák:
In theory, Mesa doesn't have to do anything. It can continue setting VRAM and if the kernel has to put a display buffer into GTT, it doesn't matter (for Mesa). Whether the VRAM placement is really used is largely determined by BO priorities.

The way I understand scather/gather is that it only allows the GTT placement. It doesn't force the GTT placement. Mesa also doesn't force the GTT placement.

Marek

On Mon, Mar 19, 2018 at 5:12 PM, Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org<mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>> wrote:
>>to my earlier point, there may be cases where it is advantageous to put
>> display buffers in vram even if s/g display is supported
>
> Agreed. That is also why the patch has the options to let user select where
> to put display buffers.
>
> As whether to put the option in Mesa or kernel, it seems the difference is
> not much. Also, since amdgpufb can request even without mesa, kernel might
> be a better choice. In addition, putting in the kernel can save client’s
> duplicate work(mesa, ogl, vulkan, 2d, kernel…)

Why do we even expose different memory pools to the UMDs in the first
place ;)  Each pool has performance characteristics that may be
relevant for a particular work load.  Only the UMDs really know the
finer points of those workloads. In general, you don't want the kernel
dictating policy if you can avoid it.  The kernel exposes
functionality and userspace sets the policy.  With the location set in
userspace, each app/user can have whatever policy makes sense for
their use case all at the same time without needing to tweak their
kernel for every use case.

Alex



_______________________________________________
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






_______________________________________________
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



[-- Attachment #1.2: Type: text/html, Size: 7518 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 5392 bytes --]

That's what I meant with use up the otherwise unused VRAM. I don't see 
any disadvantage of always setting GTT as second domain on APUs.

My assumption was that we dropped this in userspace for displayable 
surfaces, but Marek proved that wrong.

So what we should do is actually to add GTT as fallback to all BOs on 
APUs in Mesa and only make sure that the kernel is capable of handling 
GTT with optimal performance (e.g. have user huge pages etc..).

Christian.

Am 20.03.2018 um 21:04 schrieb Deucher, Alexander:
>
> I think we can also have the case of systems with similar amounts of 
> carve out and system ram.  E.g., on a system with 4 GB of system 
> memory with 1 GB carved out for vram.  It would be a big waste not to 
> use VRAM.  We'll probably need a heuristic at some point.
>
>
> Alex
>
> ------------------------------------------------------------------------
> *From:* Christian König <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> *Sent:* Tuesday, March 20, 2018 2:32:49 PM
> *To:* Marek Olšák; Koenig, Christian
> *Cc:* Alex Deucher; Deucher, Alexander; Michel Dänzer; Li, Samuel; 
> amd-gfx list
> *Subject:* Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display 
> support
> I don't think that is a good idea.
>
> Ideally GTT should now have the same performance as VRAM on APUs and 
> we should use VRAM only for things where we absolutely have to and to 
> actually use up the otherwise unused VRAM.
>
> Can you run some tests with all BOs forced to GTT and see if there is 
> any performance regression?
>
> Christian.
>
> Am 20.03.2018 um 15:51 schrieb Marek Olšák:
>> On Tue, Mar 20, 2018 at 9:55 AM, Christian König 
>> <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org 
>> <mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
>>
>>     Yes, exactly. And if I remember correctly Mesa used to always set
>>     GTT as fallback on APUs, correct?
>>
>>
>> "used to" is the key part. Mesa doesn't force GTT on APUs anymore. It 
>> expects that the combination of BO priorities and BO move throttling 
>> will result in optimal BO placements over time.
>>
>> Marek
>>
>>
>>     The problem seems to be that this fallback isn't set for
>>     displayable BOs.
>>
>>     So what needs to be done is to just enable this fallback for
>>     displayable BOs as well if the kernel can handle it.
>>
>>     Christian.
>>
>>
>>     Am 20.03.2018 um 00:01 schrieb Marek Olšák:
>>>     In theory, Mesa doesn't have to do anything. It can continue
>>>     setting VRAM and if the kernel has to put a display buffer into
>>>     GTT, it doesn't matter (for Mesa). Whether the VRAM placement is
>>>     really used is largely determined by BO priorities.
>>>
>>>     The way I understand scather/gather is that it only allows the
>>>     GTT placement. It doesn't force the GTT placement. Mesa also
>>>     doesn't force the GTT placement.
>>>
>>>     Marek
>>>
>>>     On Mon, Mar 19, 2018 at 5:12 PM, Alex Deucher
>>>     <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org <mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
>>>
>>>         On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel
>>>         <Samuel.Li-5C7GfCeVMHo@public.gmane.org <mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>> wrote:
>>>         >>to my earlier point, there may be cases where it is
>>>         advantageous to put
>>>         >> display buffers in vram even if s/g display is supported
>>>         >
>>>         > Agreed. That is also why the patch has the options to let
>>>         user select where
>>>         > to put display buffers.
>>>         >
>>>         > As whether to put the option in Mesa or kernel, it seems
>>>         the difference is
>>>         > not much. Also, since amdgpufb can request even without
>>>         mesa, kernel might
>>>         > be a better choice. In addition, putting in the kernel can
>>>         save client’s
>>>         > duplicate work(mesa, ogl, vulkan, 2d, kernel…)
>>>
>>>         Why do we even expose different memory pools to the UMDs in
>>>         the first
>>>         place ;)  Each pool has performance characteristics that may be
>>>         relevant for a particular work load. Only the UMDs really
>>>         know the
>>>         finer points of those workloads. In general, you don't want
>>>         the kernel
>>>         dictating policy if you can avoid it. The kernel exposes
>>>         functionality and userspace sets the policy.  With the
>>>         location set in
>>>         userspace, each app/user can have whatever policy makes
>>>         sense for
>>>         their use case all at the same time without needing to tweak
>>>         their
>>>         kernel for every use case.
>>>
>>>         Alex
>>>
>>>
>>>
>>>     _______________________________________________
>>>     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>
>>
>>
>>
>>
>> _______________________________________________
>> 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
>


[-- Attachment #1.2: Type: text/html, Size: 13305 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

RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Li, Samuel]

[-- Attachment #1.1: Type: text/plain, Size: 5664 bytes --]

O  I think we can also have the case of systems with similar amounts of carve out and system ram.  E.g., on a system with 4 GB of system memory with 1 GB carved out for vram.  It would be a big waste not to use VRAM.  We'll probably need a heuristic at some point.
Agreed. But for CZ/ST, due to hardware limitation as discussed before, we either use VRAM or GTT, not both. That might be changed for later ASICs, but it is beyond the scope of this patch.


Regards,
Samuel Li

From: Koenig, Christian
Sent: Tuesday, March 20, 2018 4:17 PM
To: Deucher, Alexander <Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>; Marek Olšák <maraeo@gmail.com>
Cc: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Michel Dänzer <michel@daenzer.net>; Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org>; amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9w@public.gmane.orgp.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

That's what I meant with use up the otherwise unused VRAM. I don't see any disadvantage of always setting GTT as second domain on APUs.

My assumption was that we dropped this in userspace for displayable surfaces, but Marek proved that wrong.

So what we should do is actually to add GTT as fallback to all BOs on APUs in Mesa and only make sure that the kernel is capable of handling GTT with optimal performance (e.g. have user huge pages etc..).

Christian.

Am 20.03.2018 um 21:04 schrieb Deucher, Alexander:

I think we can also have the case of systems with similar amounts of carve out and system ram.  E.g., on a system with 4 GB of system memory with 1 GB carved out for vram.  It would be a big waste not to use VRAM.  We'll probably need a heuristic at some point.



Alex

________________________________
From: Christian König <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org><mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Sent: Tuesday, March 20, 2018 2:32:49 PM
To: Marek Olšák; Koenig, Christian
Cc: Alex Deucher; Deucher, Alexander; Michel Dänzer; Li, Samuel; amd-gfx list
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

I don't think that is a good idea.

Ideally GTT should now have the same performance as VRAM on APUs and we should use VRAM only for things where we absolutely have to and to actually use up the otherwise unused VRAM.

Can you run some tests with all BOs forced to GTT and see if there is any performance regression?

Christian.

Am 20.03.2018 um 15:51 schrieb Marek Olšák:
On Tue, Mar 20, 2018 at 9:55 AM, Christian König <ckoenig.leichtzumerken@gmail.com<mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
Yes, exactly. And if I remember correctly Mesa used to always set GTT as fallback on APUs, correct?

"used to" is the key part. Mesa doesn't force GTT on APUs anymore. It expects that the combination of BO priorities and BO move throttling will result in optimal BO placements over time.
Marek


The problem seems to be that this fallback isn't set for displayable BOs.

So what needs to be done is to just enable this fallback for displayable BOs as well if the kernel can handle it.

Christian.


Am 20.03.2018 um 00:01 schrieb Marek Olšák:
In theory, Mesa doesn't have to do anything. It can continue setting VRAM and if the kernel has to put a display buffer into GTT, it doesn't matter (for Mesa). Whether the VRAM placement is really used is largely determined by BO priorities.

The way I understand scather/gather is that it only allows the GTT placement. It doesn't force the GTT placement. Mesa also doesn't force the GTT placement.

Marek

On Mon, Mar 19, 2018 at 5:12 PM, Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org<mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>> wrote:
>>to my earlier point, there may be cases where it is advantageous to put
>> display buffers in vram even if s/g display is supported
>
> Agreed. That is also why the patch has the options to let user select where
> to put display buffers.
>
> As whether to put the option in Mesa or kernel, it seems the difference is
> not much. Also, since amdgpufb can request even without mesa, kernel might
> be a better choice. In addition, putting in the kernel can save client's
> duplicate work(mesa, ogl, vulkan, 2d, kernel...)

Why do we even expose different memory pools to the UMDs in the first
place ;)  Each pool has performance characteristics that may be
relevant for a particular work load.  Only the UMDs really know the
finer points of those workloads. In general, you don't want the kernel
dictating policy if you can avoid it.  The kernel exposes
functionality and userspace sets the policy.  With the location set in
userspace, each app/user can have whatever policy makes sense for
their use case all at the same time without needing to tweak their
kernel for every use case.

Alex



_______________________________________________

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





_______________________________________________

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



[-- Attachment #1.2: Type: text/html, Size: 16518 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

RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Li, Samuel]

[-- Attachment #1.1: Type: text/plain, Size: 6634 bytes --]

Also, the change here is only for display buffer, so it is not really a "big" waste.

Regards,
Samuel Li

From: Li, Samuel
Sent: Tuesday, March 20, 2018 4:38 PM
To: Deucher, Alexander <Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>
Cc: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Michel Dänzer <michel@daenzer.net>; amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>; Marek Olšák <maraeo@gmail.com>; Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>
Subject: RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support


O  I think we can also have the case of systems with similar amounts of carve out and system ram.  E.g., on a system with 4 GB of system memory with 1 GB carved out for vram.  It would be a big waste not to use VRAM.  We'll probably need a heuristic at some point.
Agreed. But for CZ/ST, due to hardware limitation as discussed before, we either use VRAM or GTT, not both. That might be changed for later ASICs, but it is beyond the scope of this patch.


Regards,
Samuel Li

From: Koenig, Christian
Sent: Tuesday, March 20, 2018 4:17 PM
To: Deucher, Alexander <Alexander.Deucher-5C7GfCeVMHo@public.gmane.org<mailto:Alexander.Deucher@amd.com>>; Marek Olšák <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>
Cc: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>; Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org<mailto:michel-otUistvHUpPR7s880joybQ@public.gmane.org>>; Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org<mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>>; amd-gfx list <amd-gfx-PD4FTy7X32k@public.gmane.orgeedesktop.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

That's what I meant with use up the otherwise unused VRAM. I don't see any disadvantage of always setting GTT as second domain on APUs.

My assumption was that we dropped this in userspace for displayable surfaces, but Marek proved that wrong.

So what we should do is actually to add GTT as fallback to all BOs on APUs in Mesa and only make sure that the kernel is capable of handling GTT with optimal performance (e.g. have user huge pages etc..).

Christian.

Am 20.03.2018 um 21:04 schrieb Deucher, Alexander:

I think we can also have the case of systems with similar amounts of carve out and system ram.  E.g., on a system with 4 GB of system memory with 1 GB carved out for vram.  It would be a big waste not to use VRAM.  We'll probably need a heuristic at some point.



Alex

________________________________
From: Christian König <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org><mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Sent: Tuesday, March 20, 2018 2:32:49 PM
To: Marek Olšák; Koenig, Christian
Cc: Alex Deucher; Deucher, Alexander; Michel Dänzer; Li, Samuel; amd-gfx list
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

I don't think that is a good idea.

Ideally GTT should now have the same performance as VRAM on APUs and we should use VRAM only for things where we absolutely have to and to actually use up the otherwise unused VRAM.

Can you run some tests with all BOs forced to GTT and see if there is any performance regression?

Christian.

Am 20.03.2018 um 15:51 schrieb Marek Olšák:
On Tue, Mar 20, 2018 at 9:55 AM, Christian König <ckoenig.leichtzumerken@gmail.com<mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
Yes, exactly. And if I remember correctly Mesa used to always set GTT as fallback on APUs, correct?

"used to" is the key part. Mesa doesn't force GTT on APUs anymore. It expects that the combination of BO priorities and BO move throttling will result in optimal BO placements over time.
Marek


The problem seems to be that this fallback isn't set for displayable BOs.

So what needs to be done is to just enable this fallback for displayable BOs as well if the kernel can handle it.

Christian.


Am 20.03.2018 um 00:01 schrieb Marek Olšák:
In theory, Mesa doesn't have to do anything. It can continue setting VRAM and if the kernel has to put a display buffer into GTT, it doesn't matter (for Mesa). Whether the VRAM placement is really used is largely determined by BO priorities.

The way I understand scather/gather is that it only allows the GTT placement. It doesn't force the GTT placement. Mesa also doesn't force the GTT placement.

Marek

On Mon, Mar 19, 2018 at 5:12 PM, Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org<mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>> wrote:
>>to my earlier point, there may be cases where it is advantageous to put
>> display buffers in vram even if s/g display is supported
>
> Agreed. That is also why the patch has the options to let user select where
> to put display buffers.
>
> As whether to put the option in Mesa or kernel, it seems the difference is
> not much. Also, since amdgpufb can request even without mesa, kernel might
> be a better choice. In addition, putting in the kernel can save client's
> duplicate work(mesa, ogl, vulkan, 2d, kernel...)

Why do we even expose different memory pools to the UMDs in the first
place ;)  Each pool has performance characteristics that may be
relevant for a particular work load.  Only the UMDs really know the
finer points of those workloads. In general, you don't want the kernel
dictating policy if you can avoid it.  The kernel exposes
functionality and userspace sets the policy.  With the location set in
userspace, each app/user can have whatever policy makes sense for
their use case all at the same time without needing to tweak their
kernel for every use case.

Alex


_______________________________________________

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




_______________________________________________

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



[-- Attachment #1.2: Type: text/html, Size: 19276 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Marek Olšák]

[-- Attachment #1.1: Type: text/plain, Size: 751 bytes --]

On Tue, Mar 20, 2018 at 4:16 PM, Christian König <christian.koenig@amd.com>
wrote:

> That's what I meant with use up the otherwise unused VRAM. I don't see any
> disadvantage of always setting GTT as second domain on APUs.
>
> My assumption was that we dropped this in userspace for displayable
> surfaces, but Marek proved that wrong.
>
> So what we should do is actually to add GTT as fallback to all BOs on APUs
> in Mesa and only make sure that the kernel is capable of handling GTT with
> optimal performance (e.g. have user huge pages etc..).
>

VRAM|GTT is practically as good as GTT. VRAM with BO priorities and
eviction throttling is the true "VRAM|GTT".

I don't know how else to make use of VRAM intelligently.

Marek

[-- Attachment #1.2: Type: text/html, Size: 1388 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 7950 bytes --]

> But for CZ/ST, due to hardware limitation as discussed before, we 
> either use VRAM or GTT, not both.
That is actually not correct, as far as I read up on that the issue for 
switching between VRAM and GTT placement is minimal.

We should just make sure that we don't do this on every page flip, e.g. 
have double or triple buffering where one BO is in GTT and one in VRAM.

Regards,
Christian.

Am 20.03.2018 um 21:38 schrieb Li, Samuel:
>
> ØI think we can also have the case of systems with similar amounts of 
> carve out and system ram.  E.g., on a system with 4 GB of system 
> memory with 1 GB carved out for vram.  It would be a big waste not to 
> use VRAM.  We'll probably need a heuristic at some point.
>
> Agreed. But for CZ/ST, due to hardware limitation as discussed before, 
> we either use VRAM or GTT, not both. That might be changed for later 
> ASICs, but it is beyond the scope of this patch.
>
> Regards,
>
> Samuel Li
>
> *From:*Koenig, Christian
> *Sent:* Tuesday, March 20, 2018 4:17 PM
> *To:* Deucher, Alexander <Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>; Marek Olšák 
> <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> *Cc:* Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Michel Dänzer 
> <michel-otUistvHUpPR7s880joybQ@public.gmane.org>; Li, Samuel <Samuel.Li-5C7GfCeVMHo@public.gmane.org>; amd-gfx list 
> <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> *Subject:* Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display 
> support
>
> That's what I meant with use up the otherwise unused VRAM. I don't see 
> any disadvantage of always setting GTT as second domain on APUs.
>
> My assumption was that we dropped this in userspace for displayable 
> surfaces, but Marek proved that wrong.
>
> So what we should do is actually to add GTT as fallback to all BOs on 
> APUs in Mesa and only make sure that the kernel is capable of handling 
> GTT with optimal performance (e.g. have user huge pages etc..).
>
> Christian.
>
> Am 20.03.2018 um 21:04 schrieb Deucher, Alexander:
>
>     I think we can also have the case of systems with similar amounts
>     of carve out and system ram.  E.g., on a system with 4 GB of
>     system memory with 1 GB carved out for vram.  It would be a big
>     waste not to use VRAM.  We'll probably need a heuristic at some point.
>
>     Alex
>
>     ------------------------------------------------------------------------
>
>     *From:*Christian König <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>     <mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>     *Sent:* Tuesday, March 20, 2018 2:32:49 PM
>     *To:* Marek Olšák; Koenig, Christian
>     *Cc:* Alex Deucher; Deucher, Alexander; Michel Dänzer; Li, Samuel;
>     amd-gfx list
>     *Subject:* Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather
>     display support
>
>     I don't think that is a good idea.
>
>     Ideally GTT should now have the same performance as VRAM on APUs
>     and we should use VRAM only for things where we absolutely have to
>     and to actually use up the otherwise unused VRAM.
>
>     Can you run some tests with all BOs forced to GTT and see if there
>     is any performance regression?
>
>     Christian.
>
>     Am 20.03.2018 um 15:51 schrieb Marek Olšák:
>
>         On Tue, Mar 20, 2018 at 9:55 AM, Christian König
>         <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
>         <mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
>
>             Yes, exactly. And if I remember correctly Mesa used to
>             always set GTT as fallback on APUs, correct?
>
>         "used to" is the key part. Mesa doesn't force GTT on APUs
>         anymore. It expects that the combination of BO priorities and
>         BO move throttling will result in optimal BO placements over time.
>
>         Marek
>
>
>             The problem seems to be that this fallback isn't set for
>             displayable BOs.
>
>             So what needs to be done is to just enable this fallback
>             for displayable BOs as well if the kernel can handle it.
>
>             Christian.
>
>
>
>             Am 20.03.2018 um 00:01 schrieb Marek Olšák:
>
>                 In theory, Mesa doesn't have to do anything. It can
>                 continue setting VRAM and if the kernel has to put a
>                 display buffer into GTT, it doesn't matter (for Mesa).
>                 Whether the VRAM placement is really used is largely
>                 determined by BO priorities.
>
>                 The way I understand scather/gather is that it only
>                 allows the GTT placement. It doesn't force the GTT
>                 placement. Mesa also doesn't force the GTT placement.
>
>                 Marek
>
>                 On Mon, Mar 19, 2018 at 5:12 PM, Alex Deucher
>                 <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org <mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>
>                 wrote:
>
>                     On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel
>                     <Samuel.Li-5C7GfCeVMHo@public.gmane.org <mailto:Samuel.Li-5C7GfCeVMHo@public.gmane.org>> wrote:
>                     >>to my earlier point, there may be cases where it
>                     is advantageous to put
>                     >> display buffers in vram even if s/g display is
>                     supported
>                     >
>                     > Agreed. That is also why the patch has the
>                     options to let user select where
>                     > to put display buffers.
>                     >
>                     > As whether to put the option in Mesa or kernel,
>                     it seems the difference is
>                     > not much. Also, since amdgpufb can request even
>                     without mesa, kernel might
>                     > be a better choice. In addition, putting in the
>                     kernel can save client’s
>                     > duplicate work(mesa, ogl, vulkan, 2d, kernel…)
>
>                     Why do we even expose different memory pools to
>                     the UMDs in the first
>                     place ;)  Each pool has performance
>                     characteristics that may be
>                     relevant for a particular work load.  Only the
>                     UMDs really know the
>                     finer points of those workloads. In general, you
>                     don't want the kernel
>                     dictating policy if you can avoid it.  The kernel
>                     exposes
>                     functionality and userspace sets the policy.  With
>                     the location set in
>                     userspace, each app/user can have whatever policy
>                     makes sense for
>                     their use case all at the same time without
>                     needing to tweak their
>                     kernel for every use case.
>
>                     Alex
>
>
>
>                 _______________________________________________
>
>                 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
>
>
>
>         _______________________________________________
>
>         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
>
>
>
> _______________________________________________
> 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: 27230 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 1360 bytes --]

Am 21.03.2018 um 06:08 schrieb Marek Olšák:
> On Tue, Mar 20, 2018 at 4:16 PM, Christian König 
> <christian.koenig-5C7GfCeVMHo@public.gmane.org <mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>> wrote:
>
>     That's what I meant with use up the otherwise unused VRAM. I don't
>     see any disadvantage of always setting GTT as second domain on APUs.
>
>     My assumption was that we dropped this in userspace for
>     displayable surfaces, but Marek proved that wrong.
>
>     So what we should do is actually to add GTT as fallback to all BOs
>     on APUs in Mesa and only make sure that the kernel is capable of
>     handling GTT with optimal performance (e.g. have user huge pages
>     etc..).
>
>
> VRAM|GTT is practically as good as GTT. VRAM with BO priorities and 
> eviction throttling is the true "VRAM|GTT".
>
> I don't know how else to make use of VRAM intelligently.

Well why not set VRAM|GTT as default on APUs? That should still save 
quite a bunch of moves even with throttling.

I mean there really shouldn't be any advantage to use VRAM any more 
except that we want to use it up as long as it is available.

Christian.

>
> Marek
>
>
> _______________________________________________
> 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: 3434 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Marek Olšák]

[-- Attachment #1.1: Type: text/plain, Size: 1374 bytes --]

On Wed, Mar 21, 2018 at 4:13 AM, Christian König <
ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Am 21.03.2018 um 06:08 schrieb Marek Olšák:
>
> On Tue, Mar 20, 2018 at 4:16 PM, Christian König <christian.koenig@amd.com
> > wrote:
>
>> That's what I meant with use up the otherwise unused VRAM. I don't see
>> any disadvantage of always setting GTT as second domain on APUs.
>>
>> My assumption was that we dropped this in userspace for displayable
>> surfaces, but Marek proved that wrong.
>>
>> So what we should do is actually to add GTT as fallback to all BOs on
>> APUs in Mesa and only make sure that the kernel is capable of handling GTT
>> with optimal performance (e.g. have user huge pages etc..).
>>
>
> VRAM|GTT is practically as good as GTT. VRAM with BO priorities and
> eviction throttling is the true "VRAM|GTT".
>
> I don't know how else to make use of VRAM intelligently.
>
>
> Well why not set VRAM|GTT as default on APUs? That should still save quite
> a bunch of moves even with throttling.
>

I explained why: VRAM|GTT is practically as good as GTT.


>
> I mean there really shouldn't be any advantage to use VRAM any more except
> that we want to use it up as long as it is available.
>

Why are you suggesting to use VRAM|GTT then? Let's just only use GTT on all
APUs.

Marek

[-- Attachment #1.2: Type: text/html, Size: 3294 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 1944 bytes --]

Am 21.03.2018 um 14:57 schrieb Marek Olšák:
> On Wed, Mar 21, 2018 at 4:13 AM, Christian König 
> <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org 
> <mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
>
>     Am 21.03.2018 um 06:08 schrieb Marek Olšák:
>>     On Tue, Mar 20, 2018 at 4:16 PM, Christian König
>>     <christian.koenig-5C7GfCeVMHo@public.gmane.org <mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>> wrote:
>>
>>         That's what I meant with use up the otherwise unused VRAM. I
>>         don't see any disadvantage of always setting GTT as second
>>         domain on APUs.
>>
>>         My assumption was that we dropped this in userspace for
>>         displayable surfaces, but Marek proved that wrong.
>>
>>         So what we should do is actually to add GTT as fallback to
>>         all BOs on APUs in Mesa and only make sure that the kernel is
>>         capable of handling GTT with optimal performance (e.g. have
>>         user huge pages etc..).
>>
>>
>>     VRAM|GTT is practically as good as GTT. VRAM with BO priorities
>>     and eviction throttling is the true "VRAM|GTT".
>>
>>     I don't know how else to make use of VRAM intelligently.
>
>     Well why not set VRAM|GTT as default on APUs? That should still
>     save quite a bunch of moves even with throttling.
>
>
> I explained why: VRAM|GTT is practically as good as GTT.
>
>
>     I mean there really shouldn't be any advantage to use VRAM any
>     more except that we want to use it up as long as it is available.
>
>
> Why are you suggesting to use VRAM|GTT then? Let's just only use GTT 
> on all APUs.

Then we don't use the memory stolen for VRAM.

See we want to get to a point where we have any ~16MB of stolen VRAM on 
APUs and everything else in GTT.

But we still have to support cases where we have 1GB stolen VRAM, and 
wasting those 1GB would suck a bit.

Christian.

>
> Marek


[-- Attachment #1.2: Type: text/html, Size: 5788 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

RE: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Li, Samuel]

[-- Attachment #1.1: Type: text/plain, Size: 2242 bytes --]

Ø  But we still have to support cases where we have 1GB stolen VRAM, and wasting those 1GB would suck a bit.
Not really, since we only move display buffer here.

Regards,
Samuel Li

From: Koenig, Christian
Sent: Wednesday, March 21, 2018 10:07 AM
To: Marek Olšák <maraeo@gmail.com>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Alex Deucher <alexdeucher@gmail.com>; Michel Dänzer <michel@daenzer.net>; amd-gfx list <amd-gfx@lists.freedesktop.org>; Li, Samuel <Samuel.Li@amd.com>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

Am 21.03.2018 um 14:57 schrieb Marek Olšák:
On Wed, Mar 21, 2018 at 4:13 AM, Christian König <ckoenig.leichtzumerken@gmail.com<mailto:ckoenig.leichtzumerken@gmail.com>> wrote:
Am 21.03.2018 um 06:08 schrieb Marek Olšák:
On Tue, Mar 20, 2018 at 4:16 PM, Christian König <christian.koenig@amd.com<mailto:christian.koenig@amd.com>> wrote:
That's what I meant with use up the otherwise unused VRAM. I don't see any disadvantage of always setting GTT as second domain on APUs.

My assumption was that we dropped this in userspace for displayable surfaces, but Marek proved that wrong.

So what we should do is actually to add GTT as fallback to all BOs on APUs in Mesa and only make sure that the kernel is capable of handling GTT with optimal performance (e.g. have user huge pages etc..).

VRAM|GTT is practically as good as GTT. VRAM with BO priorities and eviction throttling is the true "VRAM|GTT".
I don't know how else to make use of VRAM intelligently.

Well why not set VRAM|GTT as default on APUs? That should still save quite a bunch of moves even with throttling.

I explained why: VRAM|GTT is practically as good as GTT.


I mean there really shouldn't be any advantage to use VRAM any more except that we want to use it up as long as it is available.

Why are you suggesting to use VRAM|GTT then? Let's just only use GTT on all APUs.

Then we don't use the memory stolen for VRAM.

See we want to get to a point where we have any ~16MB of stolen VRAM on APUs and everything else in GTT.

But we still have to support cases where we have 1GB stolen VRAM, and wasting those 1GB would suck a bit.

Christian.



Marek


[-- Attachment #1.2: Type: text/html, Size: 9949 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Marek Olšák]

[-- Attachment #1.1: Type: text/plain, Size: 2169 bytes --]

On Wed, Mar 21, 2018 at 10:07 AM, Christian König <christian.koenig@amd.com>
wrote:

> Am 21.03.2018 um 14:57 schrieb Marek Olšák:
>
> On Wed, Mar 21, 2018 at 4:13 AM, Christian König <
> ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> Am 21.03.2018 um 06:08 schrieb Marek Olšák:
>>
>> On Tue, Mar 20, 2018 at 4:16 PM, Christian König <
>> christian.koenig-5C7GfCeVMHo@public.gmane.org> wrote:
>>
>>> That's what I meant with use up the otherwise unused VRAM. I don't see
>>> any disadvantage of always setting GTT as second domain on APUs.
>>>
>>> My assumption was that we dropped this in userspace for displayable
>>> surfaces, but Marek proved that wrong.
>>>
>>> So what we should do is actually to add GTT as fallback to all BOs on
>>> APUs in Mesa and only make sure that the kernel is capable of handling GTT
>>> with optimal performance (e.g. have user huge pages etc..).
>>>
>>
>> VRAM|GTT is practically as good as GTT. VRAM with BO priorities and
>> eviction throttling is the true "VRAM|GTT".
>>
>> I don't know how else to make use of VRAM intelligently.
>>
>>
>> Well why not set VRAM|GTT as default on APUs? That should still save
>> quite a bunch of moves even with throttling.
>>
>
> I explained why: VRAM|GTT is practically as good as GTT.
>
>
>>
>> I mean there really shouldn't be any advantage to use VRAM any more
>> except that we want to use it up as long as it is available.
>>
>
> Why are you suggesting to use VRAM|GTT then? Let's just only use GTT on
> all APUs.
>
>
> Then we don't use the memory stolen for VRAM.
>
> See we want to get to a point where we have any ~16MB of stolen VRAM on
> APUs and everything else in GTT.
>
> But we still have to support cases where we have 1GB stolen VRAM, and
> wasting those 1GB would suck a bit.
>

BO priorities and BO move throttling should take care of optimal VRAM usage
regardless of the VRAM size. We can adjust the throttling for small VRAM,
but that's about all we can do.

VRAM|GTT doesn't guarantee that VRAM will be used usefully. In fact, it
doesn't guarantee anything about VRAM.

Marek

[-- Attachment #1.2: Type: text/html, Size: 5858 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 3139 bytes --]

Am 21.03.2018 um 19:04 schrieb Marek Olšák:
> On Wed, Mar 21, 2018 at 10:07 AM, Christian König 
> <christian.koenig-5C7GfCeVMHo@public.gmane.org <mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>> wrote:
>
>     Am 21.03.2018 um 14:57 schrieb Marek Olšák:
>>     On Wed, Mar 21, 2018 at 4:13 AM, Christian König
>>     <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
>>     <mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
>>
>>         Am 21.03.2018 um 06:08 schrieb Marek Olšák:
>>>         On Tue, Mar 20, 2018 at 4:16 PM, Christian König
>>>         <christian.koenig-5C7GfCeVMHo@public.gmane.org <mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>>
>>>         wrote:
>>>
>>>             That's what I meant with use up the otherwise unused
>>>             VRAM. I don't see any disadvantage of always setting GTT
>>>             as second domain on APUs.
>>>
>>>             My assumption was that we dropped this in userspace for
>>>             displayable surfaces, but Marek proved that wrong.
>>>
>>>             So what we should do is actually to add GTT as fallback
>>>             to all BOs on APUs in Mesa and only make sure that the
>>>             kernel is capable of handling GTT with optimal
>>>             performance (e.g. have user huge pages etc..).
>>>
>>>
>>>         VRAM|GTT is practically as good as GTT. VRAM with BO
>>>         priorities and eviction throttling is the true "VRAM|GTT".
>>>
>>>         I don't know how else to make use of VRAM intelligently.
>>
>>         Well why not set VRAM|GTT as default on APUs? That should
>>         still save quite a bunch of moves even with throttling.
>>
>>
>>     I explained why: VRAM|GTT is practically as good as GTT.
>>
>>
>>         I mean there really shouldn't be any advantage to use VRAM
>>         any more except that we want to use it up as long as it is
>>         available.
>>
>>
>>     Why are you suggesting to use VRAM|GTT then? Let's just only use
>>     GTT on all APUs.
>
>     Then we don't use the memory stolen for VRAM.
>
>     See we want to get to a point where we have any ~16MB of stolen
>     VRAM on APUs and everything else in GTT.
>
>     But we still have to support cases where we have 1GB stolen VRAM,
>     and wasting those 1GB would suck a bit.
>
>
> BO priorities and BO move throttling should take care of optimal VRAM 
> usage regardless of the VRAM size. We can adjust the throttling for 
> small VRAM, but that's about all we can do.

Well at least on APUs move throttling is complete nonsense. VRAM should 
expose the same performance as GTT.

So the only usage we have for VRAM is for special cases like page tables 
and to allow to actually use the stolen memory.

> VRAM|GTT doesn't guarantee that VRAM will be used usefully. In fact, 
> it doesn't guarantee anything about VRAM.

Why not? VRAM|GTT means that we should use VRAM as long as it is 
available and if it is used up fallback to GTT.

When BOs are evicted from VRAM they are never moved back into it. As far 
as I can see that is exactly what we need on APUs.

Christian.

>
> Marek


[-- Attachment #1.2: Type: text/html, Size: 9972 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Marek Olšák]

[-- Attachment #1.1: Type: text/plain, Size: 3037 bytes --]

On Wed, Mar 21, 2018 at 2:15 PM, Christian König <christian.koenig@amd.com>
wrote:

> Am 21.03.2018 um 19:04 schrieb Marek Olšák:
>
> On Wed, Mar 21, 2018 at 10:07 AM, Christian König <
> christian.koenig-5C7GfCeVMHo@public.gmane.org> wrote:
>
>> Am 21.03.2018 um 14:57 schrieb Marek Olšák:
>>
>> On Wed, Mar 21, 2018 at 4:13 AM, Christian König <
>> ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>>> Am 21.03.2018 um 06:08 schrieb Marek Olšák:
>>>
>>> On Tue, Mar 20, 2018 at 4:16 PM, Christian König <
>>> christian.koenig-5C7GfCeVMHo@public.gmane.org> wrote:
>>>
>>>> That's what I meant with use up the otherwise unused VRAM. I don't see
>>>> any disadvantage of always setting GTT as second domain on APUs.
>>>>
>>>> My assumption was that we dropped this in userspace for displayable
>>>> surfaces, but Marek proved that wrong.
>>>>
>>>> So what we should do is actually to add GTT as fallback to all BOs on
>>>> APUs in Mesa and only make sure that the kernel is capable of handling GTT
>>>> with optimal performance (e.g. have user huge pages etc..).
>>>>
>>>
>>> VRAM|GTT is practically as good as GTT. VRAM with BO priorities and
>>> eviction throttling is the true "VRAM|GTT".
>>>
>>> I don't know how else to make use of VRAM intelligently.
>>>
>>>
>>> Well why not set VRAM|GTT as default on APUs? That should still save
>>> quite a bunch of moves even with throttling.
>>>
>>
>> I explained why: VRAM|GTT is practically as good as GTT.
>>
>>
>>>
>>> I mean there really shouldn't be any advantage to use VRAM any more
>>> except that we want to use it up as long as it is available.
>>>
>>
>> Why are you suggesting to use VRAM|GTT then? Let's just only use GTT on
>> all APUs.
>>
>>
>> Then we don't use the memory stolen for VRAM.
>>
>> See we want to get to a point where we have any ~16MB of stolen VRAM on
>> APUs and everything else in GTT.
>>
>> But we still have to support cases where we have 1GB stolen VRAM, and
>> wasting those 1GB would suck a bit.
>>
>
> BO priorities and BO move throttling should take care of optimal VRAM
> usage regardless of the VRAM size. We can adjust the throttling for small
> VRAM, but that's about all we can do.
>
>
> Well at least on APUs move throttling is complete nonsense. VRAM should
> expose the same performance as GTT.
>
> So the only usage we have for VRAM is for special cases like page tables
> and to allow to actually use the stolen memory.
>
> VRAM|GTT doesn't guarantee that VRAM will be used usefully. In fact, it
> doesn't guarantee anything about VRAM.
>
>
> Why not? VRAM|GTT means that we should use VRAM as long as it is available
> and if it is used up fallback to GTT.
>
> When BOs are evicted from VRAM they are never moved back into it. As far
> as I can see that is exactly what we need on APUs.
>

I see. You don't want to use VRAM usefully. You just want to fill it up
with something (anything) so that it's not unused.

Marek

[-- Attachment #1.2: Type: text/html, Size: 9546 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

Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 4310 bytes --]

Am 21.03.2018 um 19:23 schrieb Marek Olšák:
> On Wed, Mar 21, 2018 at 2:15 PM, Christian König 
> <christian.koenig-5C7GfCeVMHo@public.gmane.org <mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>> wrote:
>
>     Am 21.03.2018 um 19:04 schrieb Marek Olšák:
>>     On Wed, Mar 21, 2018 at 10:07 AM, Christian König
>>     <christian.koenig-5C7GfCeVMHo@public.gmane.org <mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>> wrote:
>>
>>         Am 21.03.2018 um 14:57 schrieb Marek Olšák:
>>>         On Wed, Mar 21, 2018 at 4:13 AM, Christian König
>>>         <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
>>>         <mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
>>>
>>>             Am 21.03.2018 um 06:08 schrieb Marek Olšák:
>>>>             On Tue, Mar 20, 2018 at 4:16 PM, Christian König
>>>>             <christian.koenig-5C7GfCeVMHo@public.gmane.org
>>>>             <mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>> wrote:
>>>>
>>>>                 That's what I meant with use up the otherwise
>>>>                 unused VRAM. I don't see any disadvantage of always
>>>>                 setting GTT as second domain on APUs.
>>>>
>>>>                 My assumption was that we dropped this in userspace
>>>>                 for displayable surfaces, but Marek proved that wrong.
>>>>
>>>>                 So what we should do is actually to add GTT as
>>>>                 fallback to all BOs on APUs in Mesa and only make
>>>>                 sure that the kernel is capable of handling GTT
>>>>                 with optimal performance (e.g. have user huge pages
>>>>                 etc..).
>>>>
>>>>
>>>>             VRAM|GTT is practically as good as GTT. VRAM with BO
>>>>             priorities and eviction throttling is the true "VRAM|GTT".
>>>>
>>>>             I don't know how else to make use of VRAM intelligently.
>>>
>>>             Well why not set VRAM|GTT as default on APUs? That
>>>             should still save quite a bunch of moves even with
>>>             throttling.
>>>
>>>
>>>         I explained why: VRAM|GTT is practically as good as GTT.
>>>
>>>
>>>             I mean there really shouldn't be any advantage to use
>>>             VRAM any more except that we want to use it up as long
>>>             as it is available.
>>>
>>>
>>>         Why are you suggesting to use VRAM|GTT then? Let's just only
>>>         use GTT on all APUs.
>>
>>         Then we don't use the memory stolen for VRAM.
>>
>>         See we want to get to a point where we have any ~16MB of
>>         stolen VRAM on APUs and everything else in GTT.
>>
>>         But we still have to support cases where we have 1GB stolen
>>         VRAM, and wasting those 1GB would suck a bit.
>>
>>
>>     BO priorities and BO move throttling should take care of optimal
>>     VRAM usage regardless of the VRAM size. We can adjust the
>>     throttling for small VRAM, but that's about all we can do.
>
>     Well at least on APUs move throttling is complete nonsense. VRAM
>     should expose the same performance as GTT.
>
>     So the only usage we have for VRAM is for special cases like page
>     tables and to allow to actually use the stolen memory.
>
>>     VRAM|GTT doesn't guarantee that VRAM will be used usefully. In
>>     fact, it doesn't guarantee anything about VRAM.
>
>     Why not? VRAM|GTT means that we should use VRAM as long as it is
>     available and if it is used up fallback to GTT.
>
>     When BOs are evicted from VRAM they are never moved back into it.
>     As far as I can see that is exactly what we need on APUs.
>
>
> I see. You don't want to use VRAM usefully. You just want to fill it 
> up with something (anything) so that it's not unused.

Yes, exactly. The point is we really don't have any special use case for 
it on APUs any more on newer kernels/hardware.

We need a bit for firmware, but that is fixed and allocate at driver 
load time.

Page tables are still in VRAM, but at least for Raven that is just an 
issue that I didn't had free time to implement it otherwise.

If I could I would give the unused parts back to the OS for general 
purpose usage, but you need to reconfigure the northbridge to do that 
and well that is easier said than done.

Christian.

>
> Marek


[-- Attachment #1.2: Type: text/html, Size: 15978 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