Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d is enabled

2023-11-03 Thread Sripada, Radhakrishna 
Hi Andrzej,

The patch mentioned below does not help with the issue.

Thanks,
RK

> -Original Message-
> From: Hajda, Andrzej 
> Sent: Friday, November 3, 2023 2:18 PM
> To: Sripada, Radhakrishna ; Tvrtko Ursulin
> ; intel-gfx@lists.freedesktop.org
> Cc: Chris Wilson ; Vivi, Rodrigo
> 
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d 
> is
> enabled
> 
> 
> 
> On 03.11.2023 16:53, Sripada, Radhakrishna wrote:
> > Hi Tvrtko,
> >
> >> -Original Message-
> >> From: Tvrtko Ursulin 
> >> Sent: Friday, November 3, 2023 1:30 AM
> >> To: Sripada, Radhakrishna ; Hajda, Andrzej
> >> ; intel-gfx@lists.freedesktop.org
> >> Cc: Chris Wilson 
> >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when 
> >> vt-d
> is
> >> enabled
> >>
> >>
> >> On 02/11/2023 22:14, Sripada, Radhakrishna wrote:
> >>> Hi Tvrtko,
> >>>
> >>>> -Original Message-
> >>>> From: Tvrtko Ursulin 
> >>>> Sent: Thursday, November 2, 2023 10:41 AM
> >>>> To: Hajda, Andrzej ; Sripada, Radhakrishna
> >>>> ; intel-gfx@lists.freedesktop.org
> >>>> Cc: Chris Wilson 
> >>>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when
> vt-d
> >> is
> >>>> enabled
> >>>>
> >>>>
> >>>> On 02/11/2023 16:58, Andrzej Hajda wrote:
> >>>>> On 02.11.2023 17:06, Radhakrishna Sripada wrote:
> >>>>>> Experiments were conducted with different multipliers to VTD_GUARD
> >> macro
> >>>>>> with multiplier of 185 we were observing occasional pipe faults when
> >>>>>> running kms_cursor_legacy --run-subtest single-bo
> >>>>>>
> >>>>>> There could possibly be an underlying issue that is being
> >>>>>> investigated, for
> >>>>>> now bump the guard pages for MTL.
> >>>>>>
> >>>>>> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2017
> >>>>>> Cc: Gustavo Sousa 
> >>>>>> Cc: Chris Wilson 
> >>>>>> Signed-off-by: Radhakrishna Sripada 
> >>>>>> ---
> >>>>>>     drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++
> >>>>>>     1 file changed, 3 insertions(+)
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> >>>>>> b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> >>>>>> index 3770828f2eaf..b65f84c6bb3f 100644
> >>>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> >>>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> >>>>>> @@ -456,6 +456,9 @@ i915_gem_object_pin_to_display_plane(struct
> >>>>>> drm_i915_gem_object *obj,
> >>>>>>     if (intel_scanout_needs_vtd_wa(i915)) {
> >>>>>>     unsigned int guard = VTD_GUARD;
> >>>>>> +    if (IS_METEORLAKE(i915))
> >>>>>> +    guard *= 200;
> >>>>>> +
> >>>>> 200 * VTD_GUARD = 200 * 168 * 4K = 131MB
> >>>>>
> >>>>> Looks insanely high, 131MB for padding, if this is before and after it
> >>>>> becomes even 262MB of wasted address per plane. Just signalling, I do
> >>>>> not know if this actually hurts.
> >>>> Yeah this feels crazy. There must be some other explanation which is
> >>>> getting hidden by the crazy amount of padding so I'd rather we figured
> >>>> it out.
> >>>>
> >>>> With 262MiB per fb how many fit in GGTT before eviction hits? N screens
> >>>> with double/triple buffering?
> >>> I believe with this method we will have to limit the no of frame buffers 
> >>> in the
> >> system. One alternative
> >>> that worked is to do a proper clear range for the ggtt instead of doing a 
> >>> nop.
> >> Although it adds marginal
> >>> time during suspend/resume/boot it does not add restrictions to the no of
> fb's
> >> that can be used.
> >>
> >> And if we remember the guard pages replaced clearing to scratch, to
> >> improve suspend resume times, exactly for improving user experience. :(
> >>
> >> Luckily there is time to fix this properly on MTL one way or the other.
> >> Is it just kms_cursor_legacy --run-subtest single-bo that is affected?
> > I am trying to dump the page table entries at the time of failure for bot 
> > the fame
> buffer and if required
> > For the guard pages. Will see if I get some info from there.
> >
> > Yes the test kms_cursor_legacy is used to reliably reproduce. Looking at 
> > public
> CI, I also see pipe errors
> > being reported with varying occurrences while running kms_cursor_crc,
> kms_pipe_crc_basic,
> > and kms_plane_scaling. More details on the occurrence can be found here [1].
> >
> > Thanks,
> > RK
> >
> > 1. http://gfx-ci.igk.intel.com/cibuglog-
> ng/results/knownfailures?query_key=d9c3297dd17dda35a6c638eb96b3139bd1a
> 6633c
> 
> Could you check if [1] helps?
> 
> [1]: https://patchwork.freedesktop.org/series/125926/
> 
> Regards
> Andrzej
> 
> >> Regards,
> >>
> >> Tvrtko
> >>
> >>
> >>>> Regards,
> >>>>
> >>>> Tvrtko
> >>>>
> >>>> P.S. Where did the 185 from the commit message come from?
> >>> 185 came from experiment to increase the guard size. It is not a standard
> >> number.
> >>> Regards,
> >>> Radhakrishna(RK) Sripada

Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d is enabled

2023-11-03 Thread Andrzej Hajda 




On 03.11.2023 16:53, Sripada, Radhakrishna wrote:

Hi Tvrtko,


-Original Message-
From: Tvrtko Ursulin 
Sent: Friday, November 3, 2023 1:30 AM
To: Sripada, Radhakrishna ; Hajda, Andrzej
; intel-gfx@lists.freedesktop.org
Cc: Chris Wilson 
Subject: Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d is
enabled


On 02/11/2023 22:14, Sripada, Radhakrishna wrote:

Hi Tvrtko,


-Original Message-
From: Tvrtko Ursulin 
Sent: Thursday, November 2, 2023 10:41 AM
To: Hajda, Andrzej ; Sripada, Radhakrishna
; intel-gfx@lists.freedesktop.org
Cc: Chris Wilson 
Subject: Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d

is

enabled


On 02/11/2023 16:58, Andrzej Hajda wrote:

On 02.11.2023 17:06, Radhakrishna Sripada wrote:

Experiments were conducted with different multipliers to VTD_GUARD

macro

with multiplier of 185 we were observing occasional pipe faults when
running kms_cursor_legacy --run-subtest single-bo

There could possibly be an underlying issue that is being
investigated, for
now bump the guard pages for MTL.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2017
Cc: Gustavo Sousa 
Cc: Chris Wilson 
Signed-off-by: Radhakrishna Sripada 
---
    drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++
    1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 3770828f2eaf..b65f84c6bb3f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -456,6 +456,9 @@ i915_gem_object_pin_to_display_plane(struct
drm_i915_gem_object *obj,
    if (intel_scanout_needs_vtd_wa(i915)) {
    unsigned int guard = VTD_GUARD;
+    if (IS_METEORLAKE(i915))
+    guard *= 200;
+

200 * VTD_GUARD = 200 * 168 * 4K = 131MB

Looks insanely high, 131MB for padding, if this is before and after it
becomes even 262MB of wasted address per plane. Just signalling, I do
not know if this actually hurts.

Yeah this feels crazy. There must be some other explanation which is
getting hidden by the crazy amount of padding so I'd rather we figured
it out.

With 262MiB per fb how many fit in GGTT before eviction hits? N screens
with double/triple buffering?

I believe with this method we will have to limit the no of frame buffers in the

system. One alternative

that worked is to do a proper clear range for the ggtt instead of doing a nop.

Although it adds marginal

time during suspend/resume/boot it does not add restrictions to the no of fb's

that can be used.

And if we remember the guard pages replaced clearing to scratch, to
improve suspend resume times, exactly for improving user experience. :(

Luckily there is time to fix this properly on MTL one way or the other.
Is it just kms_cursor_legacy --run-subtest single-bo that is affected?

I am trying to dump the page table entries at the time of failure for bot the 
fame buffer and if required
For the guard pages. Will see if I get some info from there.

Yes the test kms_cursor_legacy is used to reliably reproduce. Looking at public 
CI, I also see pipe errors
being reported with varying occurrences while running kms_cursor_crc, 
kms_pipe_crc_basic,
and kms_plane_scaling. More details on the occurrence can be found here [1].

Thanks,
RK

1. 
http://gfx-ci.igk.intel.com/cibuglog-ng/results/knownfailures?query_key=d9c3297dd17dda35a6c638eb96b3139bd1a6633c


Could you check if [1] helps?

[1]: https://patchwork.freedesktop.org/series/125926/

Regards
Andrzej


Regards,

Tvrtko



Regards,

Tvrtko

P.S. Where did the 185 from the commit message come from?

185 came from experiment to increase the guard size. It is not a standard

number.

Regards,
Radhakrishna(RK) Sripada

Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d is enabled

2023-11-03 Thread Sripada, Radhakrishna 
Hi Rodrigo,

> -Original Message-
> From: Vivi, Rodrigo 
> Sent: Friday, November 3, 2023 8:35 AM
> To: Sousa, Gustavo 
> Cc: Sripada, Radhakrishna ; intel-
> g...@lists.freedesktop.org; Chris Wilson 
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d 
> is
> enabled
> 
> On Thu, Nov 02, 2023 at 01:35:53PM -0300, Gustavo Sousa wrote:
> > Quoting Radhakrishna Sripada (2023-11-02 13:06:44-03:00)
> > >Experiments were conducted with different multipliers to VTD_GUARD macro
> > >with multiplier of 185 we were observing occasional pipe faults when
> > >running kms_cursor_legacy --run-subtest single-bo
> > >
> > >There could possibly be an underlying issue that is being investigated, for
> > >now bump the guard pages for MTL.
> > >
> > >Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2017
> > >Cc: Gustavo Sousa 
> > >Cc: Chris Wilson 
> > >Signed-off-by: Radhakrishna Sripada 
> > >---
> > > drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > >diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> > >index 3770828f2eaf..b65f84c6bb3f 100644
> > >--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> > >+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> > >@@ -456,6 +456,9 @@ i915_gem_object_pin_to_display_plane(struct
> drm_i915_gem_object *obj,
> > > if (intel_scanout_needs_vtd_wa(i915)) {
> > > unsigned int guard = VTD_GUARD;
> > >
> >
> > I remember trying increasing the guard, but with a much smaller multiplier. 
> > So
> > it turns out that using a much higher value did the "trick".
> 
> a much smaller multiplier could mess with the flags range?
> it is really hard to understand what of that 'flags' is really those 12 flags
> or what is this 'guard' and where that ends up...
Based on my glance, if the multiplier fits in 32 bits then it should work. But 
the
problem here pointed out by Tvrtko is that we are adding awful lot(262 mb per 
fb) of padding
in the gurad pages clobbering up the ggtt address pace. Enough(10 to 20) fb's 
created we
will fall into the realm of evictions.

Regards,
Radhakrishna(RK) Sripada

> 
> >
> > I would add a FIXME comment here to remind us that this is a hack.
> >
> > With the FIXME in place,
> >
> > Reviewed-by: Gustavo Sousa 
> >
> > >+if (IS_METEORLAKE(i915))
> > >+guard *= 200;
> > >+
> > > if (i915_gem_object_is_tiled(obj))
> > > guard = max(guard,
> > > 
> > > i915_gem_object_get_tile_row_size(obj));
> > >--
> > >2.34.1
> > >

Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d is enabled

2023-11-03 Thread Sripada, Radhakrishna 
Hi Tvrtko,

> -Original Message-
> From: Tvrtko Ursulin 
> Sent: Friday, November 3, 2023 1:30 AM
> To: Sripada, Radhakrishna ; Hajda, Andrzej
> ; intel-gfx@lists.freedesktop.org
> Cc: Chris Wilson 
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d 
> is
> enabled
> 
> 
> On 02/11/2023 22:14, Sripada, Radhakrishna wrote:
> > Hi Tvrtko,
> >
> >> -Original Message-
> >> From: Tvrtko Ursulin 
> >> Sent: Thursday, November 2, 2023 10:41 AM
> >> To: Hajda, Andrzej ; Sripada, Radhakrishna
> >> ; intel-gfx@lists.freedesktop.org
> >> Cc: Chris Wilson 
> >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when 
> >> vt-d
> is
> >> enabled
> >>
> >>
> >> On 02/11/2023 16:58, Andrzej Hajda wrote:
> >>> On 02.11.2023 17:06, Radhakrishna Sripada wrote:
> >>>> Experiments were conducted with different multipliers to VTD_GUARD
> macro
> >>>> with multiplier of 185 we were observing occasional pipe faults when
> >>>> running kms_cursor_legacy --run-subtest single-bo
> >>>>
> >>>> There could possibly be an underlying issue that is being
> >>>> investigated, for
> >>>> now bump the guard pages for MTL.
> >>>>
> >>>> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2017
> >>>> Cc: Gustavo Sousa 
> >>>> Cc: Chris Wilson 
> >>>> Signed-off-by: Radhakrishna Sripada 
> >>>> ---
> >>>>    drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++
> >>>>    1 file changed, 3 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> >>>> b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> >>>> index 3770828f2eaf..b65f84c6bb3f 100644
> >>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> >>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> >>>> @@ -456,6 +456,9 @@ i915_gem_object_pin_to_display_plane(struct
> >>>> drm_i915_gem_object *obj,
> >>>>    if (intel_scanout_needs_vtd_wa(i915)) {
> >>>>    unsigned int guard = VTD_GUARD;
> >>>> +    if (IS_METEORLAKE(i915))
> >>>> +    guard *= 200;
> >>>> +
> >>>
> >>> 200 * VTD_GUARD = 200 * 168 * 4K = 131MB
> >>>
> >>> Looks insanely high, 131MB for padding, if this is before and after it
> >>> becomes even 262MB of wasted address per plane. Just signalling, I do
> >>> not know if this actually hurts.
> >>
> >> Yeah this feels crazy. There must be some other explanation which is
> >> getting hidden by the crazy amount of padding so I'd rather we figured
> >> it out.
> >>
> >> With 262MiB per fb how many fit in GGTT before eviction hits? N screens
> >> with double/triple buffering?
> >
> > I believe with this method we will have to limit the no of frame buffers in 
> > the
> system. One alternative
> > that worked is to do a proper clear range for the ggtt instead of doing a 
> > nop.
> Although it adds marginal
> > time during suspend/resume/boot it does not add restrictions to the no of 
> > fb's
> that can be used.
> 
> And if we remember the guard pages replaced clearing to scratch, to
> improve suspend resume times, exactly for improving user experience. :(
> 
> Luckily there is time to fix this properly on MTL one way or the other.
> Is it just kms_cursor_legacy --run-subtest single-bo that is affected?

I am trying to dump the page table entries at the time of failure for bot the 
fame buffer and if required
For the guard pages. Will see if I get some info from there.

Yes the test kms_cursor_legacy is used to reliably reproduce. Looking at public 
CI, I also see pipe errors
being reported with varying occurrences while running kms_cursor_crc, 
kms_pipe_crc_basic,
and kms_plane_scaling. More details on the occurrence can be found here [1].

Thanks,
RK

1. 
http://gfx-ci.igk.intel.com/cibuglog-ng/results/knownfailures?query_key=d9c3297dd17dda35a6c638eb96b3139bd1a6633c

> 
> Regards,
> 
> Tvrtko
> 
> 
> >>
> >> Regards,
> >>
> >> Tvrtko
> >>
> >> P.S. Where did the 185 from the commit message come from?
> > 185 came from experiment to increase the guard size. It is not a standard
> number.
> >
> > Regards,
> > Radhakrishna(RK) Sripada

Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d is enabled

2023-11-03 Thread Rodrigo Vivi 
On Thu, Nov 02, 2023 at 01:35:53PM -0300, Gustavo Sousa wrote:
> Quoting Radhakrishna Sripada (2023-11-02 13:06:44-03:00)
> >Experiments were conducted with different multipliers to VTD_GUARD macro
> >with multiplier of 185 we were observing occasional pipe faults when
> >running kms_cursor_legacy --run-subtest single-bo
> >
> >There could possibly be an underlying issue that is being investigated, for
> >now bump the guard pages for MTL.
> >
> >Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2017
> >Cc: Gustavo Sousa 
> >Cc: Chris Wilson 
> >Signed-off-by: Radhakrishna Sripada 
> >---
> > drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c 
> >b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> >index 3770828f2eaf..b65f84c6bb3f 100644
> >--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> >+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> >@@ -456,6 +456,9 @@ i915_gem_object_pin_to_display_plane(struct 
> >drm_i915_gem_object *obj,
> > if (intel_scanout_needs_vtd_wa(i915)) {
> > unsigned int guard = VTD_GUARD;
> > 
> 
> I remember trying increasing the guard, but with a much smaller multiplier. So
> it turns out that using a much higher value did the "trick".

a much smaller multiplier could mess with the flags range?
it is really hard to understand what of that 'flags' is really those 12 flags
or what is this 'guard' and where that ends up...

> 
> I would add a FIXME comment here to remind us that this is a hack.
> 
> With the FIXME in place,
> 
> Reviewed-by: Gustavo Sousa 
> 
> >+if (IS_METEORLAKE(i915))
> >+guard *= 200;
> >+
> > if (i915_gem_object_is_tiled(obj))
> > guard = max(guard,
> > i915_gem_object_get_tile_row_size(obj));
> >-- 
> >2.34.1
> >

Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d is enabled

2023-11-03 Thread Tvrtko Ursulin 



On 02/11/2023 22:14, Sripada, Radhakrishna wrote:

Hi Tvrtko,


-Original Message-
From: Tvrtko Ursulin 
Sent: Thursday, November 2, 2023 10:41 AM
To: Hajda, Andrzej ; Sripada, Radhakrishna
; intel-gfx@lists.freedesktop.org
Cc: Chris Wilson 
Subject: Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d is
enabled


On 02/11/2023 16:58, Andrzej Hajda wrote:

On 02.11.2023 17:06, Radhakrishna Sripada wrote:

Experiments were conducted with different multipliers to VTD_GUARD macro
with multiplier of 185 we were observing occasional pipe faults when
running kms_cursor_legacy --run-subtest single-bo

There could possibly be an underlying issue that is being
investigated, for
now bump the guard pages for MTL.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2017
Cc: Gustavo Sousa 
Cc: Chris Wilson 
Signed-off-by: Radhakrishna Sripada 
---
   drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++
   1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 3770828f2eaf..b65f84c6bb3f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -456,6 +456,9 @@ i915_gem_object_pin_to_display_plane(struct
drm_i915_gem_object *obj,
   if (intel_scanout_needs_vtd_wa(i915)) {
   unsigned int guard = VTD_GUARD;
+    if (IS_METEORLAKE(i915))
+    guard *= 200;
+


200 * VTD_GUARD = 200 * 168 * 4K = 131MB

Looks insanely high, 131MB for padding, if this is before and after it
becomes even 262MB of wasted address per plane. Just signalling, I do
not know if this actually hurts.


Yeah this feels crazy. There must be some other explanation which is
getting hidden by the crazy amount of padding so I'd rather we figured
it out.

With 262MiB per fb how many fit in GGTT before eviction hits? N screens
with double/triple buffering?


I believe with this method we will have to limit the no of frame buffers in the 
system. One alternative
that worked is to do a proper clear range for the ggtt instead of doing a nop. 
Although it adds marginal
time during suspend/resume/boot it does not add restrictions to the no of fb's 
that can be used.


And if we remember the guard pages replaced clearing to scratch, to 
improve suspend resume times, exactly for improving user experience. :(


Luckily there is time to fix this properly on MTL one way or the other. 
Is it just kms_cursor_legacy --run-subtest single-bo that is affected?


Regards,

Tvrtko




Regards,

Tvrtko

P.S. Where did the 185 from the commit message come from?

185 came from experiment to increase the guard size. It is not a standard 
number.

Regards,
Radhakrishna(RK) Sripada

Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d is enabled

2023-11-02 Thread Sripada, Radhakrishna 
Hi Tvrtko,

> -Original Message-
> From: Tvrtko Ursulin 
> Sent: Thursday, November 2, 2023 10:41 AM
> To: Hajda, Andrzej ; Sripada, Radhakrishna
> ; intel-gfx@lists.freedesktop.org
> Cc: Chris Wilson 
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d 
> is
> enabled
> 
> 
> On 02/11/2023 16:58, Andrzej Hajda wrote:
> > On 02.11.2023 17:06, Radhakrishna Sripada wrote:
> >> Experiments were conducted with different multipliers to VTD_GUARD macro
> >> with multiplier of 185 we were observing occasional pipe faults when
> >> running kms_cursor_legacy --run-subtest single-bo
> >>
> >> There could possibly be an underlying issue that is being
> >> investigated, for
> >> now bump the guard pages for MTL.
> >>
> >> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2017
> >> Cc: Gustavo Sousa 
> >> Cc: Chris Wilson 
> >> Signed-off-by: Radhakrishna Sripada 
> >> ---
> >>   drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++
> >>   1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> >> b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> >> index 3770828f2eaf..b65f84c6bb3f 100644
> >> --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> >> +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> >> @@ -456,6 +456,9 @@ i915_gem_object_pin_to_display_plane(struct
> >> drm_i915_gem_object *obj,
> >>   if (intel_scanout_needs_vtd_wa(i915)) {
> >>   unsigned int guard = VTD_GUARD;
> >> +    if (IS_METEORLAKE(i915))
> >> +    guard *= 200;
> >> +
> >
> > 200 * VTD_GUARD = 200 * 168 * 4K = 131MB
> >
> > Looks insanely high, 131MB for padding, if this is before and after it
> > becomes even 262MB of wasted address per plane. Just signalling, I do
> > not know if this actually hurts.
> 
> Yeah this feels crazy. There must be some other explanation which is
> getting hidden by the crazy amount of padding so I'd rather we figured
> it out.
> 
> With 262MiB per fb how many fit in GGTT before eviction hits? N screens
> with double/triple buffering?

I believe with this method we will have to limit the no of frame buffers in the 
system. One alternative
that worked is to do a proper clear range for the ggtt instead of doing a nop. 
Although it adds marginal
time during suspend/resume/boot it does not add restrictions to the no of fb's 
that can be used.
 
> 
> Regards,
> 
> Tvrtko
> 
> P.S. Where did the 185 from the commit message come from?
185 came from experiment to increase the guard size. It is not a standard 
number.

Regards,
Radhakrishna(RK) Sripada

Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d is enabled

2023-11-02 Thread Tvrtko Ursulin 



On 02/11/2023 16:58, Andrzej Hajda wrote:

On 02.11.2023 17:06, Radhakrishna Sripada wrote:

Experiments were conducted with different multipliers to VTD_GUARD macro
with multiplier of 185 we were observing occasional pipe faults when
running kms_cursor_legacy --run-subtest single-bo

There could possibly be an underlying issue that is being 
investigated, for

now bump the guard pages for MTL.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2017
Cc: Gustavo Sousa 
Cc: Chris Wilson 
Signed-off-by: Radhakrishna Sripada 
---
  drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c 
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c

index 3770828f2eaf..b65f84c6bb3f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -456,6 +456,9 @@ i915_gem_object_pin_to_display_plane(struct 
drm_i915_gem_object *obj,

  if (intel_scanout_needs_vtd_wa(i915)) {
  unsigned int guard = VTD_GUARD;
+    if (IS_METEORLAKE(i915))
+    guard *= 200;
+


200 * VTD_GUARD = 200 * 168 * 4K = 131MB

Looks insanely high, 131MB for padding, if this is before and after it 
becomes even 262MB of wasted address per plane. Just signalling, I do 
not know if this actually hurts.


Yeah this feels crazy. There must be some other explanation which is 
getting hidden by the crazy amount of padding so I'd rather we figured 
it out.


With 262MiB per fb how many fit in GGTT before eviction hits? N screens 
with double/triple buffering?


Regards,

Tvrtko

P.S. Where did the 185 from the commit message come from?

Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d is enabled

2023-11-02 Thread Gustavo Sousa 
Quoting Gustavo Sousa (2023-11-02 13:35:53-03:00)
>Quoting Radhakrishna Sripada (2023-11-02 13:06:44-03:00)
>>Experiments were conducted with different multipliers to VTD_GUARD macro
>>with multiplier of 185 we were observing occasional pipe faults when
>>running kms_cursor_legacy --run-subtest single-bo
>>
>>There could possibly be an underlying issue that is being investigated, for
>>now bump the guard pages for MTL.
>>
>>Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2017
>>Cc: Gustavo Sousa 
>>Cc: Chris Wilson 
>>Signed-off-by: Radhakrishna Sripada 
>>---
>> drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c 
>>b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
>>index 3770828f2eaf..b65f84c6bb3f 100644
>>--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
>>+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
>>@@ -456,6 +456,9 @@ i915_gem_object_pin_to_display_plane(struct 
>>drm_i915_gem_object *obj,
>> if (intel_scanout_needs_vtd_wa(i915)) {
>> unsigned int guard = VTD_GUARD;
>> 
>
>I remember trying increasing the guard, but with a much smaller multiplier. So
>it turns out that using a much higher value did the "trick".
>
>I would add a FIXME comment here to remind us that this is a hack.
>
>With the FIXME in place,
>
>Reviewed-by: Gustavo Sousa 

Oops. I was too hasty on providing an r-b and did not really pay attention to
the resulting size of the padding and its implications as highlighted by the
other replies here. My bad, sorry about that. Please dismiss the r-b.

--
Gustavo Sousa

>
>>+if (IS_METEORLAKE(i915))
>>+guard *= 200;
>>+
>> if (i915_gem_object_is_tiled(obj))
>> guard = max(guard,
>> i915_gem_object_get_tile_row_size(obj));
>>-- 
>>2.34.1
>>

Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d is enabled

2023-11-02 Thread Andrzej Hajda 

On 02.11.2023 17:06, Radhakrishna Sripada wrote:

Experiments were conducted with different multipliers to VTD_GUARD macro
with multiplier of 185 we were observing occasional pipe faults when
running kms_cursor_legacy --run-subtest single-bo

There could possibly be an underlying issue that is being investigated, for
now bump the guard pages for MTL.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2017
Cc: Gustavo Sousa 
Cc: Chris Wilson 
Signed-off-by: Radhakrishna Sripada 
---
  drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c 
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 3770828f2eaf..b65f84c6bb3f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -456,6 +456,9 @@ i915_gem_object_pin_to_display_plane(struct 
drm_i915_gem_object *obj,
if (intel_scanout_needs_vtd_wa(i915)) {
unsigned int guard = VTD_GUARD;
  
+		if (IS_METEORLAKE(i915))

+   guard *= 200;
+


200 * VTD_GUARD = 200 * 168 * 4K = 131MB

Looks insanely high, 131MB for padding, if this is before and after it 
becomes even 262MB of wasted address per plane. Just signalling, I do 
not know if this actually hurts.


Regards
Andrzej




if (i915_gem_object_is_tiled(obj))
guard = max(guard,
i915_gem_object_get_tile_row_size(obj));

Re: [Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d is enabled

2023-11-02 Thread Gustavo Sousa 
Quoting Radhakrishna Sripada (2023-11-02 13:06:44-03:00)
>Experiments were conducted with different multipliers to VTD_GUARD macro
>with multiplier of 185 we were observing occasional pipe faults when
>running kms_cursor_legacy --run-subtest single-bo
>
>There could possibly be an underlying issue that is being investigated, for
>now bump the guard pages for MTL.
>
>Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2017
>Cc: Gustavo Sousa 
>Cc: Chris Wilson 
>Signed-off-by: Radhakrishna Sripada 
>---
> drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c 
>b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
>index 3770828f2eaf..b65f84c6bb3f 100644
>--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
>+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
>@@ -456,6 +456,9 @@ i915_gem_object_pin_to_display_plane(struct 
>drm_i915_gem_object *obj,
> if (intel_scanout_needs_vtd_wa(i915)) {
> unsigned int guard = VTD_GUARD;
> 

I remember trying increasing the guard, but with a much smaller multiplier. So
it turns out that using a much higher value did the "trick".

I would add a FIXME comment here to remind us that this is a hack.

With the FIXME in place,

Reviewed-by: Gustavo Sousa 

>+if (IS_METEORLAKE(i915))
>+guard *= 200;
>+
> if (i915_gem_object_is_tiled(obj))
> guard = max(guard,
> i915_gem_object_get_tile_row_size(obj));
>-- 
>2.34.1
>

[Intel-gfx] [PATCH] drm/i915/mtl: Increase guard pages when vt-d is enabled

2023-11-02 Thread Radhakrishna Sripada 
Experiments were conducted with different multipliers to VTD_GUARD macro
with multiplier of 185 we were observing occasional pipe faults when
running kms_cursor_legacy --run-subtest single-bo

There could possibly be an underlying issue that is being investigated, for
now bump the guard pages for MTL.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2017
Cc: Gustavo Sousa 
Cc: Chris Wilson 
Signed-off-by: Radhakrishna Sripada 
---
 drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c 
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 3770828f2eaf..b65f84c6bb3f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -456,6 +456,9 @@ i915_gem_object_pin_to_display_plane(struct 
drm_i915_gem_object *obj,
if (intel_scanout_needs_vtd_wa(i915)) {
unsigned int guard = VTD_GUARD;
 
+   if (IS_METEORLAKE(i915))
+   guard *= 200;
+
if (i915_gem_object_is_tiled(obj))
guard = max(guard,
i915_gem_object_get_tile_row_size(obj));
-- 
2.34.1