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

[Christian König]

Am 21.03.2018 um 19:23 schrieb Marek Olšák:
On Wed, Mar 21, 2018 at 2:15 PM, Christian König 
> wrote:


Am 21.03.2018 um 19:04 schrieb Marek Olšák:

On Wed, Mar 21, 2018 at 10:07 AM, Christian König
> wrote:

Am 21.03.2018 um 14:57 schrieb Marek Olšák:

On Wed, Mar 21, 2018 at 4:13 AM, Christian König
> wrote:

Am 21.03.2018 um 06:08 schrieb Marek Olšák:

On Tue, Mar 20, 2018 at 4:16 PM, Christian König
> 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


___
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]

On Wed, Mar 21, 2018 at 2:15 PM, Christian König 
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.koe...@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.leichtzumer...@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.koe...@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.
>>
>
> 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
___
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 21.03.2018 um 19:04 schrieb Marek Olšák:
On Wed, Mar 21, 2018 at 10:07 AM, Christian König 
> wrote:


Am 21.03.2018 um 14:57 schrieb Marek Olšák:

On Wed, Mar 21, 2018 at 4:13 AM, Christian König
> wrote:

Am 21.03.2018 um 06:08 schrieb Marek Olšák:

On Tue, Mar 20, 2018 at 4:16 PM, Christian König
>
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


___
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]

On Wed, Mar 21, 2018 at 10:07 AM, Christian König 
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.leichtzumer...@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.koe...@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.
>

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
___
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]

Ø  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 <mar...@gmail.com>
Cc: Deucher, Alexander <alexander.deuc...@amd.com>; Alex Deucher 
<alexdeuc...@gmail.com>; Michel Dänzer <mic...@daenzer.net>; amd-gfx list 
<amd-gfx@lists.freedesktop.org>; Li, Samuel <samuel...@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.leichtzumer...@gmail.com<mailto:ckoenig.leichtzumer...@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.koe...@amd.com<mailto:christian.koe...@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

___
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 21.03.2018 um 14:57 schrieb Marek Olšák:
On Wed, Mar 21, 2018 at 4:13 AM, Christian König 
> wrote:


Am 21.03.2018 um 06:08 schrieb Marek Olšák:

On Tue, Mar 20, 2018 at 4:16 PM, Christian König
> 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


___
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]

On Wed, Mar 21, 2018 at 4:13 AM, Christian König <
ckoenig.leichtzumer...@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  > 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
___
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 21.03.2018 um 06:08 schrieb Marek Olšák:
On Tue, Mar 20, 2018 at 4:16 PM, Christian König 
> 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@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]

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.deuc...@amd.com>; Marek Olšák 
<mar...@gmail.com>
*Cc:* Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer 
<mic...@daenzer.net>; Li, Samuel <samuel...@amd.com>; amd-gfx list 
<amd-gfx@lists.freedesktop.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.leichtzumer...@gmail.com>
<mailto:ckoenig.leichtzumer...@gmail.com>
*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.leichtzumer...@gmail.com
<mailto:ckoenig.leichtzumer...@gmail.com>> 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
<alexdeuc...@gmail.com <mailto:alexdeuc...@gmail.com>>
wrote:

On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel
<samuel...@amd.com <mailto:samuel...@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
   

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

[Li, Samuel]

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.deuc...@amd.com>
Cc: Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer <mic...@daenzer.net>; 
amd-gfx list <amd-gfx@lists.freedesktop.org>; Marek Olšák <mar...@gmail.com>; 
Koenig, Christian <christian.koe...@amd.com>
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.deuc...@amd.com<mailto:alexander.deuc...@amd.com>>; Marek Olšák 
<mar...@gmail.com<mailto:mar...@gmail.com>>
Cc: Alex Deucher <alexdeuc...@gmail.com<mailto:alexdeuc...@gmail.com>>; Michel 
Dänzer <mic...@daenzer.net<mailto:mic...@daenzer.net>>; Li, Samuel 
<samuel...@amd.com<mailto:samuel...@amd.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

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.leichtzumer...@gmail.com><mailto:ckoenig.leichtzumer...@gmail.com>
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.leichtzumer...@gmail.com<mailto:ckoenig.leichtzumer...@gmail.com>> 
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 
<alexdeuc...@gmail.com<mailto:alexdeuc...@gmail.com>> wrote:
On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel 
<samuel...@amd.com<mailto:samuel...@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. I

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

[Li, Samuel]

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.deuc...@amd.com>; Marek Olšák 
<mar...@gmail.com>
Cc: Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer <mic...@daenzer.net>; 
Li, Samuel <samuel...@amd.com>; amd-gfx list <amd-gfx@lists.freedesktop.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.leichtzumer...@gmail.com><mailto:ckoenig.leichtzumer...@gmail.com>
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.leichtzumer...@gmail.com<mailto:ckoenig.leichtzumer...@gmail.com>> 
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 
<alexdeuc...@gmail.com<mailto:alexdeuc...@gmail.com>> wrote:
On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel 
<samuel...@amd.com<mailto:samuel...@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



___

amd-gfx mailing

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

[Christian König]

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.leichtzumer...@gmail.com>
*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.leichtzumer...@gmail.com 
<mailto:ckoenig.leichtzumer...@gmail.com>> 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
<alexdeuc...@gmail.com <mailto:alexdeuc...@gmail.com>> wrote:

On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel
<samuel...@amd.com <mailto:samuel...@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



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





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org <mailto: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

[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.leichtzumer...@gmail.com>
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.leichtzumer...@gmail.com<mailto:ckoenig.leichtzumer...@gmail.com>> 
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 
<alexdeuc...@gmail.com<mailto:alexdeuc...@gmail.com>> wrote:
On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel 
<samuel...@amd.com<mailto:samuel...@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



___
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


___
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 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 
> 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
> wrote:

On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel
> 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@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]

On Tue, Mar 20, 2018 at 9:55 AM, Christian König <
ckoenig.leichtzumer...@gmail.com> 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 
> wrote:
>
>> On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel  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@lists.freedesktop.orghttps://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]

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 > wrote:


On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel > 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@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]

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  wrote:

> On Mon, Mar 19, 2018 at 4:29 PM, Li, Samuel  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@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...@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:mar...@gmail.com]
> Sent: Monday, March 19, 2018 4:27 PM
> To: Deucher, Alexander <alexander.deuc...@amd.com>
> Cc: Li, Samuel <samuel...@amd.com>; Koenig, Christian
> <christian.koe...@amd.com>; Alex Deucher <alexdeuc...@gmail.com>; Michel
> Dänzer <mic...@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.deuc...@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-boun...@lists.freedesktop.org] On Behalf Of
> Deucher, Alexander
> Sent: Monday, March 19, 2018 4:13 PM
>
>
> To: Li, Samuel <samuel...@amd.com>; Koenig, Christian
> <christian.koe...@amd.com>; Marek Olšák <mar...@gmail.com>
> Cc: Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer
> <mic...@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...@amd.com>; Koenig, Christian
> <christian.koe...@amd.com>; Marek Olšák <mar...@gmail.com>
> Cc: Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer
> <mic...@daenzer.net>; amd-gfx list &l

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

[Marek Olšák]

On Mon, Mar 19, 2018 at 4:53 PM, Alex Deucher  wrote:

> On Mon, Mar 19, 2018 at 4:26 PM, Marek Olšák  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
___
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 <mar...@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.deuc...@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-boun...@lists.freedesktop.org] On Behalf Of
>> Deucher, Alexander
>> Sent: Monday, March 19, 2018 4:13 PM
>>
>>
>> To: Li, Samuel <samuel...@amd.com>; Koenig, Christian
>> <christian.koe...@amd.com>; Marek Olšák <mar...@gmail.com>
>> Cc: Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer
>> <mic...@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...@amd.com>; Koenig, Christian
>> <christian.koe...@amd.com>; Marek Olšák <mar...@gmail.com>
>> Cc: Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer
>> <mic...@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-boun...@lists.freedesktop.org> on behalf of Li,
>> Samuel <samuel...@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

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

[Li, Samuel]

>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:mar...@gmail.com]
Sent: Monday, March 19, 2018 4:27 PM
To: Deucher, Alexander <alexander.deuc...@amd.com>
Cc: Li, Samuel <samuel...@amd.com>; Koenig, Christian 
<christian.koe...@amd.com>; Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer 
<mic...@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.deuc...@amd.com<mailto:alexander.deuc...@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-boun...@lists.freedesktop.org<mailto:amd-gfx-boun...@lists.freedesktop.org>]
 On Behalf Of Deucher, Alexander
Sent: Monday, March 19, 2018 4:13 PM

To: Li, Samuel <samuel...@amd.com<mailto:samuel...@amd.com>>; Koenig, Christian 
<christian.koe...@amd.com<mailto:christian.koe...@amd.com>>; Marek Olšák 
<mar...@gmail.com<mailto:mar...@gmail.com>>
Cc: Alex Deucher <alexdeuc...@gmail.com<mailto:alexdeuc...@gmail.com>>; Michel 
Dänzer <mic...@daenzer.net<mailto:mic...@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...@amd.com<mailto:samuel...@amd.com>>; Koenig, Christian 
<christian.koe...@amd.com<mailto:christian.koe...@amd.com>>; Marek Olšák 
<mar...@gmail.com<mailto:mar...@gmail.com>>
Cc: Alex Deucher <alexdeuc...@gmail.com<mailto:alexdeuc...@gmail.com>>; Michel 
Dänzer <mic...@daenzer.net<mailto:mic...@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-boun...@lists.freedesktop.org<mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Li, Samuel <samuel...@amd.com<mailto:samuel...@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 comm

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

[Marek Olšák]

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.deuc...@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-boun...@lists.freedesktop.org] *On Behalf
> Of *Deucher, Alexander
> *Sent:* Monday, March 19, 2018 4:13 PM
>
> *To:* Li, Samuel <samuel...@amd.com>; Koenig, Christian <
> christian.koe...@amd.com>; Marek Olšák <mar...@gmail.com>
> *Cc:* Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer <
> mic...@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...@amd.com>; Koenig, Christian <
> christian.koe...@amd.com>; Marek Olšák <mar...@gmail.com>
> *Cc:* Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer <
> mic...@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-boun...@lists.freedesktop.org> on behalf of Li,
> Samuel <samuel...@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.leichtzumer...@gmail.com
> <ckoenig.leichtzumer...@gmail.com>]
> *Sent:* Monday, March 19, 2018 3:45 PM
> *To:* Li, Samuel <samuel...@amd.com>; Marek Olšák <mar...@gmail.com>;
> Koenig, Christian <christian.koe...@amd.com>
> *Cc:* Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer <
> mic...@daenzer.net>; amd-gfx lis

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

[Deucher, Alexander]

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-boun...@lists.freedesktop.org] On Behalf Of 
Deucher, Alexander
Sent: Monday, March 19, 2018 4:13 PM
To: Li, Samuel <samuel...@amd.com>; Koenig, Christian 
<christian.koe...@amd.com>; Marek Olšák <mar...@gmail.com>
Cc: Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer <mic...@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...@amd.com>; Koenig, Christian 
<christian.koe...@amd.com>; Marek Olšák <mar...@gmail.com>
Cc: Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer <mic...@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-boun...@lists.freedesktop.org<mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Li, Samuel <samuel...@amd.com<mailto:samuel...@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.leichtzumer...@gmail.com]
Sent: Monday, March 19, 2018 3:45 PM
To: Li, Samuel <samuel...@amd.com<mailto:samuel...@amd.com>>; Marek Olšák 
<mar...@gmail.com<mailto:mar...@gmail.com>>; Koenig, Christian 
<christian.koe...@amd.com<mailto:christian.koe...@amd.com>>
Cc: Alex Deucher <alexdeuc...@gmail.com<mailto:alexdeuc...@gmail.com>>; Michel 
Dänzer <mic...@daenzer.net<mailto:mic...@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.

Regar

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

[Deucher, Alexander]

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...@amd.com>; Koenig, Christian 
<christian.koe...@amd.com>; Marek Olšák <mar...@gmail.com>
Cc: Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer <mic...@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-boun...@lists.freedesktop.org<mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Li, Samuel <samuel...@amd.com<mailto:samuel...@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.leichtzumer...@gmail.com]
Sent: Monday, March 19, 2018 3:45 PM
To: Li, Samuel <samuel...@amd.com<mailto:samuel...@amd.com>>; Marek Olšák 
<mar...@gmail.com<mailto:mar...@gmail.com>>; Koenig, Christian 
<christian.koe...@amd.com<mailto:christian.koe...@amd.com>>
Cc: Alex Deucher <alexdeuc...@gmail.com<mailto:alexdeuc...@gmail.com>>; Michel 
Dänzer <mic...@daenzer.net<mailto:mic...@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:mar...@gmail.com]
Sent: Monday, March 19, 2018 3:39 PM
To: Koenig, Christian 
<christian.koe...@amd.com><mailto:christian.koe...@amd.com>
Cc: Li, Samuel <samuel...@amd.com><mailto:samuel...@amd.com>; Michel Dänzer 
<mic...@daenzer.net><mailto:mic...@daenzer.net>; Alex Deucher 
<alexdeuc...@gmail.com><ma

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

[Christian König]

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...@amd.com>; Koenig, Christian 
<christian.koe...@amd.com>; Marek Olšák <mar...@gmail.com>
*Cc:* Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer 
<mic...@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-boun...@lists.freedesktop.org 
<mailto:amd-gfx-boun...@lists.freedesktop.org>> on behalf of Li, 
Samuel <samuel...@amd.com <mailto:samuel...@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.leichtzumer...@gmail.com]
*Sent:* Monday, March 19, 2018 3:45 PM
*To:* Li, Samuel <samuel...@amd.com <mailto:samuel...@amd.com>>; Marek 
Olšák <mar...@gmail.com <mailto:mar...@gmail.com>>; Koenig, Christian 
<christian.koe...@amd.com <mailto:christian.koe...@amd.com>>
*Cc:* Alex Deucher <alexdeuc...@gmail.com 
<mailto:alexdeuc...@gmail.com>>; Michel Dänzer <mic...@daenzer.net 
<mailto:mic...@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:mar...@gmail.com]
*Sent:* Monday, March 19, 2018 3:39 PM
*To:* Koenig, Christian <christian.koe...@amd.com>
<mailto:christian.koe...@amd.com>
*Cc:* Li, Samuel <samuel...@amd.com> <mailto:samuel...@amd.com>;
Michel Dänzer <mic...@daenzer.net> <mailto:mic...@daenzer.net>;
    Alex Deucher <alexdeuc...@gmail.com>
<mailto:alexdeuc...@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.koe...@amd.com <mailto:christian.koe...@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 i

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

[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...@amd.com>; Koenig, Christian 
<christian.koe...@amd.com>; Marek Olšák <mar...@gmail.com>
Cc: Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer <mic...@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-boun...@lists.freedesktop.org<mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Li, Samuel <samuel...@amd.com<mailto:samuel...@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.leichtzumer...@gmail.com]
Sent: Monday, March 19, 2018 3:45 PM
To: Li, Samuel <samuel...@amd.com<mailto:samuel...@amd.com>>; Marek Olšák 
<mar...@gmail.com<mailto:mar...@gmail.com>>; Koenig, Christian 
<christian.koe...@amd.com<mailto:christian.koe...@amd.com>>
Cc: Alex Deucher <alexdeuc...@gmail.com<mailto:alexdeuc...@gmail.com>>; Michel 
Dänzer <mic...@daenzer.net<mailto:mic...@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:mar...@gmail.com]
Sent: Monday, March 19, 2018 3:39 PM
To: Koenig, Christian 
<christian.koe...@amd.com><mailto:christian.koe...@amd.com>
Cc: Li, Samuel <samuel...@amd.com><mailto:samuel...@amd.com>; Michel Dänzer 
<mic...@daenzer.net><mailto:mic...@daenzer.net>; Alex Deucher 
<alexdeuc...@gmail.com><mailto:alexdeuc...@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.koe...@amd.com<mailto:christian.koe...@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 <mic...@daenzer.net<mailto:mic...@daenzer.net>>; Li, Samuel
<samuel...@amd.com<mailto:samuel...@amd.com>>;

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

[Deucher, Alexander]

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


Alex


From: amd-gfx <amd-gfx-boun...@lists.freedesktop.org> on behalf of Li, Samuel 
<samuel...@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.leichtzumer...@gmail.com]
Sent: Monday, March 19, 2018 3:45 PM
To: Li, Samuel <samuel...@amd.com>; Marek Olšák <mar...@gmail.com>; Koenig, 
Christian <christian.koe...@amd.com>
Cc: Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer <mic...@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:mar...@gmail.com]
Sent: Monday, March 19, 2018 3:39 PM
To: Koenig, Christian 
<christian.koe...@amd.com><mailto:christian.koe...@amd.com>
Cc: Li, Samuel <samuel...@amd.com><mailto:samuel...@amd.com>; Michel Dänzer 
<mic...@daenzer.net><mailto:mic...@daenzer.net>; Alex Deucher 
<alexdeuc...@gmail.com><mailto:alexdeuc...@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.koe...@amd.com<mailto:christian.koe...@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 <mic...@daenzer.net<mailto:mic...@daenzer.net>>; Li, Samuel
<samuel...@amd.com<mailto:samuel...@amd.com>>; Alex Deucher 
<alexdeuc...@gmail.com<mailto:alexdeuc...@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 

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

[Li, Samuel]

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.leichtzumer...@gmail.com]
Sent: Monday, March 19, 2018 3:45 PM
To: Li, Samuel <samuel...@amd.com>; Marek Olšák <mar...@gmail.com>; Koenig, 
Christian <christian.koe...@amd.com>
Cc: Alex Deucher <alexdeuc...@gmail.com>; Michel Dänzer <mic...@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:mar...@gmail.com]
Sent: Monday, March 19, 2018 3:39 PM
To: Koenig, Christian 
<christian.koe...@amd.com><mailto:christian.koe...@amd.com>
Cc: Li, Samuel <samuel...@amd.com><mailto:samuel...@amd.com>; Michel Dänzer 
<mic...@daenzer.net><mailto:mic...@daenzer.net>; Alex Deucher 
<alexdeuc...@gmail.com><mailto:alexdeuc...@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.koe...@amd.com<mailto:christian.koe...@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 <mic...@daenzer.net<mailto:mic...@daenzer.net>>; Li, Samuel
<samuel...@amd.com<mailto:samuel...@amd.com>>; Alex Deucher 
<alexdeuc...@gmail.com<mailto:alexdeuc...@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

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

[Christian König]

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:mar...@gmail.com]
*Sent:* Monday, March 19, 2018 3:39 PM
*To:* Koenig, Christian <christian.koe...@amd.com>
*Cc:* Li, Samuel <samuel...@amd.com>; Michel Dänzer 
<mic...@daenzer.net>; Alex Deucher <alexdeuc...@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.koe...@amd.com <mailto:christian.koe...@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 <mic...@daenzer.net
<mailto:mic...@daenzer.net>>; Li, Samuel
<samuel...@amd.com <mailto:samuel...@amd.com>>; Alex
Deucher <alexdeuc...@gmail.com <mailto:alexdeuc...@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 buffe

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

[Christian König]

Am 19.03.2018 um 20:39 schrieb Marek Olšák:
On Mon, Mar 19, 2018 at 3:27 PM, Christian König 
<christian.koe...@amd.com <mailto:christian.koe...@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.


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 <mic...@daenzer.net
<mailto:mic...@daenzer.net>>; Li, Samuel
<samuel...@amd.com <mailto:samuel...@amd.com>>; Alex
Deucher <alexdeuc...@gmail.com <mailto:alexdeuc...@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
<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]

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:mar...@gmail.com]
Sent: Monday, March 19, 2018 3:39 PM
To: Koenig, Christian <christian.koe...@amd.com>
Cc: Li, Samuel <samuel...@amd.com>; Michel Dänzer <mic...@daenzer.net>; Alex 
Deucher <alexdeuc...@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.koe...@amd.com<mailto:christian.koe...@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 <mic...@daenzer.net<mailto:mic...@daenzer.net>>; Li, Samuel
<samuel...@amd.com<mailto:samuel...@amd.com>>; Alex Deucher 
<alexdeuc...@gmail.com<mailto:alexdeuc...@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
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Marek Olšák]

On Mon, Mar 19, 2018 at 3:27 PM, Christian König <christian.koe...@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 <mic...@daenzer.net>; Li, Samuel
>>> <samuel...@amd.com>; Alex Deucher <alexdeuc...@gmail.com>
>>> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
>>> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display
>>> support
>>>
>>> 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

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 <mic...@daenzer.net>; Li, Samuel
<samuel...@amd.com>; Alex Deucher <alexdeuc...@gmail.com>
Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

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 <mic...@daenzer.net>; Li, Samuel
> <samuel...@amd.com>; Alex Deucher <alexdeuc...@gmail.com>
> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
> 
> 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]

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

[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

[Alex Deucher]

On Wed, Mar 7, 2018 at 2:02 PM, Li, Samuel <samuel...@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...@amd.com]
>> Sent: Wednesday, March 07, 2018 1:54 PM
>> To: Alex Deucher <alexdeuc...@gmail.com>
>> Cc: Michel Dänzer <mic...@daenzer.net>; Koenig, Christian
>> <christian.koe...@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...@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...@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...@amd.com]
> Sent: Wednesday, March 07, 2018 1:54 PM
> To: Alex Deucher <alexdeuc...@gmail.com>
> Cc: Michel Dänzer <mic...@daenzer.net>; Koenig, Christian
> <christian.koe...@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...@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...@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  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  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  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  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]

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  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 06:38 PM, Alex Deucher wrote:
> On Wed, Mar 7, 2018 at 12:29 PM, Samuel Li  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

[Alex Deucher]

On Wed, Mar 7, 2018 at 12:29 PM, Samuel Li  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

[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

[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

[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 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 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-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 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: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

[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.leichtzumer...@gmail.com]
> Sent: Tuesday, March 06, 2018 1:02 PM
> To: Michel Dänzer <mic...@daenzer.net>; Koenig, Christian
> <christian.koe...@amd.com>; Li, Samuel <samuel...@amd.com>; Alex
> Deucher <alexdeuc...@gmail.com>
> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
> 
> 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

[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

[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:22 schrieb Li, Samuel:

And exactly that's the problematical assumption.

Not assumption, I tested. You have any typical use case that it might become a 
problem?


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


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





addition to that I agree with Michel that the module parameter is overkill.

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


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


Regards,
Christian.



Regards,
Samuel Li



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

And exactly that's the problematical assumption.

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

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

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

Regards,
Christian.

Am 06.03.2018 um 18:09 schrieb Li, Samuel:

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

module is loaded.

Regards,
Samuel Li



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

On Tue, Mar 6, 2018 at 11:49 AM, Samuel Li <samuel...@amd.com> wrote:

domain = amdgpu_display_framebuffer_domains(adev);
+if (domain == AMDGPU_GEM_DOMAIN_GTT) {
+DRM_INFO("Scatter gather display: enabled\n");
+} else if (domain & AMDGPU_GEM_DOMAIN_GTT) {
+DRM_INFO("Scatter gather display: auto\n");
+}

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

Christian.

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

suggestions?

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


Sam


On 2018-03-03 08:41 AM, Christian König wrote:

Am 03.03.2018 um 00:25 schrieb Samuel Li:

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 292c7e7..6b0ee34 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -129,6 +129,7 @@ extern int amdgpu_lbpw;
extern int amdgpu_compute_multipipe;
extern int amdgpu_gpu_recovery;
extern int amdgpu_emu_mode;
+extern int amdgpu_sg_display;
  #ifdef CONFIG_DRM_AMDGPU_SI
extern int amdgpu_si_support;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 5495b29..dfa11b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -513,8 +513,13 @@ uint32_t

amdgpu_display_framebuffer_domains(struct amdgpu_device *adev)

#if defined(CONFIG_DRM_AMD_DC)
if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type <

CHIP_RAVEN &&

adev->flags & AMD_IS_APU &&
-amdgpu_device_asic_has_dc_support(adev->asic_type))
-domain |= AMDGPU_GEM_DOMAIN_GTT;
+amdgpu_device_asic_has_dc_support(adev->asic_type)) {
+if (amdgpu_sg_display == 1) {
+domain = AMDGPU_GEM_DOMAIN_GTT;
+} else if (amdgpu_sg_display == -1) {
+domain |= AMDGPU_GEM_DOMAIN_GTT;
+}

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


+}
#endif
  return domain;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index e670936..f0ada24 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -132,6 +132,7 @@ int amdgpu_lbpw = -1;
int amdgpu_compute_multipipe = -1;
int amdgpu_gpu_recovery = -1; /* auto */
int amdgpu_emu_mode = 0;
+int amdgpu_sg_display = 1;
  MODULE_PARM_DESC(vramlimit, "Restrict VRAM fo

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...@amd.com>; Alex Deucher
> <alexdeuc...@gmail.com>
> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support
> 
> And exactly that's the problematical assumption.
> 
> This doesn't print only when the module is loaded, but rather when a
> framebuffer object is created.
> 
> And depending on the use case that can even be many many times per
> second.
> 
> Please remove that printing, addition to that I agree with Michel that the
> module parameter is overkill.
> 
> Regards,
> Christian.
> 
> Am 06.03.2018 um 18:09 schrieb Li, Samuel:
> > This information is kind of important, and it only prints once typically 
> > when
> module is loaded.
> >
> > Regards,
> > Samuel Li
> >
> >
> >> -Original Message-
> >> From: Alex Deucher [mailto:alexdeuc...@gmail.com]
> >> Sent: Tuesday, March 06, 2018 12:04 PM
> >> To: Li, Samuel <samuel...@amd.com>
> >> Cc: Koenig, Christian <christian.koe...@amd.com>; amd-gfx list  >> g...@lists.freedesktop.org>
> >> Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display
> >> support
> >>
> >> On Tue, Mar 6, 2018 at 11:49 AM, Samuel Li <samuel...@amd.com> wrote:
> >>>>>domain = amdgpu_display_framebuffer_domains(adev);
> >>>>> +if (domain == AMDGPU_GEM_DOMAIN_GTT) {
> >>>>> +DRM_INFO("Scatter gather display: enabled\n");
> >>>>> +} else if (domain & AMDGPU_GEM_DOMAIN_GTT) {
> >>>>> +DRM_INFO("Scatter gather display: auto\n");
> >>>>> +}
> >>>> Dito and printing that here is not a good idea as far as I can see.
> >>>>
> >>>> Christian.
> >>>
> >>> The intention here is to print out when fb is created. You have
> >>> other
> >> suggestions?
> >>
> >> Make it DRM_DEBUG?  otherwise you'll spam the logs.
> >>
> >>> Sam
> >>>
> >>>
> >>> On 2018-03-03 08:41 AM, Christian König wrote:
> >>>> Am 03.03.2018 um 00:25 schrieb Samuel Li:
> >>>>> It's enabled by default. -1 is auto, to allow both vram and gtt
> >>>>> memory be available, for testing purpose only.
> >>>>> ---
> >>>>>drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> >>>>>drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 9 +++--
> >>>>>drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 
> >>>>>drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c  | 5 +
> >>>>>4 files changed, 17 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>>>> index 292c7e7..6b0ee34 100644
> >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>>>> @@ -129,6 +129,7 @@ extern int amdgpu_lbpw;
> >>>>>extern int amdgpu_compute_multipipe;
> >>>>>extern int amdgpu_gpu_recovery;
> >>>>>extern int amdgpu_emu_mode;
> >>>>> +extern int amdgpu_sg_display;
> >>>>>  #ifdef CONFIG_DRM_AMDGPU_SI
> >>>>>extern int amdgpu_si_support;
> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> >>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> >>>>> index 5495b29..dfa11b1 100644
> >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> >>>>> @@ -513,8 +513,13 @@ uint32_t
> >> amdgpu_display_framebuffer_domains(struct amdgpu_device *adev)
> >>>>>#if defined

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:alexdeuc...@gmail.com]
Sent: Tuesday, March 06, 2018 12:04 PM
To: Li, Samuel <samuel...@amd.com>
Cc: Koenig, Christian <christian.koe...@amd.com>; amd-gfx list 
Subject: Re: [PATCH 1/2] drm/amdgpu: Enable scatter gather display support

On Tue, Mar 6, 2018 at 11:49 AM, Samuel Li <samuel...@amd.com> wrote:

   domain = amdgpu_display_framebuffer_domains(adev);
+if (domain == AMDGPU_GEM_DOMAIN_GTT) {
+DRM_INFO("Scatter gather display: enabled\n");
+} else if (domain & AMDGPU_GEM_DOMAIN_GTT) {
+DRM_INFO("Scatter gather display: auto\n");
+}

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

Christian.


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

suggestions?

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


Sam


On 2018-03-03 08:41 AM, Christian König wrote:

Am 03.03.2018 um 00:25 schrieb Samuel Li:

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 292c7e7..6b0ee34 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -129,6 +129,7 @@ extern int amdgpu_lbpw;
   extern int amdgpu_compute_multipipe;
   extern int amdgpu_gpu_recovery;
   extern int amdgpu_emu_mode;
+extern int amdgpu_sg_display;
 #ifdef CONFIG_DRM_AMDGPU_SI
   extern int amdgpu_si_support;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 5495b29..dfa11b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -513,8 +513,13 @@ uint32_t

amdgpu_display_framebuffer_domains(struct amdgpu_device *adev)

   #if defined(CONFIG_DRM_AMD_DC)
   if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type <

CHIP_RAVEN &&

   adev->flags & AMD_IS_APU &&
-amdgpu_device_asic_has_dc_support(adev->asic_type))
-domain |= AMDGPU_GEM_DOMAIN_GTT;
+amdgpu_device_asic_has_dc_support(adev->asic_type)) {
+if (amdgpu_sg_display == 1) {
+domain = AMDGPU_GEM_DOMAIN_GTT;
+} else if (amdgpu_sg_display == -1) {
+domain |= AMDGPU_GEM_DOMAIN_GTT;
+}

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


+}
   #endif
 return domain;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index e670936..f0ada24 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -132,6 +132,7 @@ int amdgpu_lbpw = -1;
   int amdgpu_compute_multipipe = -1;
   int amdgpu_gpu_recovery = -1; /* auto */
   int amdgpu_emu_mode = 0;
+int amdgpu_sg_display = 1;
 MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in

megabytes");

   module_param_named(vramlimit, amdgpu_vram_limit, int, 0600); @@
-290,6 +291,9 @@ module_param_named(gpu_recovery,

amdgpu_gpu_recovery, int, 0444);

   MODULE_PARM_DESC(emu_mode, "Emulation mode, (1 = enable, 0 =

disable)");

   module_param_named(emu_mode, amdgpu_emu_mode, int, 0444);
   +MODULE_PARM_DESC(sg_display, "Enable scatter gather display, (1 =
enable, 0 = disable, -1 = auto");
+module_param_named(sg_display, amdgpu_sg_display, int, 0444);
+
   #ifdef CONFIG_DRM_AMDGPU_SI
 #if defined(CONFIG_DRM_RADEON) ||
defined(CONFIG_DRM_RADEON_MODULE) diff --git
a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index 1206301..10f1f4f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -138,6 +138,11 @@ static int

amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,

   mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd-

width, cpp,

 fb_tiled);
   domain = amdgpu_display_framebuffer_domains(adev);
+if (domain == AMDGPU_GEM_DOMAIN_GTT) {
+DRM_INFO("Scatter gather d

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

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

[Alex Deucher]

On Tue, Mar 6, 2018 at 11:49 AM, Samuel Li  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

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

[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

[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

[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