* [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix sel fetch plane offset calculation
2021-07-31 0:10 [Intel-gfx] [PATCH 1/4] drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications José Roberto de Souza
@ 2021-07-31 0:10 ` José Roberto de Souza
2021-08-13 7:40 ` Gwan-gyeong Mun
2021-07-31 0:10 ` [Intel-gfx] [PATCH 3/4] drm/i915: Nuke ORIGIN_GTT José Roberto de Souza
` (4 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: José Roberto de Souza @ 2021-07-31 0:10 UTC (permalink / raw)
To: intel-gfx
Cc: Gwan-gyeong Mun, Ville Syrjälä, José Roberto de Souza
skl_calc_main_surface_offset() is used to calculate an aligned plane
surface address considering the inner framebuffer x and y offset.
It can not be used by selective fetch functions becase there is no
PLANE_SEL_FETCH_SURF.
So the PLANE_SEL_FETCH_OFFSET.y should only be PLANE_OFFSET.y +
damaged_area_within_plane.y1.
This fixes glitches seen in fbcon caused by typing something in
the terminal.
BSpec: 55229
Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/display/intel_psr.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index caf92f414a6e7..894a2d35668a2 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1487,8 +1487,8 @@ void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
enum pipe pipe = plane->pipe;
const struct drm_rect *clip;
- u32 val, offset;
- int ret, x, y;
+ u32 val;
+ int x, y;
if (!crtc_state->enable_psr2_sel_fetch)
return;
@@ -1508,10 +1508,6 @@ void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
/* TODO: consider auxiliary surfaces */
x = plane_state->uapi.src.x1 >> 16;
y = (plane_state->uapi.src.y1 >> 16) + clip->y1;
- ret = skl_calc_main_surface_offset(plane_state, &x, &y, &offset);
- if (ret)
- drm_warn_once(&dev_priv->drm, "skl_calc_main_surface_offset() returned %i\n",
- ret);
val = y << 16 | x;
intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane->id),
val);
--
2.32.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix sel fetch plane offset calculation
2021-07-31 0:10 ` [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix sel fetch plane offset calculation José Roberto de Souza
@ 2021-08-13 7:40 ` Gwan-gyeong Mun
0 siblings, 0 replies; 16+ messages in thread
From: Gwan-gyeong Mun @ 2021-08-13 7:40 UTC (permalink / raw)
To: José Roberto de Souza, intel-gfx; +Cc: Ville Syrjälä
Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
On 7/31/21 3:10 AM, José Roberto de Souza wrote:
> skl_calc_main_surface_offset() is used to calculate an aligned plane
> surface address considering the inner framebuffer x and y offset.
> It can not be used by selective fetch functions becase there is no
> PLANE_SEL_FETCH_SURF.
> So the PLANE_SEL_FETCH_OFFSET.y should only be PLANE_OFFSET.y +
> damaged_area_within_plane.y1.
>
> This fixes glitches seen in fbcon caused by typing something in
> the terminal.
>
> BSpec: 55229
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index caf92f414a6e7..894a2d35668a2 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1487,8 +1487,8 @@ void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> enum pipe pipe = plane->pipe;
> const struct drm_rect *clip;
> - u32 val, offset;
> - int ret, x, y;
> + u32 val;
> + int x, y;
>
> if (!crtc_state->enable_psr2_sel_fetch)
> return;
> @@ -1508,10 +1508,6 @@ void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> /* TODO: consider auxiliary surfaces */
> x = plane_state->uapi.src.x1 >> 16;
> y = (plane_state->uapi.src.y1 >> 16) + clip->y1;
> - ret = skl_calc_main_surface_offset(plane_state, &x, &y, &offset);
> - if (ret)
> - drm_warn_once(&dev_priv->drm, "skl_calc_main_surface_offset() returned %i\n",
> - ret);
> val = y << 16 | x;
> intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane->id),
> val);
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH 3/4] drm/i915: Nuke ORIGIN_GTT
2021-07-31 0:10 [Intel-gfx] [PATCH 1/4] drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications José Roberto de Souza
2021-07-31 0:10 ` [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix sel fetch plane offset calculation José Roberto de Souza
@ 2021-07-31 0:10 ` José Roberto de Souza
2021-08-03 11:20 ` Gwan-gyeong Mun
2021-07-31 0:10 ` [Intel-gfx] [PATCH 4/4] DO_NOT_MERGE: drm/i915/display: Enable PSR2 selective fetch by default José Roberto de Souza
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: José Roberto de Souza @ 2021-07-31 0:10 UTC (permalink / raw)
To: intel-gfx; +Cc: Gwan-gyeong Mun, José Roberto de Souza
There is no users of it, so no need to keep handling for it.
Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/display/intel_fbc.c | 10 +---------
drivers/gpu/drm/i915/display/intel_frontbuffer.h | 3 +--
2 files changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index ddfc17e21668a..e4d412d395c34 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -1129,7 +1129,7 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
if (!HAS_FBC(dev_priv))
return;
- if (origin == ORIGIN_GTT || origin == ORIGIN_FLIP)
+ if (origin == ORIGIN_FLIP)
return;
mutex_lock(&fbc->lock);
@@ -1150,14 +1150,6 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
if (!HAS_FBC(dev_priv))
return;
- /*
- * GTT tracking does not nuke the entire cfb
- * so don't clear busy_bits set for some other
- * reason.
- */
- if (origin == ORIGIN_GTT)
- return;
-
mutex_lock(&fbc->lock);
fbc->busy_bits &= ~frontbuffer_bits;
diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.h b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
index 6d41f53944250..4b977c1e4d52b 100644
--- a/drivers/gpu/drm/i915/display/intel_frontbuffer.h
+++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
@@ -33,8 +33,7 @@
struct drm_i915_private;
enum fb_op_origin {
- ORIGIN_GTT,
- ORIGIN_CPU,
+ ORIGIN_CPU = 0,
ORIGIN_CS,
ORIGIN_FLIP,
ORIGIN_DIRTYFB,
--
2.32.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH 3/4] drm/i915: Nuke ORIGIN_GTT
2021-07-31 0:10 ` [Intel-gfx] [PATCH 3/4] drm/i915: Nuke ORIGIN_GTT José Roberto de Souza
@ 2021-08-03 11:20 ` Gwan-gyeong Mun
2021-08-03 17:19 ` Souza, Jose
0 siblings, 1 reply; 16+ messages in thread
From: Gwan-gyeong Mun @ 2021-08-03 11:20 UTC (permalink / raw)
To: José Roberto de Souza, intel-gfx
On 7/31/21 3:10 AM, José Roberto de Souza wrote:
> There is no users of it, so no need to keep handling for it.
>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_fbc.c | 10 +---------
> drivers/gpu/drm/i915/display/intel_frontbuffer.h | 3 +--
> 2 files changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index ddfc17e21668a..e4d412d395c34 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -1129,7 +1129,7 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
> if (!HAS_FBC(dev_priv))
> return;
>
> - if (origin == ORIGIN_GTT || origin == ORIGIN_FLIP)
> + if (origin == ORIGIN_FLIP)
> return;
>
> mutex_lock(&fbc->lock);
> @@ -1150,14 +1150,6 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
> if (!HAS_FBC(dev_priv))
> return;
>
> - /*
> - * GTT tracking does not nuke the entire cfb
> - * so don't clear busy_bits set for some other
> - * reason.
> - */
> - if (origin == ORIGIN_GTT)
> - return;
> -
> mutex_lock(&fbc->lock);
>
> fbc->busy_bits &= ~frontbuffer_bits;
> diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.h b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
> index 6d41f53944250..4b977c1e4d52b 100644
> --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.h
> +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
> @@ -33,8 +33,7 @@
> struct drm_i915_private;
>
> enum fb_op_origin {
> - ORIGIN_GTT,
> - ORIGIN_CPU,
> + ORIGIN_CPU = 0,
> ORIGIN_CS,
> ORIGIN_FLIP,
> ORIGIN_DIRTYFB,
>
Is this patch absolutely necessary for this series?
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH 3/4] drm/i915: Nuke ORIGIN_GTT
2021-08-03 11:20 ` Gwan-gyeong Mun
@ 2021-08-03 17:19 ` Souza, Jose
2021-08-13 7:41 ` Gwan-gyeong Mun
0 siblings, 1 reply; 16+ messages in thread
From: Souza, Jose @ 2021-08-03 17:19 UTC (permalink / raw)
To: Mun, Gwan-gyeong, intel-gfx
On Tue, 2021-08-03 at 14:20 +0300, Gwan-gyeong Mun wrote:
>
> On 7/31/21 3:10 AM, José Roberto de Souza wrote:
> > There is no users of it, so no need to keep handling for it.
> >
> > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_fbc.c | 10 +---------
> > drivers/gpu/drm/i915/display/intel_frontbuffer.h | 3 +--
> > 2 files changed, 2 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index ddfc17e21668a..e4d412d395c34 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -1129,7 +1129,7 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
> > if (!HAS_FBC(dev_priv))
> > return;
> >
> > - if (origin == ORIGIN_GTT || origin == ORIGIN_FLIP)
> > + if (origin == ORIGIN_FLIP)
> > return;
> >
> > mutex_lock(&fbc->lock);
> > @@ -1150,14 +1150,6 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
> > if (!HAS_FBC(dev_priv))
> > return;
> >
> > - /*
> > - * GTT tracking does not nuke the entire cfb
> > - * so don't clear busy_bits set for some other
> > - * reason.
> > - */
> > - if (origin == ORIGIN_GTT)
> > - return;
> > -
> > mutex_lock(&fbc->lock);
> >
> > fbc->busy_bits &= ~frontbuffer_bits;
> > diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.h b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
> > index 6d41f53944250..4b977c1e4d52b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.h
> > +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
> > @@ -33,8 +33,7 @@
> > struct drm_i915_private;
> >
> > enum fb_op_origin {
> > - ORIGIN_GTT,
> > - ORIGIN_CPU,
> > + ORIGIN_CPU = 0,
> > ORIGIN_CS,
> > ORIGIN_FLIP,
> > ORIGIN_DIRTYFB,
> >
> Is this patch absolutely necessary for this series?
Nope, just notice this unused ORIGIN_GTT while debugging.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH 3/4] drm/i915: Nuke ORIGIN_GTT
2021-08-03 17:19 ` Souza, Jose
@ 2021-08-13 7:41 ` Gwan-gyeong Mun
0 siblings, 0 replies; 16+ messages in thread
From: Gwan-gyeong Mun @ 2021-08-13 7:41 UTC (permalink / raw)
To: Souza, Jose, intel-gfx
Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
On 8/3/21 8:19 PM, Souza, Jose wrote:
> On Tue, 2021-08-03 at 14:20 +0300, Gwan-gyeong Mun wrote:
>>
>> On 7/31/21 3:10 AM, José Roberto de Souza wrote:
>>> There is no users of it, so no need to keep handling for it.
>>>
>>> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>>> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/display/intel_fbc.c | 10 +---------
>>> drivers/gpu/drm/i915/display/intel_frontbuffer.h | 3 +--
>>> 2 files changed, 2 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
>>> index ddfc17e21668a..e4d412d395c34 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
>>> @@ -1129,7 +1129,7 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
>>> if (!HAS_FBC(dev_priv))
>>> return;
>>>
>>> -if (origin == ORIGIN_GTT || origin == ORIGIN_FLIP)
>>> +if (origin == ORIGIN_FLIP)
>>> return;
>>>
>>> mutex_lock(&fbc->lock);
>>> @@ -1150,14 +1150,6 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
>>> if (!HAS_FBC(dev_priv))
>>> return;
>>>
>>> -/*
>>> - * GTT tracking does not nuke the entire cfb
>>> - * so don't clear busy_bits set for some other
>>> - * reason.
>>> - */
>>> -if (origin == ORIGIN_GTT)
>>> -return;
>>> -
>>> mutex_lock(&fbc->lock);
>>>
>>> fbc->busy_bits &= ~frontbuffer_bits;
>>> diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.h b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
>>> index 6d41f53944250..4b977c1e4d52b 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
>>> @@ -33,8 +33,7 @@
>>> struct drm_i915_private;
>>>
>>> enum fb_op_origin {
>>> -ORIGIN_GTT,
>>> -ORIGIN_CPU,
>>> +ORIGIN_CPU = 0,
>>> ORIGIN_CS,
>>> ORIGIN_FLIP,
>>> ORIGIN_DIRTYFB,
>>>
>> Is this patch absolutely necessary for this series?
>
> Nope, just notice this unused ORIGIN_GTT while debugging.
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH 4/4] DO_NOT_MERGE: drm/i915/display: Enable PSR2 selective fetch by default
2021-07-31 0:10 [Intel-gfx] [PATCH 1/4] drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications José Roberto de Souza
2021-07-31 0:10 ` [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix sel fetch plane offset calculation José Roberto de Souza
2021-07-31 0:10 ` [Intel-gfx] [PATCH 3/4] drm/i915: Nuke ORIGIN_GTT José Roberto de Souza
@ 2021-07-31 0:10 ` José Roberto de Souza
2021-08-03 11:17 ` Gwan-gyeong Mun
2021-07-31 0:14 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications Patchwork
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: José Roberto de Souza @ 2021-07-31 0:10 UTC (permalink / raw)
To: intel-gfx; +Cc: José Roberto de Souza
Only to execute tests with PSR2 selective fetch enabled and check what
is broken.
IGT tests know to fail with this:
- kms_cursor_legacy: all tests that checks if evasion happend, I have
fix for it making cursor_slowpath() returns true for display 12+.
- kms_psr2_su: The pageflip test, it needs to have the damage clip set
otherwise it will update the whole screen and the selective blocks
will not match with expected.
- kms_psr: psr2_*_(mmap_gtt, mmap_cpu, blt and render), all those
tests should be dropped or skipped for display 12+.
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/display/intel_psr.c | 9 ---------
drivers/gpu/drm/i915/i915_params.h | 2 +-
2 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 894a2d35668a2..e128f0c2aeecc 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -877,15 +877,6 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
return false;
}
- /*
- * We are missing the implementation of some workarounds to enabled PSR2
- * in Alderlake_P, until ready PSR2 should be kept disabled.
- */
- if (IS_ALDERLAKE_P(dev_priv)) {
- drm_dbg_kms(&dev_priv->drm, "PSR2 is missing the implementation of workarounds\n");
- return false;
- }
-
if (!transcoder_has_psr2(dev_priv, crtc_state->cpu_transcoder)) {
drm_dbg_kms(&dev_priv->drm,
"PSR2 not supported in transcoder %s\n",
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index f27eceb82c0f5..8d725b64592d8 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -55,7 +55,7 @@ struct drm_printer;
param(int, enable_fbc, -1, 0600) \
param(int, enable_psr, -1, 0600) \
param(bool, psr_safest_params, false, 0400) \
- param(bool, enable_psr2_sel_fetch, false, 0400) \
+ param(bool, enable_psr2_sel_fetch, true, 0400) \
param(int, disable_power_well, -1, 0400) \
param(int, enable_ips, 1, 0600) \
param(int, invert_brightness, 0, 0600) \
--
2.32.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH 4/4] DO_NOT_MERGE: drm/i915/display: Enable PSR2 selective fetch by default
2021-07-31 0:10 ` [Intel-gfx] [PATCH 4/4] DO_NOT_MERGE: drm/i915/display: Enable PSR2 selective fetch by default José Roberto de Souza
@ 2021-08-03 11:17 ` Gwan-gyeong Mun
2021-08-03 17:18 ` Souza, Jose
0 siblings, 1 reply; 16+ messages in thread
From: Gwan-gyeong Mun @ 2021-08-03 11:17 UTC (permalink / raw)
To: José Roberto de Souza, intel-gfx
On 7/31/21 3:10 AM, José Roberto de Souza wrote:
> Only to execute tests with PSR2 selective fetch enabled and check what
> is broken.
>
> IGT tests know to fail with this:
> - kms_cursor_legacy: all tests that checks if evasion happend, I have
> fix for it making cursor_slowpath() returns true for display 12+.
>
> - kms_psr2_su: The pageflip test, it needs to have the damage clip set
> otherwise it will update the whole screen and the selective blocks
> will not match with expected.
>
kms_psr2_su is a test case for intel PSR2 HW tracking and kms_psr2_sf is
used as a test for intel PSR2 manual tracking. Is it necessary to modify
kms_psr2_su for testing PSR2 manual tracking?
> - kms_psr: psr2_*_(mmap_gtt, mmap_cpu, blt and render), all those
> tests should be dropped or skipped for display 12+.
>
Could you explain in more detail why we need to skip on display 12+?
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 9 ---------
> drivers/gpu/drm/i915/i915_params.h | 2 +-
> 2 files changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 894a2d35668a2..e128f0c2aeecc 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -877,15 +877,6 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> return false;
> }
>
> - /*
> - * We are missing the implementation of some workarounds to enabled PSR2
> - * in Alderlake_P, until ready PSR2 should be kept disabled.
> - */
> - if (IS_ALDERLAKE_P(dev_priv)) {
> - drm_dbg_kms(&dev_priv->drm, "PSR2 is missing the implementation of workarounds\n");
> - return false;
> - }
> -
> if (!transcoder_has_psr2(dev_priv, crtc_state->cpu_transcoder)) {
> drm_dbg_kms(&dev_priv->drm,
> "PSR2 not supported in transcoder %s\n",
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index f27eceb82c0f5..8d725b64592d8 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -55,7 +55,7 @@ struct drm_printer;
> param(int, enable_fbc, -1, 0600) \
> param(int, enable_psr, -1, 0600) \
> param(bool, psr_safest_params, false, 0400) \
> - param(bool, enable_psr2_sel_fetch, false, 0400) \
> + param(bool, enable_psr2_sel_fetch, true, 0400) \
> param(int, disable_power_well, -1, 0400) \
> param(int, enable_ips, 1, 0600) \
> param(int, invert_brightness, 0, 0600) \
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH 4/4] DO_NOT_MERGE: drm/i915/display: Enable PSR2 selective fetch by default
2021-08-03 11:17 ` Gwan-gyeong Mun
@ 2021-08-03 17:18 ` Souza, Jose
2021-08-05 18:26 ` Gwan-gyeong Mun
0 siblings, 1 reply; 16+ messages in thread
From: Souza, Jose @ 2021-08-03 17:18 UTC (permalink / raw)
To: Mun, Gwan-gyeong, intel-gfx
On Tue, 2021-08-03 at 14:17 +0300, Gwan-gyeong Mun wrote:
>
> On 7/31/21 3:10 AM, José Roberto de Souza wrote:
> > Only to execute tests with PSR2 selective fetch enabled and check what
> > is broken.
> >
> > IGT tests know to fail with this:
> > - kms_cursor_legacy: all tests that checks if evasion happend, I have
> > fix for it making cursor_slowpath() returns true for display 12+.
> >
> > - kms_psr2_su: The pageflip test, it needs to have the damage clip set
> > otherwise it will update the whole screen and the selective blocks
> > will not match with expected.
> >
> kms_psr2_su is a test case for intel PSR2 HW tracking and kms_psr2_sf is
> used as a test for intel PSR2 manual tracking. Is it necessary to modify
> kms_psr2_su for testing PSR2 manual tracking?
kms_psr2_su is to test that PSR2 is sending selective updates, just adding a couple of lines we can make it work with selective fetch.
> > - kms_psr: psr2_*_(mmap_gtt, mmap_cpu, blt and render), all those
> > tests should be dropped or skipped for display 12+.
> >
> Could you explain in more detail why we need to skip on display 12+?
This are stuff that would end up calling intel_psr_invalidate/flush().
>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_psr.c | 9 ---------
> > drivers/gpu/drm/i915/i915_params.h | 2 +-
> > 2 files changed, 1 insertion(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 894a2d35668a2..e128f0c2aeecc 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -877,15 +877,6 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> > return false;
> > }
> >
> > - /*
> > - * We are missing the implementation of some workarounds to enabled PSR2
> > - * in Alderlake_P, until ready PSR2 should be kept disabled.
> > - */
> > - if (IS_ALDERLAKE_P(dev_priv)) {
> > - drm_dbg_kms(&dev_priv->drm, "PSR2 is missing the implementation of workarounds\n");
> > - return false;
> > - }
> > -
> > if (!transcoder_has_psr2(dev_priv, crtc_state->cpu_transcoder)) {
> > drm_dbg_kms(&dev_priv->drm,
> > "PSR2 not supported in transcoder %s\n",
> > diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> > index f27eceb82c0f5..8d725b64592d8 100644
> > --- a/drivers/gpu/drm/i915/i915_params.h
> > +++ b/drivers/gpu/drm/i915/i915_params.h
> > @@ -55,7 +55,7 @@ struct drm_printer;
> > param(int, enable_fbc, -1, 0600) \
> > param(int, enable_psr, -1, 0600) \
> > param(bool, psr_safest_params, false, 0400) \
> > - param(bool, enable_psr2_sel_fetch, false, 0400) \
> > + param(bool, enable_psr2_sel_fetch, true, 0400) \
> > param(int, disable_power_well, -1, 0400) \
> > param(int, enable_ips, 1, 0600) \
> > param(int, invert_brightness, 0, 0600) \
> >
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH 4/4] DO_NOT_MERGE: drm/i915/display: Enable PSR2 selective fetch by default
2021-08-03 17:18 ` Souza, Jose
@ 2021-08-05 18:26 ` Gwan-gyeong Mun
2021-08-05 21:47 ` Souza, Jose
0 siblings, 1 reply; 16+ messages in thread
From: Gwan-gyeong Mun @ 2021-08-05 18:26 UTC (permalink / raw)
To: Souza, Jose, intel-gfx
On 8/3/21 8:18 PM, Souza, Jose wrote:
> On Tue, 2021-08-03 at 14:17 +0300, Gwan-gyeong Mun wrote:
>>
>> On 7/31/21 3:10 AM, José Roberto de Souza wrote:
>>> Only to execute tests with PSR2 selective fetch enabled and check what
>>> is broken.
>>>
>>> IGT tests know to fail with this:
>>> - kms_cursor_legacy: all tests that checks if evasion happend, I have
>>> fix for it making cursor_slowpath() returns true for display 12+.
>>>
>>> - kms_psr2_su: The pageflip test, it needs to have the damage clip set
>>> otherwise it will update the whole screen and the selective blocks
>>> will not match with expected.
>>>
>> kms_psr2_su is a test case for intel PSR2 HW tracking and kms_psr2_sf is
>> used as a test for intel PSR2 manual tracking. Is it necessary to modify
>> kms_psr2_su for testing PSR2 manual tracking?
>
> kms_psr2_su is to test that PSR2 is sending selective updates, just adding a couple of lines we can make it work with selective fetch.
>
>>> - kms_psr: psr2_*_(mmap_gtt, mmap_cpu, blt and render), all those
>>> tests should be dropped or skipped for display 12+.
>>>
>> Could you explain in more detail why we need to skip on display 12+?
>
> This are stuff that would end up calling intel_psr_invalidate/flush().
>
Thanks for the explanation.
And there is an issue confirmed in local tests, so I leave additional
comments.
>>
>>> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/display/intel_psr.c | 9 ---------
>>> drivers/gpu/drm/i915/i915_params.h | 2 +-
>>> 2 files changed, 1 insertion(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
>>> index 894a2d35668a2..e128f0c2aeecc 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_psr.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>>> @@ -877,15 +877,6 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
>>> return false;
>>> }
>>>
>>> -/*
>>> - * We are missing the implementation of some workarounds to enabled PSR2
>>> - * in Alderlake_P, until ready PSR2 should be kept disabled.
>>> - */
>>> -if (IS_ALDERLAKE_P(dev_priv)) {
>>> -drm_dbg_kms(&dev_priv->drm, "PSR2 is missing the implementation of workarounds\n");
>>> -return false;
>>> -}
>>> -
>>> if (!transcoder_has_psr2(dev_priv, crtc_state->cpu_transcoder)) {
>>> drm_dbg_kms(&dev_priv->drm,
>>> "PSR2 not supported in transcoder %s\n",
>>> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
>>> index f27eceb82c0f5..8d725b64592d8 100644
>>> --- a/drivers/gpu/drm/i915/i915_params.h
>>> +++ b/drivers/gpu/drm/i915/i915_params.h
>>> @@ -55,7 +55,7 @@ struct drm_printer;
>>> param(int, enable_fbc, -1, 0600) \
>>> param(int, enable_psr, -1, 0600) \
>>> param(bool, psr_safest_params, false, 0400) \
>>> -param(bool, enable_psr2_sel_fetch, false, 0400) \
>>> +param(bool, enable_psr2_sel_fetch, true, 0400) \
If we do not modify this part and do not enable it by default at boot
time as shown in the original code below,
param(bool, enable_psr2_sel_fetch, false, 0400) \
when we execute the kms_psr2_sf test case of igt, the FIFO underrun as
below still occurs.
i915 0000:00:02.0: [drm] *ERROR* CPU pipe A FIFO underrun: port,transcoder,
When PSR2 panel is used, PSR1 is enabled by default when
enable_psr2_sel_fetch is not enabled by default.
And when kms_psr2_sf is executed, the mode is changed to PSR2, and when
kms_psr2_sf is terminated, PSR2 is deactivated and PSR1 is re-enabled.
At this point. I suspect there is a problem.
>>> param(int, disable_power_well, -1, 0400) \
>>> param(int, enable_ips, 1, 0600) \
>>> param(int, invert_brightness, 0, 0600) \
>>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH 4/4] DO_NOT_MERGE: drm/i915/display: Enable PSR2 selective fetch by default
2021-08-05 18:26 ` Gwan-gyeong Mun
@ 2021-08-05 21:47 ` Souza, Jose
0 siblings, 0 replies; 16+ messages in thread
From: Souza, Jose @ 2021-08-05 21:47 UTC (permalink / raw)
To: Mun, Gwan-gyeong, intel-gfx
On Thu, 2021-08-05 at 21:26 +0300, Gwan-gyeong Mun wrote:
>
> On 8/3/21 8:18 PM, Souza, Jose wrote:
> > On Tue, 2021-08-03 at 14:17 +0300, Gwan-gyeong Mun wrote:
> > >
> > > On 7/31/21 3:10 AM, José Roberto de Souza wrote:
> > > > Only to execute tests with PSR2 selective fetch enabled and check what
> > > > is broken.
> > > >
> > > > IGT tests know to fail with this:
> > > > - kms_cursor_legacy: all tests that checks if evasion happend, I have
> > > > fix for it making cursor_slowpath() returns true for display 12+.
> > > >
> > > > - kms_psr2_su: The pageflip test, it needs to have the damage clip set
> > > > otherwise it will update the whole screen and the selective blocks
> > > > will not match with expected.
> > > >
> > > kms_psr2_su is a test case for intel PSR2 HW tracking and kms_psr2_sf is
> > > used as a test for intel PSR2 manual tracking. Is it necessary to modify
> > > kms_psr2_su for testing PSR2 manual tracking?
> >
> > kms_psr2_su is to test that PSR2 is sending selective updates, just adding a couple of lines we can make it work with selective fetch.
> >
> > > > - kms_psr: psr2_*_(mmap_gtt, mmap_cpu, blt and render), all those
> > > > tests should be dropped or skipped for display 12+.
> > > >
> > > Could you explain in more detail why we need to skip on display 12+?
> >
> > This are stuff that would end up calling intel_psr_invalidate/flush().
> >
>
> Thanks for the explanation.
> And there is an issue confirmed in local tests, so I leave additional
> comments.
> > >
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_psr.c | 9 ---------
> > > > drivers/gpu/drm/i915/i915_params.h | 2 +-
> > > > 2 files changed, 1 insertion(+), 10 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index 894a2d35668a2..e128f0c2aeecc 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -877,15 +877,6 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> > > > return false;
> > > > }
> > > >
> > > > -/*
> > > > - * We are missing the implementation of some workarounds to enabled PSR2
> > > > - * in Alderlake_P, until ready PSR2 should be kept disabled.
> > > > - */
> > > > -if (IS_ALDERLAKE_P(dev_priv)) {
> > > > -drm_dbg_kms(&dev_priv->drm, "PSR2 is missing the implementation of workarounds\n");
> > > > -return false;
> > > > -}
> > > > -
> > > > if (!transcoder_has_psr2(dev_priv, crtc_state->cpu_transcoder)) {
> > > > drm_dbg_kms(&dev_priv->drm,
> > > > "PSR2 not supported in transcoder %s\n",
> > > > diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> > > > index f27eceb82c0f5..8d725b64592d8 100644
> > > > --- a/drivers/gpu/drm/i915/i915_params.h
> > > > +++ b/drivers/gpu/drm/i915/i915_params.h
> > > > @@ -55,7 +55,7 @@ struct drm_printer;
> > > > param(int, enable_fbc, -1, 0600) \
> > > > param(int, enable_psr, -1, 0600) \
> > > > param(bool, psr_safest_params, false, 0400) \
> > > > -param(bool, enable_psr2_sel_fetch, false, 0400) \
> > > > +param(bool, enable_psr2_sel_fetch, true, 0400) \
> If we do not modify this part and do not enable it by default at boot
> time as shown in the original code below,
> param(bool, enable_psr2_sel_fetch, false, 0400) \
>
> when we execute the kms_psr2_sf test case of igt, the FIFO underrun as
> below still occurs.
>
> i915 0000:00:02.0: [drm] *ERROR* CPU pipe A FIFO underrun: port,transcoder,
>
> When PSR2 panel is used, PSR1 is enabled by default when
> enable_psr2_sel_fetch is not enabled by default.
> And when kms_psr2_sf is executed, the mode is changed to PSR2, and when
> kms_psr2_sf is terminated, PSR2 is deactivated and PSR1 is re-enabled.
> At this point. I suspect there is a problem.
Was able to reproduce this even with enable_psr2_sel_fetch set to true.
Added some debug messages to intel_psr_exit() and intel_psr_activate() and those functions are not called and the underrun still happens.
Could be a regression recently introduced because I was not seeing this underrun a few weeks ago.
Anyways this underrun happens with and without(just doing the changes to allow PSR2 in alderlake-P in intel_psr2_config_valid()) this patches.
>
> > > > param(int, disable_power_well, -1, 0400) \
> > > > param(int, enable_ips, 1, 0600) \
> > > > param(int, invert_brightness, 0, 0600) \
> > > >
> >
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications
2021-07-31 0:10 [Intel-gfx] [PATCH 1/4] drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications José Roberto de Souza
` (2 preceding siblings ...)
2021-07-31 0:10 ` [Intel-gfx] [PATCH 4/4] DO_NOT_MERGE: drm/i915/display: Enable PSR2 selective fetch by default José Roberto de Souza
@ 2021-07-31 0:14 ` Patchwork
2021-07-31 0:45 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2021-08-12 17:24 ` [Intel-gfx] [PATCH 1/4] " Souza, Jose
5 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2021-07-31 0:14 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/4] drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications
URL : https://patchwork.freedesktop.org/series/93252/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
762853c5a7c2 drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications
d75f9be16572 drm/i915/display: Fix sel fetch plane offset calculation
06de84df5401 drm/i915: Nuke ORIGIN_GTT
87fee2709b10 DO_NOT_MERGE: drm/i915/display: Enable PSR2 selective fetch by default
-:14: WARNING:TYPO_SPELLING: 'happend' may be misspelled - perhaps 'happened'?
#14:
- kms_cursor_legacy: all tests that checks if evasion happend, I have
^^^^^^^
total: 0 errors, 1 warnings, 0 checks, 23 lines checked
^ permalink raw reply [flat|nested] 16+ messages in thread* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications
2021-07-31 0:10 [Intel-gfx] [PATCH 1/4] drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications José Roberto de Souza
` (3 preceding siblings ...)
2021-07-31 0:14 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications Patchwork
@ 2021-07-31 0:45 ` Patchwork
2021-08-12 17:24 ` [Intel-gfx] [PATCH 1/4] " Souza, Jose
5 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2021-07-31 0:45 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 5125 bytes --]
== Series Details ==
Series: series starting with [1/4] drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications
URL : https://patchwork.freedesktop.org/series/93252/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_10427 -> Patchwork_20753
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_20753 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_20753, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20753/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_20753:
### IGT changes ###
#### Possible regressions ####
* igt@kms_cursor_legacy@basic-flip-before-cursor-atomic:
- fi-tgl-1115g4: [PASS][1] -> [FAIL][2] +3 similar issues
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10427/fi-tgl-1115g4/igt@kms_cursor_legacy@basic-flip-before-cursor-atomic.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20753/fi-tgl-1115g4/igt@kms_cursor_legacy@basic-flip-before-cursor-atomic.html
* igt@kms_cursor_legacy@basic-flip-before-cursor-legacy:
- fi-tgl-u2: [PASS][3] -> [FAIL][4] +3 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10427/fi-tgl-u2/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20753/fi-tgl-u2/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@kms_cursor_legacy@basic-flip-before-cursor-atomic:
- {fi-tgl-dsi}: [PASS][5] -> [FAIL][6] +3 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10427/fi-tgl-dsi/igt@kms_cursor_legacy@basic-flip-before-cursor-atomic.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20753/fi-tgl-dsi/igt@kms_cursor_legacy@basic-flip-before-cursor-atomic.html
Known issues
------------
Here are the changes found in Patchwork_20753 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@amdgpu/amd_cs_nop@sync-fork-compute0:
- fi-kbl-soraka: NOTRUN -> [SKIP][7] ([fdo#109271]) +10 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20753/fi-kbl-soraka/igt@amdgpu/amd_cs_nop@sync-fork-compute0.html
* igt@i915_selftest@live@execlists:
- fi-icl-y: [PASS][8] -> [DMESG-FAIL][9] ([i915#1993])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10427/fi-icl-y/igt@i915_selftest@live@execlists.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20753/fi-icl-y/igt@i915_selftest@live@execlists.html
#### Possible fixes ####
* igt@gem_exec_suspend@basic-s3:
- fi-tgl-1115g4: [FAIL][10] ([i915#1888]) -> [PASS][11]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10427/fi-tgl-1115g4/igt@gem_exec_suspend@basic-s3.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20753/fi-tgl-1115g4/igt@gem_exec_suspend@basic-s3.html
* igt@i915_selftest@live@workarounds:
- fi-tgl-u2: [DMESG-WARN][12] ([i915#2867]) -> [PASS][13] +16 similar issues
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10427/fi-tgl-u2/igt@i915_selftest@live@workarounds.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20753/fi-tgl-u2/igt@i915_selftest@live@workarounds.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
[i915#1993]: https://gitlab.freedesktop.org/drm/intel/issues/1993
[i915#2867]: https://gitlab.freedesktop.org/drm/intel/issues/2867
Participating hosts (37 -> 33)
------------------------------
Missing (4): fi-bdw-samus fi-bsw-cyan bat-jsl-1 fi-hsw-4200u
Build changes
-------------
* Linux: CI_DRM_10427 -> Patchwork_20753
CI-20190529: 20190529
CI_DRM_10427: e5efc20bce6b7996df9bdc720796f8713a45701a @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_6159: 6135b9cc319ed965e3aafb5b2ae2abf4762a06b2 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_20753: 87fee2709b10c39c5cd3c558f356f6821d01df03 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
87fee2709b10 DO_NOT_MERGE: drm/i915/display: Enable PSR2 selective fetch by default
06de84df5401 drm/i915: Nuke ORIGIN_GTT
d75f9be16572 drm/i915/display: Fix sel fetch plane offset calculation
762853c5a7c2 drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20753/index.html
[-- Attachment #2: Type: text/html, Size: 5958 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH 1/4] drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications
2021-07-31 0:10 [Intel-gfx] [PATCH 1/4] drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications José Roberto de Souza
` (4 preceding siblings ...)
2021-07-31 0:45 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
@ 2021-08-12 17:24 ` Souza, Jose
2021-08-12 19:02 ` Daniel Vetter
5 siblings, 1 reply; 16+ messages in thread
From: Souza, Jose @ 2021-08-12 17:24 UTC (permalink / raw)
To: Vivi, Rodrigo, Nikula, Jani, intel-gfx
Cc: daniel, Mun, Gwan-gyeong, ville.syrjala
On Fri, 2021-07-30 at 17:10 -0700, José Roberto de Souza wrote:
> PSR2 selective fetch requires plane and transcoder registers to
> be programed during the vblank to properly update the display and
> there is no way around it.
>
> We could disable PSR2 at every notification of dirty front buffer from
> user space but that would hurt the power savings and it would still
> cause some race conditions between PSR2 exit sequence and atomic
> commits that causes underruns and glitches.
>
> So from display 12 and newer we will start to do atomic commits
> every time user space notify that front buffer is dirty and ignore
> all frontbuffer flushes and invalidates on the PSR side.
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_cursor.c | 3 ++-
> drivers/gpu/drm/i915/display/intel_display.c | 7 ++++++-
> drivers/gpu/drm/i915/display/intel_psr.c | 6 ++++++
> 3 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
> index c7618fef01439..d44022cb46a65 100644
> --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> @@ -617,6 +617,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
> u32 src_w, u32 src_h,
> struct drm_modeset_acquire_ctx *ctx)
> {
> + struct drm_i915_private *i915 = to_i915(_crtc->dev);
> struct intel_plane *plane = to_intel_plane(_plane);
> struct intel_crtc *crtc = to_intel_crtc(_crtc);
> struct intel_plane_state *old_plane_state =
> @@ -638,7 +639,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
> */
> if (!crtc_state->hw.active || intel_crtc_needs_modeset(crtc_state) ||
> crtc_state->update_pipe || crtc_state->bigjoiner ||
> - crtc_state->enable_psr2_sel_fetch)
> + DISPLAY_VER(i915) >= 12)
> goto slow;
>
> /*
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 5ff0a011b28eb..4a936e1e7fa82 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -11720,10 +11720,15 @@ static int intel_user_framebuffer_dirty(struct drm_framebuffer *fb,
> unsigned num_clips)
> {
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> + struct drm_i915_private *i915 = to_i915(obj->base.dev);
>
> i915_gem_object_flush_if_display(obj);
> - intel_frontbuffer_flush(to_intel_frontbuffer(fb), ORIGIN_DIRTYFB);
>
> + if (DISPLAY_VER(i915) >= 12)
> + return drm_atomic_helper_dirtyfb(fb, file, flags, color, clips,
> + num_clips);
> +
> + intel_frontbuffer_flush(to_intel_frontbuffer(fb), ORIGIN_DIRTYFB);
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 1b0daf649e823..caf92f414a6e7 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2039,6 +2039,9 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
> {
> struct intel_encoder *encoder;
>
> + if (DISPLAY_VER(dev_priv) >= 12)
> + return;
> +
> if (origin == ORIGIN_FLIP)
> return;
>
> @@ -2123,6 +2126,9 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
> continue;
> }
>
> + if (DISPLAY_VER(dev_priv) >= 12)
> + continue;
> +
> mutex_lock(&intel_dp->psr.lock);
> if (!intel_dp->psr.enabled) {
> mutex_unlock(&intel_dp->psr.lock);
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH 1/4] drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications
2021-08-12 17:24 ` [Intel-gfx] [PATCH 1/4] " Souza, Jose
@ 2021-08-12 19:02 ` Daniel Vetter
0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2021-08-12 19:02 UTC (permalink / raw)
To: Souza, Jose
Cc: Vivi, Rodrigo, Nikula, Jani, intel-gfx, Mun, Gwan-gyeong, ville.syrjala
On Thu, Aug 12, 2021 at 7:24 PM Souza, Jose <jose.souza@intel.com> wrote:
>
> On Fri, 2021-07-30 at 17:10 -0700, José Roberto de Souza wrote:
> > PSR2 selective fetch requires plane and transcoder registers to
> > be programed during the vblank to properly update the display and
> > there is no way around it.
> >
> > We could disable PSR2 at every notification of dirty front buffer from
> > user space but that would hurt the power savings and it would still
> > cause some race conditions between PSR2 exit sequence and atomic
> > commits that causes underruns and glitches.
> >
> > So from display 12 and newer we will start to do atomic commits
> > every time user space notify that front buffer is dirty and ignore
> > all frontbuffer flushes and invalidates on the PSR side.
So I filed a JIRA about this a while ago, and:
- This should be done on gen9 (or maybe gen10+), because we stopped
having userspace that was busted. Would be good to check whether we
can't push this down further even.
- We need to disable the entire intel_frontbuffer.c interface/uapi for
_all_ things. It's a horrendous uapi, and it needs to die.
You're already touching intel_user_framebuffer_dirty, so the other
pieces really should be done too. Maybe make a macro or function or
something so that we have a consistent check for which platforms still
have to support frontbuffer tracking.
-Daniel
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
>
> >
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_cursor.c | 3 ++-
> > drivers/gpu/drm/i915/display/intel_display.c | 7 ++++++-
> > drivers/gpu/drm/i915/display/intel_psr.c | 6 ++++++
> > 3 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
> > index c7618fef01439..d44022cb46a65 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> > @@ -617,6 +617,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
> > u32 src_w, u32 src_h,
> > struct drm_modeset_acquire_ctx *ctx)
> > {
> > + struct drm_i915_private *i915 = to_i915(_crtc->dev);
> > struct intel_plane *plane = to_intel_plane(_plane);
> > struct intel_crtc *crtc = to_intel_crtc(_crtc);
> > struct intel_plane_state *old_plane_state =
> > @@ -638,7 +639,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
> > */
> > if (!crtc_state->hw.active || intel_crtc_needs_modeset(crtc_state) ||
> > crtc_state->update_pipe || crtc_state->bigjoiner ||
> > - crtc_state->enable_psr2_sel_fetch)
> > + DISPLAY_VER(i915) >= 12)
> > goto slow;
> >
> > /*
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 5ff0a011b28eb..4a936e1e7fa82 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -11720,10 +11720,15 @@ static int intel_user_framebuffer_dirty(struct drm_framebuffer *fb,
> > unsigned num_clips)
> > {
> > struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> > + struct drm_i915_private *i915 = to_i915(obj->base.dev);
> >
> > i915_gem_object_flush_if_display(obj);
> > - intel_frontbuffer_flush(to_intel_frontbuffer(fb), ORIGIN_DIRTYFB);
> >
> > + if (DISPLAY_VER(i915) >= 12)
> > + return drm_atomic_helper_dirtyfb(fb, file, flags, color, clips,
> > + num_clips);
> > +
> > + intel_frontbuffer_flush(to_intel_frontbuffer(fb), ORIGIN_DIRTYFB);
> > return 0;
> > }
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 1b0daf649e823..caf92f414a6e7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -2039,6 +2039,9 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
> > {
> > struct intel_encoder *encoder;
> >
> > + if (DISPLAY_VER(dev_priv) >= 12)
> > + return;
> > +
> > if (origin == ORIGIN_FLIP)
> > return;
> >
> > @@ -2123,6 +2126,9 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
> > continue;
> > }
> >
> > + if (DISPLAY_VER(dev_priv) >= 12)
> > + continue;
> > +
> > mutex_lock(&intel_dp->psr.lock);
> > if (!intel_dp->psr.enabled) {
> > mutex_unlock(&intel_dp->psr.lock);
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 16+ messages in thread