* [Intel-gfx] [PATCH 1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch
@ 2020-09-01 1:09 José Roberto de Souza
2020-09-01 1:09 ` [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix state of PSR2 sub features José Roberto de Souza
` (5 more replies)
0 siblings, 6 replies; 19+ messages in thread
From: José Roberto de Souza @ 2020-09-01 1:09 UTC (permalink / raw)
To: intel-gfx
For platforms without selective fetch this register is reserved so
do not write 0 to it.
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 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 8a9d0bdde1bf..4e09ae61d4aa 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -942,7 +942,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
intel_de_write(dev_priv, EXITLINE(cpu_transcoder), val);
}
- if (HAS_PSR_HW_TRACKING(dev_priv))
+ if (HAS_PSR_HW_TRACKING(dev_priv) && HAS_PSR2_SEL_FETCH(dev_priv))
intel_de_rmw(dev_priv, CHICKEN_PAR1_1, IGNORE_PSR2_HW_TRACKING,
dev_priv->psr.psr2_sel_fetch_enabled ?
IGNORE_PSR2_HW_TRACKING : 0);
--
2.28.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix state of PSR2 sub features
2020-09-01 1:09 [Intel-gfx] [PATCH 1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch José Roberto de Souza
@ 2020-09-01 1:09 ` José Roberto de Souza
2020-09-14 14:24 ` Ville Syrjälä
2020-09-01 1:09 ` [Intel-gfx] [PATCH 3/4] drm/i915/display: Program PSR2 selective fetch registers José Roberto de Souza
` (4 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: José Roberto de Souza @ 2020-09-01 1:09 UTC (permalink / raw)
To: intel-gfx
In case PSR2 is disabled by debugfs dc3co_enabled and
psr2_sel_fetch_enabled were still being set causing some code paths
to be executed were it should not.
We have tests for PSR1 and PSR2 so keep those features disabled when
PSR1 is active but PSR2 is supported is important.
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 | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 4e09ae61d4aa..6698d0209879 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -962,12 +962,14 @@ static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state);
dev_priv->psr.busy_frontbuffer_bits = 0;
dev_priv->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
- dev_priv->psr.dc3co_enabled = !!crtc_state->dc3co_exitline;
+ dev_priv->psr.dc3co_enabled = !!crtc_state->dc3co_exitline &&
+ dev_priv->psr.psr2_enabled;
dev_priv->psr.transcoder = crtc_state->cpu_transcoder;
/* DC5/DC6 requires at least 6 idle frames */
val = usecs_to_jiffies(intel_get_frame_time_us(crtc_state) * 6);
dev_priv->psr.dc3co_exit_delay = val;
- dev_priv->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch;
+ dev_priv->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch &&
+ dev_priv->psr.psr2_enabled;
/*
* If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
@@ -1178,7 +1180,7 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
struct i915_psr *psr = &dev_priv->psr;
if (!HAS_PSR2_SEL_FETCH(dev_priv) ||
- !crtc_state->enable_psr2_sel_fetch)
+ !dev_priv->psr.psr2_sel_fetch_enabled)
return;
intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(psr->transcoder),
@@ -1189,8 +1191,9 @@ void intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
struct intel_crtc *crtc)
{
struct intel_crtc_state *crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
- if (!crtc_state->enable_psr2_sel_fetch)
+ if (!dev_priv->psr.psr2_sel_fetch_enabled)
return;
crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE |
--
2.28.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Intel-gfx] [PATCH 3/4] drm/i915/display: Program PSR2 selective fetch registers
2020-09-01 1:09 [Intel-gfx] [PATCH 1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch José Roberto de Souza
2020-09-01 1:09 ` [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix state of PSR2 sub features José Roberto de Souza
@ 2020-09-01 1:09 ` José Roberto de Souza
2020-09-14 14:28 ` Ville Syrjälä
2020-09-01 1:09 ` [Intel-gfx] [PATCH 4/4] HAX/DO_NOT_MERGE_IT: drm/i915/display: Enable PSR2 selective fetch for testing José Roberto de Souza
` (3 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: José Roberto de Souza @ 2020-09-01 1:09 UTC (permalink / raw)
To: intel-gfx
Another step towards PSR2 selective fetch, here programming plane
selective fetch registers and MAN_TRK_CTL enabling selective fetch but
for now it is fetching the whole area of the planes.
The damaged area calculation will come as next and final step.
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_display.c | 10 +-
.../drm/i915/display/intel_display_types.h | 2 +
drivers/gpu/drm/i915/display/intel_psr.c | 129 +++++++++++++++++-
drivers/gpu/drm/i915/display/intel_psr.h | 10 +-
drivers/gpu/drm/i915/display/intel_sprite.c | 3 +
5 files changed, 145 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index c8b1dd1a9e46..865486e89915 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -11799,6 +11799,9 @@ static void i9xx_update_cursor(struct intel_plane *plane,
if (INTEL_GEN(dev_priv) >= 9)
skl_write_cursor_wm(plane, crtc_state);
+ if (!needs_modeset(crtc_state))
+ intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, 0);
+
if (plane->cursor.base != base ||
plane->cursor.size != fbc_ctl ||
plane->cursor.cntl != cntl) {
@@ -12810,8 +12813,11 @@ static int intel_crtc_atomic_check(struct intel_atomic_state *state,
}
- if (!mode_changed)
- intel_psr2_sel_fetch_update(state, crtc);
+ if (!mode_changed) {
+ ret = intel_psr2_sel_fetch_update(state, crtc);
+ if (ret)
+ return ret;
+ }
return 0;
}
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 9349b15afff6..2138bb0f1587 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -586,6 +586,8 @@ struct intel_plane_state {
u32 planar_slave;
struct drm_intel_sprite_colorkey ckey;
+
+ struct drm_rect psr2_sel_fetch_area;
};
struct intel_initial_plane_config {
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 6698d0209879..b60ea133a527 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1173,6 +1173,46 @@ static void psr_force_hw_tracking_exit(struct drm_i915_private *dev_priv)
intel_psr_exit(dev_priv);
}
+void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
+ const struct intel_crtc_state *crtc_state,
+ const struct intel_plane_state *plane_state,
+ int color_plane)
+{
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+ const struct drm_rect *clip;
+ enum pipe pipe = plane->pipe;
+ u32 val;
+
+ if (!plane_state || !dev_priv->psr.psr2_sel_fetch_enabled)
+ return;
+
+ /*
+ * skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0 for gen11+, so
+ * plane_state->ctl is the right value
+ */
+ intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id), plane_state->ctl);
+ if (!plane_state->ctl || plane->id == PLANE_CURSOR)
+ return;
+
+ clip = &plane_state->psr2_sel_fetch_area;
+
+ val = (clip->y1 + plane_state->uapi.crtc_y) << 16;
+ val |= plane_state->uapi.crtc_x;
+ intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane->id),
+ val);
+
+ val = (clip->y1 + plane_state->color_plane[color_plane].y) << 16;
+ val |= plane_state->color_plane[color_plane].x;
+ intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane->id),
+ val);
+
+ /* Sizes are 0 based */
+ val = (drm_rect_height(clip) - 1) << 16;
+ val |= (drm_rect_width(&plane_state->uapi.src) >> 16) - 1;
+ intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, plane->id),
+ val);
+}
+
void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_state)
{
struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
@@ -1187,17 +1227,96 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
crtc_state->psr2_man_track_ctl);
}
-void intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
- struct intel_crtc *crtc)
+static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state,
+ struct drm_rect *clip, bool full_update)
+{
+ u32 val = PSR2_MAN_TRK_CTL_ENABLE;
+
+ if (full_update) {
+ val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
+ goto exit;
+ }
+
+ drm_WARN_ON_ONCE(crtc_state->uapi.crtc->dev, clip->y1 == -1);
+
+ val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
+ val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
+ val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip->y2, 4) + 1);
+exit:
+ crtc_state->psr2_man_track_ctl = val;
+}
+
+static void clip_area_update(struct drm_rect *overlap_damage_area,
+ struct drm_rect *damage_area)
+{
+ if (overlap_damage_area->y1 == -1) {
+ overlap_damage_area->y1 = damage_area->y1;
+ overlap_damage_area->y2 = damage_area->y2;
+ return;
+ }
+
+ if (damage_area->y1 < overlap_damage_area->y1)
+ overlap_damage_area->y1 = damage_area->y1;
+
+ if (damage_area->y2 > overlap_damage_area->y2)
+ overlap_damage_area->y2 = damage_area->y2;
+}
+
+int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
{
struct intel_crtc_state *crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ struct intel_plane_state *new_plane_state, *old_plane_state;
+ struct drm_rect pipe_clip = { .y1 = -1 };
+ struct intel_plane *plane;
+ bool full_update = false;
+ int i, ret;
if (!dev_priv->psr.psr2_sel_fetch_enabled)
- return;
+ return 0;
+
+ ret = drm_atomic_add_affected_planes(&state->base, &crtc->base);
+ if (ret)
+ return ret;
+
+ for_each_oldnew_intel_plane_in_state(state, plane, old_plane_state,
+ new_plane_state, i) {
+ struct drm_rect *plane_sel_fetch_area, temp;
- crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE |
- PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
+ if (new_plane_state->uapi.crtc != crtc_state->uapi.crtc)
+ continue;
+
+ /*
+ * TODO: Not clear how to handle planes with negative position,
+ * also planes are not updated if they have a negative X
+ * position so for now doing a full update in this cases
+ */
+ if (new_plane_state->uapi.crtc_y < 0 ||
+ new_plane_state->uapi.crtc_x < 0) {
+ full_update = true;
+ break;
+ }
+
+ if (!new_plane_state->uapi.visible)
+ continue;
+
+ /*
+ * For now doing a selective fetch in the whole plane area,
+ * optimizations will come in the future.
+ */
+ plane_sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
+ plane_sel_fetch_area->y1 = new_plane_state->uapi.src.y1 >> 16;
+ plane_sel_fetch_area->y2 = new_plane_state->uapi.src.y2 >> 16;
+
+ temp = *plane_sel_fetch_area;
+ temp.y1 += new_plane_state->uapi.crtc_y;
+ temp.y2 += new_plane_state->uapi.crtc_y;
+ clip_area_update(&pipe_clip, &temp);
+ }
+
+ psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, full_update);
+ return 0;
}
/**
diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h
index 6a83c8e682e6..3eca9dcec3c0 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.h
+++ b/drivers/gpu/drm/i915/display/intel_psr.h
@@ -15,6 +15,8 @@ struct intel_crtc_state;
struct intel_dp;
struct intel_crtc;
struct intel_atomic_state;
+struct intel_plane_state;
+struct intel_plane;
#define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv->psr.sink_support)
void intel_psr_init_dpcd(struct intel_dp *intel_dp);
@@ -45,8 +47,12 @@ void intel_psr_atomic_check(struct drm_connector *connector,
struct drm_connector_state *old_state,
struct drm_connector_state *new_state);
void intel_psr_set_force_mode_changed(struct intel_dp *intel_dp);
-void intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
- struct intel_crtc *crtc);
+int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
+ struct intel_crtc *crtc);
void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_state);
+void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
+ const struct intel_crtc_state *crtc_state,
+ const struct intel_plane_state *plane_state,
+ int color_plane);
#endif /* __INTEL_PSR_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index 1797a06cfd60..24ee9b08ec4a 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -690,6 +690,9 @@ skl_program_plane(struct intel_plane *plane,
intel_de_write_fw(dev_priv, PLANE_AUX_OFFSET(pipe, plane_id),
(plane_state->color_plane[1].y << 16) | plane_state->color_plane[1].x);
+ if (!drm_atomic_crtc_needs_modeset(&crtc_state->uapi))
+ intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, color_plane);
+
/*
* The control register self-arms if the plane was previously
* disabled. Try to make the plane enable atomic by writing
--
2.28.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Intel-gfx] [PATCH 4/4] HAX/DO_NOT_MERGE_IT: drm/i915/display: Enable PSR2 selective fetch for testing
2020-09-01 1:09 [Intel-gfx] [PATCH 1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch José Roberto de Souza
2020-09-01 1:09 ` [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix state of PSR2 sub features José Roberto de Souza
2020-09-01 1:09 ` [Intel-gfx] [PATCH 3/4] drm/i915/display: Program PSR2 selective fetch registers José Roberto de Souza
@ 2020-09-01 1:09 ` José Roberto de Souza
2020-09-01 1:46 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch Patchwork
` (2 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: José Roberto de Souza @ 2020-09-01 1:09 UTC (permalink / raw)
To: intel-gfx
Enabling it to check if it causes regressions in CI but the feature is
still not ready to be enabled by default.
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/i915_params.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 330c03e2b4f7..b8b19270c339 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -54,7 +54,7 @@ struct drm_printer;
param(int, enable_fbc, -1, 0600) \
param(int, enable_psr, -1, 0600) \
param(bool, psr_safest_params, false, 0600) \
- param(bool, enable_psr2_sel_fetch, false, 0600) \
+ param(bool, enable_psr2_sel_fetch, true, 0600) \
param(int, disable_power_well, -1, 0400) \
param(int, enable_ips, 1, 0600) \
param(int, invert_brightness, 0, 0600) \
--
2.28.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch
2020-09-01 1:09 [Intel-gfx] [PATCH 1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch José Roberto de Souza
` (2 preceding siblings ...)
2020-09-01 1:09 ` [Intel-gfx] [PATCH 4/4] HAX/DO_NOT_MERGE_IT: drm/i915/display: Enable PSR2 selective fetch for testing José Roberto de Souza
@ 2020-09-01 1:46 ` Patchwork
2020-09-01 9:59 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2020-09-01 19:30 ` [Intel-gfx] [PATCH 1/4] " Rodrigo Vivi
5 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2020-09-01 1:46 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 5380 bytes --]
== Series Details ==
Series: series starting with [1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch
URL : https://patchwork.freedesktop.org/series/81201/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_8948 -> Patchwork_18426
====================================================
Summary
-------
**WARNING**
Minor unknown changes coming with Patchwork_18426 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_18426, 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_18426/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_18426:
### IGT changes ###
#### Warnings ####
* igt@runner@aborted:
- fi-tgl-u2: [FAIL][1] ([i915#2045]) -> [FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/fi-tgl-u2/igt@runner@aborted.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/fi-tgl-u2/igt@runner@aborted.html
Known issues
------------
Here are the changes found in Patchwork_18426 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_parallel@engines@contexts:
- fi-cfl-8109u: [PASS][3] -> [INCOMPLETE][4] ([i915#2398])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/fi-cfl-8109u/igt@gem_exec_parallel@engines@contexts.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/fi-cfl-8109u/igt@gem_exec_parallel@engines@contexts.html
- fi-icl-y: [PASS][5] -> [INCOMPLETE][6] ([i915#2398])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/fi-icl-y/igt@gem_exec_parallel@engines@contexts.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/fi-icl-y/igt@gem_exec_parallel@engines@contexts.html
* igt@i915_selftest@live@execlists:
- fi-icl-y: [PASS][7] -> [INCOMPLETE][8] ([i915#2276])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/fi-icl-y/igt@i915_selftest@live@execlists.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/fi-icl-y/igt@i915_selftest@live@execlists.html
* igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1:
- fi-icl-u2: [PASS][9] -> [DMESG-WARN][10] ([i915#1982]) +1 similar issue
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1.html
#### Possible fixes ####
* igt@gem_exec_store@basic:
- fi-apl-guc: [TIMEOUT][11] ([i915#1635]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/fi-apl-guc/igt@gem_exec_store@basic.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/fi-apl-guc/igt@gem_exec_store@basic.html
* igt@i915_pm_rpm@module-reload:
- {fi-kbl-7560u}: [WARN][13] ([i915#2249]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/fi-kbl-7560u/igt@i915_pm_rpm@module-reload.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/fi-kbl-7560u/igt@i915_pm_rpm@module-reload.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
- fi-byt-j1900: [DMESG-WARN][15] ([i915#1982]) -> [PASS][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/fi-byt-j1900/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/fi-byt-j1900/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2045]: https://gitlab.freedesktop.org/drm/intel/issues/2045
[i915#2249]: https://gitlab.freedesktop.org/drm/intel/issues/2249
[i915#2276]: https://gitlab.freedesktop.org/drm/intel/issues/2276
[i915#2398]: https://gitlab.freedesktop.org/drm/intel/issues/2398
Participating hosts (38 -> 33)
------------------------------
Missing (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper
Build changes
-------------
* Linux: CI_DRM_8948 -> Patchwork_18426
CI-20190529: 20190529
CI_DRM_8948: 4a551ee5aa0f402a647f797437b69af12ce78642 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5774: 2a5db9f60241383272aeec176e1b97b3f487209f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_18426: 8316e2705e8f88f7a0b59150fe3ce4bb99edc616 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
8316e2705e8f HAX/DO_NOT_MERGE_IT: drm/i915/display: Enable PSR2 selective fetch for testing
0cd1cad6e012 drm/i915/display: Program PSR2 selective fetch registers
394059cffb10 drm/i915/display: Fix state of PSR2 sub features
2335d5eca219 drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/index.html
[-- Attachment #1.2: Type: text/html, Size: 6343 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch
2020-09-01 1:09 [Intel-gfx] [PATCH 1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch José Roberto de Souza
` (3 preceding siblings ...)
2020-09-01 1:46 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch Patchwork
@ 2020-09-01 9:59 ` Patchwork
2020-09-01 16:49 ` Souza, Jose
2020-09-01 19:30 ` [Intel-gfx] [PATCH 1/4] " Rodrigo Vivi
5 siblings, 1 reply; 19+ messages in thread
From: Patchwork @ 2020-09-01 9:59 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 16480 bytes --]
== Series Details ==
Series: series starting with [1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch
URL : https://patchwork.freedesktop.org/series/81201/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_8948_full -> Patchwork_18426_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_18426_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_18426_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_18426_full:
### IGT changes ###
#### Possible regressions ####
* igt@runner@aborted:
- shard-tglb: NOTRUN -> ([FAIL][1], [FAIL][2], [FAIL][3], [FAIL][4], [FAIL][5], [FAIL][6], [FAIL][7], [FAIL][8], [FAIL][9], [FAIL][10], [FAIL][11], [FAIL][12], [FAIL][13], [FAIL][14], [FAIL][15], [FAIL][16], [FAIL][17], [FAIL][18], [FAIL][19], [FAIL][20], [FAIL][21], [FAIL][22], [FAIL][23], [FAIL][24], [FAIL][25])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb8/igt@runner@aborted.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb7/igt@runner@aborted.html
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb1/igt@runner@aborted.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb5/igt@runner@aborted.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb7/igt@runner@aborted.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb3/igt@runner@aborted.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb8/igt@runner@aborted.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb1/igt@runner@aborted.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb2/igt@runner@aborted.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb2/igt@runner@aborted.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb5/igt@runner@aborted.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb2/igt@runner@aborted.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb1/igt@runner@aborted.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb5/igt@runner@aborted.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb3/igt@runner@aborted.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb1/igt@runner@aborted.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb2/igt@runner@aborted.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb7/igt@runner@aborted.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb3/igt@runner@aborted.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb3/igt@runner@aborted.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb5/igt@runner@aborted.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb5/igt@runner@aborted.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb8/igt@runner@aborted.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb7/igt@runner@aborted.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-tglb1/igt@runner@aborted.html
Known issues
------------
Here are the changes found in Patchwork_18426_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_whisper@basic-contexts-all:
- shard-glk: [PASS][26] -> [TIMEOUT][27] ([i915#1958]) +2 similar issues
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-glk5/igt@gem_exec_whisper@basic-contexts-all.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-glk2/igt@gem_exec_whisper@basic-contexts-all.html
* igt@gem_partial_pwrite_pread@reads-display:
- shard-glk: [PASS][28] -> [FAIL][29] ([i915#2261]) +1 similar issue
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-glk7/igt@gem_partial_pwrite_pread@reads-display.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-glk3/igt@gem_partial_pwrite_pread@reads-display.html
* igt@gem_sync@basic-store-all:
- shard-glk: [PASS][30] -> [FAIL][31] ([i915#2356])
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-glk8/igt@gem_sync@basic-store-all.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-glk4/igt@gem_sync@basic-store-all.html
* igt@i915_selftest@mock@contexts:
- shard-apl: [PASS][32] -> [INCOMPLETE][33] ([i915#1635] / [i915#2278])
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-apl1/igt@i915_selftest@mock@contexts.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-apl6/igt@i915_selftest@mock@contexts.html
* igt@kms_big_fb@linear-64bpp-rotate-180:
- shard-glk: [PASS][34] -> [DMESG-FAIL][35] ([i915#118] / [i915#95])
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-glk6/igt@kms_big_fb@linear-64bpp-rotate-180.html
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-glk8/igt@kms_big_fb@linear-64bpp-rotate-180.html
* igt@kms_color@pipe-c-ctm-0-25:
- shard-skl: [PASS][36] -> [DMESG-WARN][37] ([i915#1982]) +5 similar issues
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-skl7/igt@kms_color@pipe-c-ctm-0-25.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-skl2/igt@kms_color@pipe-c-ctm-0-25.html
* igt@kms_flip@plain-flip-ts-check@a-dp1:
- shard-kbl: [PASS][38] -> [DMESG-WARN][39] ([i915#1982]) +1 similar issue
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-kbl2/igt@kms_flip@plain-flip-ts-check@a-dp1.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-kbl1/igt@kms_flip@plain-flip-ts-check@a-dp1.html
* igt@kms_frontbuffer_tracking@fbc-suspend:
- shard-kbl: [PASS][40] -> [DMESG-WARN][41] ([i915#180]) +3 similar issues
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-kbl2/igt@kms_frontbuffer_tracking@fbc-suspend.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-suspend.html
* igt@kms_hdr@bpc-switch-dpms:
- shard-skl: [PASS][42] -> [FAIL][43] ([i915#1188]) +1 similar issue
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-skl6/igt@kms_hdr@bpc-switch-dpms.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-skl7/igt@kms_hdr@bpc-switch-dpms.html
* igt@kms_plane@plane-panning-bottom-right-pipe-a-planes:
- shard-iclb: [PASS][44] -> [FAIL][45] ([i915#1036])
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-iclb4/igt@kms_plane@plane-panning-bottom-right-pipe-a-planes.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-iclb7/igt@kms_plane@plane-panning-bottom-right-pipe-a-planes.html
* igt@kms_plane_cursor@pipe-b-primary-size-64:
- shard-apl: [PASS][46] -> [DMESG-WARN][47] ([i915#1635] / [i915#1982])
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-apl6/igt@kms_plane_cursor@pipe-b-primary-size-64.html
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-apl2/igt@kms_plane_cursor@pipe-b-primary-size-64.html
* igt@kms_psr@psr2_sprite_mmap_gtt:
- shard-iclb: [PASS][48] -> [SKIP][49] ([fdo#109441]) +1 similar issue
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-iclb5/igt@kms_psr@psr2_sprite_mmap_gtt.html
* igt@kms_setmode@basic:
- shard-kbl: [PASS][50] -> [FAIL][51] ([i915#31])
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-kbl1/igt@kms_setmode@basic.html
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-kbl7/igt@kms_setmode@basic.html
#### Possible fixes ####
* igt@gem_exec_gttfill@basic:
- shard-glk: [DMESG-WARN][52] ([i915#118] / [i915#95]) -> [PASS][53]
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-glk4/igt@gem_exec_gttfill@basic.html
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-glk1/igt@gem_exec_gttfill@basic.html
* igt@gem_exec_reloc@basic-concurrent0:
- shard-skl: [TIMEOUT][54] ([i915#1958]) -> [PASS][55]
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-skl6/igt@gem_exec_reloc@basic-concurrent0.html
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-skl5/igt@gem_exec_reloc@basic-concurrent0.html
* igt@gem_exec_whisper@basic-fds-forked-all:
- shard-kbl: [TIMEOUT][56] ([i915#1958]) -> [PASS][57]
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-kbl1/igt@gem_exec_whisper@basic-fds-forked-all.html
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-kbl7/igt@gem_exec_whisper@basic-fds-forked-all.html
* igt@gem_exec_whisper@basic-forked:
- shard-glk: [TIMEOUT][58] ([i915#1958]) -> [PASS][59]
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-glk5/igt@gem_exec_whisper@basic-forked.html
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-glk9/igt@gem_exec_whisper@basic-forked.html
* igt@gem_exec_whisper@basic-queues-forked:
- shard-skl: [DMESG-WARN][60] ([i915#1982]) -> [PASS][61] +8 similar issues
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-skl4/igt@gem_exec_whisper@basic-queues-forked.html
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-skl3/igt@gem_exec_whisper@basic-queues-forked.html
* igt@i915_pm_dc@dc6-psr:
- shard-skl: [FAIL][62] ([i915#454]) -> [PASS][63]
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-skl4/igt@i915_pm_dc@dc6-psr.html
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-skl9/igt@i915_pm_dc@dc6-psr.html
* igt@kms_draw_crc@draw-method-rgb565-blt-xtiled:
- shard-apl: [DMESG-WARN][64] ([i915#1635] / [i915#1982]) -> [PASS][65] +1 similar issue
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-apl7/igt@kms_draw_crc@draw-method-rgb565-blt-xtiled.html
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-apl2/igt@kms_draw_crc@draw-method-rgb565-blt-xtiled.html
* igt@kms_plane@plane-panning-bottom-right-pipe-a-planes:
- shard-glk: [FAIL][66] ([i915#1036]) -> [PASS][67]
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-glk3/igt@kms_plane@plane-panning-bottom-right-pipe-a-planes.html
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-glk6/igt@kms_plane@plane-panning-bottom-right-pipe-a-planes.html
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
- shard-kbl: [DMESG-WARN][68] ([i915#180]) -> [PASS][69] +2 similar issues
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-kbl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-kbl7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
- shard-skl: [INCOMPLETE][70] ([i915#198]) -> [PASS][71]
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-skl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-skl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
* igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
- shard-skl: [FAIL][72] ([fdo#108145] / [i915#265]) -> [PASS][73]
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-skl9/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-skl9/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
* igt@kms_plane_scaling@pipe-c-scaler-with-clipping-clamping:
- shard-iclb: [DMESG-WARN][74] ([i915#1982]) -> [PASS][75]
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-iclb3/igt@kms_plane_scaling@pipe-c-scaler-with-clipping-clamping.html
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-iclb4/igt@kms_plane_scaling@pipe-c-scaler-with-clipping-clamping.html
* igt@kms_psr@psr2_cursor_plane_onoff:
- shard-iclb: [SKIP][76] ([fdo#109441]) -> [PASS][77] +1 similar issue
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-iclb6/igt@kms_psr@psr2_cursor_plane_onoff.html
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html
#### Warnings ####
* igt@kms_flip@flip-vs-expired-vblank@a-edp1:
- shard-skl: [DMESG-WARN][78] ([i915#1982]) -> [DMESG-FAIL][79] ([i915#1982] / [i915#79])
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-skl10/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-skl10/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html
* igt@runner@aborted:
- shard-skl: [FAIL][80] ([i915#1436]) -> ([FAIL][81], [FAIL][82]) ([i915#1436] / [i915#1814] / [i915#2029])
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8948/shard-skl5/igt@runner@aborted.html
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-skl5/igt@runner@aborted.html
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/shard-skl4/igt@runner@aborted.html
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[i915#1036]: https://gitlab.freedesktop.org/drm/intel/issues/1036
[i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
[i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
[i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
[i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#1814]: https://gitlab.freedesktop.org/drm/intel/issues/1814
[i915#1958]: https://gitlab.freedesktop.org/drm/intel/issues/1958
[i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2029]: https://gitlab.freedesktop.org/drm/intel/issues/2029
[i915#2261]: https://gitlab.freedesktop.org/drm/intel/issues/2261
[i915#2278]: https://gitlab.freedesktop.org/drm/intel/issues/2278
[i915#2356]: https://gitlab.freedesktop.org/drm/intel/issues/2356
[i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
[i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
[i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
[i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
[i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
Participating hosts (10 -> 10)
------------------------------
No changes in participating hosts
Build changes
-------------
* Linux: CI_DRM_8948 -> Patchwork_18426
CI-20190529: 20190529
CI_DRM_8948: 4a551ee5aa0f402a647f797437b69af12ce78642 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5774: 2a5db9f60241383272aeec176e1b97b3f487209f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_18426: 8316e2705e8f88f7a0b59150fe3ce4bb99edc616 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/index.html
[-- Attachment #1.2: Type: text/html, Size: 19213 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch
2020-09-01 9:59 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
@ 2020-09-01 16:49 ` Souza, Jose
0 siblings, 0 replies; 19+ messages in thread
From: Souza, Jose @ 2020-09-01 16:49 UTC (permalink / raw)
To: intel-gfx
On Tue, 2020-09-01 at 09:59 +0000, Patchwork wrote:
> Patch Details
> Series: series starting with [1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch
> URL: https://patchwork.freedesktop.org/series/81201/
> State: failure
> Details: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18426/index.html
> CI Bug Log - changes from CI_DRM_8948_full -> Patchwork_18426_full
> Summary
> FAILURE
>
> Serious unknown changes coming with Patchwork_18426_full absolutely need to be
> verified manually.
>
> If you think the reported changes have nothing to do with the changes
> introduced in Patchwork_18426_full, please notify your bug team to allow them
> to document this new failure mode, which will reduce false positives in CI.
>
> Possible new issues
> Here are the unknown changes that may have been introduced in Patchwork_18426_full:
>
> IGT changes
> Possible regressions
> igt@runner@aborted:
> shard-tglb: NOTRUN -> (FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL)
Okay, so it is a valid a state that have CRTC enabled with all planes not visible.Will fix it for next version.
> Known issues
> Here are the changes found in Patchwork_18426_full that come from known issues:
>
> IGT changes
> Issues hit
> igt@gem_exec_whisper@basic-contexts-all:
>
> shard-glk: PASS -> TIMEOUT (i915#1958) +2 similar issues
> igt@gem_partial_pwrite_pread@reads-display:
>
> shard-glk: PASS -> FAIL (i915#2261) +1 similar issue
> igt@gem_sync@basic-store-all:
>
> shard-glk: PASS -> FAIL (i915#2356)
> igt@i915_selftest@mock@contexts:
>
> shard-apl: PASS -> INCOMPLETE (i915#1635 / i915#2278)
> igt@kms_big_fb@linear-64bpp-rotate-180:
>
> shard-glk: PASS -> DMESG-FAIL (i915#118 / i915#95)
> igt@kms_color@pipe-c-ctm-0-25:
>
> shard-skl: PASS -> DMESG-WARN (i915#1982) +5 similar issues
> igt@kms_flip@plain-flip-ts-check@a-dp1:
>
> shard-kbl: PASS -> DMESG-WARN (i915#1982) +1 similar issue
> igt@kms_frontbuffer_tracking@fbc-suspend:
>
> shard-kbl: PASS -> DMESG-WARN (i915#180) +3 similar issues
> igt@kms_hdr@bpc-switch-dpms:
>
> shard-skl: PASS -> FAIL (i915#1188) +1 similar issue
> igt@kms_plane@plane-panning-bottom-right-pipe-a-planes:
>
> shard-iclb: PASS -> FAIL (i915#1036)
> igt@kms_plane_cursor@pipe-b-primary-size-64:
>
> shard-apl: PASS -> DMESG-WARN (i915#1635 / i915#1982)
> igt@kms_psr@psr2_sprite_mmap_gtt:
>
> shard-iclb: PASS -> SKIP (fdo#109441) +1 similar issue
> igt@kms_setmode@basic:
>
> shard-kbl: PASS -> FAIL (i915#31)
> Possible fixes
> igt@gem_exec_gttfill@basic:
>
> shard-glk: DMESG-WARN (i915#118 / i915#95) -> PASS
> igt@gem_exec_reloc@basic-concurrent0:
>
> shard-skl: TIMEOUT (i915#1958) -> PASS
> igt@gem_exec_whisper@basic-fds-forked-all:
>
> shard-kbl: TIMEOUT (i915#1958) -> PASS
> igt@gem_exec_whisper@basic-forked:
>
> shard-glk: TIMEOUT (i915#1958) -> PASS
> igt@gem_exec_whisper@basic-queues-forked:
>
> shard-skl: DMESG-WARN (i915#1982) -> PASS +8 similar issues
> igt@i915_pm_dc@dc6-psr:
>
> shard-skl: FAIL (i915#454) -> PASS
> igt@kms_draw_crc@draw-method-rgb565-blt-xtiled:
>
> shard-apl: DMESG-WARN (i915#1635 / i915#1982) -> PASS +1 similar issue
> igt@kms_plane@plane-panning-bottom-right-pipe-a-planes:
>
> shard-glk: FAIL (i915#1036) -> PASS
> igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
>
> shard-kbl: DMESG-WARN (i915#180) -> PASS +2 similar issues
> igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
>
> shard-skl: INCOMPLETE (i915#198) -> PASS
> igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
>
> shard-skl: FAIL (fdo#108145 / i915#265) -> PASS
> igt@kms_plane_scaling@pipe-c-scaler-with-clipping-clamping:
>
> shard-iclb: DMESG-WARN (i915#1982) -> PASS
> igt@kms_psr@psr2_cursor_plane_onoff:
>
> shard-iclb: SKIP (fdo#109441) -> PASS +1 similar issue
> Warnings
> igt@kms_flip@flip-vs-expired-vblank@a-edp1:
>
> shard-skl: DMESG-WARN (i915#1982) -> DMESG-FAIL (i915#1982 / i915#79)
> igt@runner@aborted:
>
> shard-skl: FAIL (i915#1436) -> (FAIL, FAIL) (i915#1436 / i915#1814 / i915#2029)
> Participating hosts (10 -> 10)
> No changes in participating hosts
>
> Build changes
> Linux: CI_DRM_8948 -> Patchwork_18426
> CI-20190529: 20190529
> CI_DRM_8948: 4a551ee5aa0f402a647f797437b69af12ce78642 @ git://anongit.freedesktop.org/gfx-ci/linux
> IGT_5774: 2a5db9f60241383272aeec176e1b97b3f487209f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
> Patchwork_18426: 8316e2705e8f88f7a0b59150fe3ce4bb99edc616 @ git://anongit.freedesktop.org/gfx-ci/linux
> piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch
2020-09-01 1:09 [Intel-gfx] [PATCH 1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch José Roberto de Souza
` (4 preceding siblings ...)
2020-09-01 9:59 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
@ 2020-09-01 19:30 ` Rodrigo Vivi
5 siblings, 0 replies; 19+ messages in thread
From: Rodrigo Vivi @ 2020-09-01 19:30 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
On Mon, Aug 31, 2020 at 06:09:21PM -0700, José Roberto de Souza wrote:
> For platforms without selective fetch this register is reserved so
> do not write 0 to it.
>
> 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>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 8a9d0bdde1bf..4e09ae61d4aa 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -942,7 +942,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
> intel_de_write(dev_priv, EXITLINE(cpu_transcoder), val);
> }
>
> - if (HAS_PSR_HW_TRACKING(dev_priv))
> + if (HAS_PSR_HW_TRACKING(dev_priv) && HAS_PSR2_SEL_FETCH(dev_priv))
> intel_de_rmw(dev_priv, CHICKEN_PAR1_1, IGNORE_PSR2_HW_TRACKING,
> dev_priv->psr.psr2_sel_fetch_enabled ?
> IGNORE_PSR2_HW_TRACKING : 0);
> --
> 2.28.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix state of PSR2 sub features
2020-09-01 1:09 ` [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix state of PSR2 sub features José Roberto de Souza
@ 2020-09-14 14:24 ` Ville Syrjälä
2020-09-14 19:57 ` Souza, Jose
0 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjälä @ 2020-09-14 14:24 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
On Mon, Aug 31, 2020 at 06:09:22PM -0700, José Roberto de Souza wrote:
> In case PSR2 is disabled by debugfs dc3co_enabled and
> psr2_sel_fetch_enabled were still being set causing some code paths
> to be executed were it should not.
> We have tests for PSR1 and PSR2 so keep those features disabled when
> PSR1 is active but PSR2 is supported is important.
>
> 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 | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 4e09ae61d4aa..6698d0209879 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -962,12 +962,14 @@ static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
> dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state);
> dev_priv->psr.busy_frontbuffer_bits = 0;
> dev_priv->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
> - dev_priv->psr.dc3co_enabled = !!crtc_state->dc3co_exitline;
> + dev_priv->psr.dc3co_enabled = !!crtc_state->dc3co_exitline &&
> + dev_priv->psr.psr2_enabled;
> dev_priv->psr.transcoder = crtc_state->cpu_transcoder;
> /* DC5/DC6 requires at least 6 idle frames */
> val = usecs_to_jiffies(intel_get_frame_time_us(crtc_state) * 6);
> dev_priv->psr.dc3co_exit_delay = val;
> - dev_priv->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch;
> + dev_priv->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch &&
> + dev_priv->psr.psr2_enabled;
>
> /*
> * If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
> @@ -1178,7 +1180,7 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
> struct i915_psr *psr = &dev_priv->psr;
>
> if (!HAS_PSR2_SEL_FETCH(dev_priv) ||
> - !crtc_state->enable_psr2_sel_fetch)
> + !dev_priv->psr.psr2_sel_fetch_enabled)
> return;
>
> intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(psr->transcoder),
> @@ -1189,8 +1191,9 @@ void intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> struct intel_crtc *crtc)
> {
> struct intel_crtc_state *crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>
> - if (!crtc_state->enable_psr2_sel_fetch)
> + if (!dev_priv->psr.psr2_sel_fetch_enabled)
This looks rather sketchy. AFAICS this gets called during atomic_check()
so looking at stuff outside the crtc state is very suspicious.
> return;
>
> crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE |
> --
> 2.28.0
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915/display: Program PSR2 selective fetch registers
2020-09-01 1:09 ` [Intel-gfx] [PATCH 3/4] drm/i915/display: Program PSR2 selective fetch registers José Roberto de Souza
@ 2020-09-14 14:28 ` Ville Syrjälä
2020-09-14 20:15 ` Souza, Jose
0 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjälä @ 2020-09-14 14:28 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
On Mon, Aug 31, 2020 at 06:09:23PM -0700, José Roberto de Souza wrote:
> Another step towards PSR2 selective fetch, here programming plane
> selective fetch registers and MAN_TRK_CTL enabling selective fetch but
> for now it is fetching the whole area of the planes.
> The damaged area calculation will come as next and final step.
>
> 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_display.c | 10 +-
> .../drm/i915/display/intel_display_types.h | 2 +
> drivers/gpu/drm/i915/display/intel_psr.c | 129 +++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_psr.h | 10 +-
> drivers/gpu/drm/i915/display/intel_sprite.c | 3 +
> 5 files changed, 145 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index c8b1dd1a9e46..865486e89915 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -11799,6 +11799,9 @@ static void i9xx_update_cursor(struct intel_plane *plane,
> if (INTEL_GEN(dev_priv) >= 9)
> skl_write_cursor_wm(plane, crtc_state);
>
> + if (!needs_modeset(crtc_state))
> + intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, 0);
> +
> if (plane->cursor.base != base ||
> plane->cursor.size != fbc_ctl ||
> plane->cursor.cntl != cntl) {
> @@ -12810,8 +12813,11 @@ static int intel_crtc_atomic_check(struct intel_atomic_state *state,
>
> }
>
> - if (!mode_changed)
> - intel_psr2_sel_fetch_update(state, crtc);
> + if (!mode_changed) {
> + ret = intel_psr2_sel_fetch_update(state, crtc);
> + if (ret)
> + return ret;
> + }
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 9349b15afff6..2138bb0f1587 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -586,6 +586,8 @@ struct intel_plane_state {
> u32 planar_slave;
>
> struct drm_intel_sprite_colorkey ckey;
> +
> + struct drm_rect psr2_sel_fetch_area;
> };
>
> struct intel_initial_plane_config {
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 6698d0209879..b60ea133a527 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1173,6 +1173,46 @@ static void psr_force_hw_tracking_exit(struct drm_i915_private *dev_priv)
> intel_psr_exit(dev_priv);
> }
>
> +void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> + const struct intel_crtc_state *crtc_state,
> + const struct intel_plane_state *plane_state,
> + int color_plane)
> +{
> + struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> + const struct drm_rect *clip;
> + enum pipe pipe = plane->pipe;
> + u32 val;
> +
> + if (!plane_state || !dev_priv->psr.psr2_sel_fetch_enabled)
> + return;
> +
> + /*
> + * skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0 for gen11+, so
> + * plane_state->ctl is the right value
> + */
> + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id), plane_state->ctl);
> + if (!plane_state->ctl || plane->id == PLANE_CURSOR)
> + return;
> +
> + clip = &plane_state->psr2_sel_fetch_area;
> +
> + val = (clip->y1 + plane_state->uapi.crtc_y) << 16;
crtc_x/y are the raw values usrspace gave us. That is definitely not
what we should be looking at.
As the first step I think these functions should just program the
registers with *exactly* the same values as we program into the
normal plane register. That gets us to the point where we're
actually programming something into the register without having to
complicate things with calculating the selective fetch area.
> + val |= plane_state->uapi.crtc_x;
> + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane->id),
> + val);
> +
> + val = (clip->y1 + plane_state->color_plane[color_plane].y) << 16;
> + val |= plane_state->color_plane[color_plane].x;
> + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane->id),
> + val);
> +
> + /* Sizes are 0 based */
> + val = (drm_rect_height(clip) - 1) << 16;
> + val |= (drm_rect_width(&plane_state->uapi.src) >> 16) - 1;
> + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, plane->id),
> + val);
> +}
> +
> void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> @@ -1187,17 +1227,96 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
> crtc_state->psr2_man_track_ctl);
> }
>
> -void intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> - struct intel_crtc *crtc)
> +static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state,
> + struct drm_rect *clip, bool full_update)
> +{
> + u32 val = PSR2_MAN_TRK_CTL_ENABLE;
> +
> + if (full_update) {
> + val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> + goto exit;
> + }
> +
> + drm_WARN_ON_ONCE(crtc_state->uapi.crtc->dev, clip->y1 == -1);
> +
> + val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
> + val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
> + val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip->y2, 4) + 1);
> +exit:
> + crtc_state->psr2_man_track_ctl = val;
> +}
> +
> +static void clip_area_update(struct drm_rect *overlap_damage_area,
> + struct drm_rect *damage_area)
> +{
> + if (overlap_damage_area->y1 == -1) {
> + overlap_damage_area->y1 = damage_area->y1;
> + overlap_damage_area->y2 = damage_area->y2;
> + return;
> + }
> +
> + if (damage_area->y1 < overlap_damage_area->y1)
> + overlap_damage_area->y1 = damage_area->y1;
> +
> + if (damage_area->y2 > overlap_damage_area->y2)
> + overlap_damage_area->y2 = damage_area->y2;
> +}
> +
> +int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> + struct intel_crtc *crtc)
> {
> struct intel_crtc_state *crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + struct intel_plane_state *new_plane_state, *old_plane_state;
> + struct drm_rect pipe_clip = { .y1 = -1 };
> + struct intel_plane *plane;
> + bool full_update = false;
> + int i, ret;
>
> if (!dev_priv->psr.psr2_sel_fetch_enabled)
> - return;
> + return 0;
> +
> + ret = drm_atomic_add_affected_planes(&state->base, &crtc->base);
> + if (ret)
> + return ret;
> +
> + for_each_oldnew_intel_plane_in_state(state, plane, old_plane_state,
> + new_plane_state, i) {
> + struct drm_rect *plane_sel_fetch_area, temp;
>
> - crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE |
> - PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> + if (new_plane_state->uapi.crtc != crtc_state->uapi.crtc)
> + continue;
> +
> + /*
> + * TODO: Not clear how to handle planes with negative position,
> + * also planes are not updated if they have a negative X
> + * position so for now doing a full update in this cases
> + */
> + if (new_plane_state->uapi.crtc_y < 0 ||
> + new_plane_state->uapi.crtc_x < 0) {
> + full_update = true;
> + break;
> + }
> +
> + if (!new_plane_state->uapi.visible)
> + continue;
> +
> + /*
> + * For now doing a selective fetch in the whole plane area,
> + * optimizations will come in the future.
> + */
> + plane_sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
> + plane_sel_fetch_area->y1 = new_plane_state->uapi.src.y1 >> 16;
> + plane_sel_fetch_area->y2 = new_plane_state->uapi.src.y2 >> 16;
> +
> + temp = *plane_sel_fetch_area;
> + temp.y1 += new_plane_state->uapi.crtc_y;
> + temp.y2 += new_plane_state->uapi.crtc_y;
> + clip_area_update(&pipe_clip, &temp);
> + }
> +
> + psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, full_update);
> + return 0;
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h
> index 6a83c8e682e6..3eca9dcec3c0 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> @@ -15,6 +15,8 @@ struct intel_crtc_state;
> struct intel_dp;
> struct intel_crtc;
> struct intel_atomic_state;
> +struct intel_plane_state;
> +struct intel_plane;
>
> #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv->psr.sink_support)
> void intel_psr_init_dpcd(struct intel_dp *intel_dp);
> @@ -45,8 +47,12 @@ void intel_psr_atomic_check(struct drm_connector *connector,
> struct drm_connector_state *old_state,
> struct drm_connector_state *new_state);
> void intel_psr_set_force_mode_changed(struct intel_dp *intel_dp);
> -void intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> - struct intel_crtc *crtc);
> +int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> + struct intel_crtc *crtc);
> void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_state);
> +void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> + const struct intel_crtc_state *crtc_state,
> + const struct intel_plane_state *plane_state,
> + int color_plane);
>
> #endif /* __INTEL_PSR_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index 1797a06cfd60..24ee9b08ec4a 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -690,6 +690,9 @@ skl_program_plane(struct intel_plane *plane,
> intel_de_write_fw(dev_priv, PLANE_AUX_OFFSET(pipe, plane_id),
> (plane_state->color_plane[1].y << 16) | plane_state->color_plane[1].x);
>
> + if (!drm_atomic_crtc_needs_modeset(&crtc_state->uapi))
> + intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, color_plane);
> +
> /*
> * The control register self-arms if the plane was previously
> * disabled. Try to make the plane enable atomic by writing
> --
> 2.28.0
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix state of PSR2 sub features
2020-09-14 14:24 ` Ville Syrjälä
@ 2020-09-14 19:57 ` Souza, Jose
2020-09-14 20:30 ` Ville Syrjälä
0 siblings, 1 reply; 19+ messages in thread
From: Souza, Jose @ 2020-09-14 19:57 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Mon, 2020-09-14 at 17:24 +0300, Ville Syrjälä wrote:
> On Mon, Aug 31, 2020 at 06:09:22PM -0700, José Roberto de Souza wrote:
> > In case PSR2 is disabled by debugfs dc3co_enabled and
> > psr2_sel_fetch_enabled were still being set causing some code paths
> > to be executed were it should not.
> > We have tests for PSR1 and PSR2 so keep those features disabled when
> > PSR1 is active but PSR2 is supported is important.
> >
> > 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 | 11 +++++++----
> > 1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 4e09ae61d4aa..6698d0209879 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -962,12 +962,14 @@ static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
> > dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state);
> > dev_priv->psr.busy_frontbuffer_bits = 0;
> > dev_priv->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
> > - dev_priv->psr.dc3co_enabled = !!crtc_state->dc3co_exitline;
> > + dev_priv->psr.dc3co_enabled = !!crtc_state->dc3co_exitline &&
> > + dev_priv->psr.psr2_enabled;
> > dev_priv->psr.transcoder = crtc_state->cpu_transcoder;
> > /* DC5/DC6 requires at least 6 idle frames */
> > val = usecs_to_jiffies(intel_get_frame_time_us(crtc_state) * 6);
> > dev_priv->psr.dc3co_exit_delay = val;
> > - dev_priv->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch;
> > + dev_priv->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch &&
> > + dev_priv->psr.psr2_enabled;
> >
> > /*
> > * If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
> > @@ -1178,7 +1180,7 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
> > struct i915_psr *psr = &dev_priv->psr;
> >
> > if (!HAS_PSR2_SEL_FETCH(dev_priv) ||
> > - !crtc_state->enable_psr2_sel_fetch)
> > + !dev_priv->psr.psr2_sel_fetch_enabled)
> > return;
> >
> > intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(psr->transcoder),
> > @@ -1189,8 +1191,9 @@ void intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> > struct intel_crtc *crtc)
> > {
> > struct intel_crtc_state *crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
> > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >
> > - if (!crtc_state->enable_psr2_sel_fetch)
> > + if (!dev_priv->psr.psr2_sel_fetch_enabled)
>
> This looks rather sketchy. AFAICS this gets called during atomic_check()
> so looking at stuff outside the crtc state is very suspicious.
This is called after the functions that change the PSR state so no issues, also we can't really on information in CRTC state, as PSR is only enabled
if supported by state, i915 PSR parameter and PSR debug fs value.
>
> > return;
> >
> > crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE |
> > --
> > 2.28.0
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915/display: Program PSR2 selective fetch registers
2020-09-14 14:28 ` Ville Syrjälä
@ 2020-09-14 20:15 ` Souza, Jose
2020-09-15 19:28 ` Mun, Gwan-gyeong
0 siblings, 1 reply; 19+ messages in thread
From: Souza, Jose @ 2020-09-14 20:15 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Mon, 2020-09-14 at 17:28 +0300, Ville Syrjälä wrote:
> On Mon, Aug 31, 2020 at 06:09:23PM -0700, José Roberto de Souza wrote:
> > Another step towards PSR2 selective fetch, here programming plane
> > selective fetch registers and MAN_TRK_CTL enabling selective fetch but
> > for now it is fetching the whole area of the planes.
> > The damaged area calculation will come as next and final step.
> >
> > 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_display.c | 10 +-
> > .../drm/i915/display/intel_display_types.h | 2 +
> > drivers/gpu/drm/i915/display/intel_psr.c | 129 +++++++++++++++++-
> > drivers/gpu/drm/i915/display/intel_psr.h | 10 +-
> > drivers/gpu/drm/i915/display/intel_sprite.c | 3 +
> > 5 files changed, 145 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index c8b1dd1a9e46..865486e89915 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -11799,6 +11799,9 @@ static void i9xx_update_cursor(struct intel_plane *plane,
> > if (INTEL_GEN(dev_priv) >= 9)
> > skl_write_cursor_wm(plane, crtc_state);
> >
> > + if (!needs_modeset(crtc_state))
> > + intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, 0);
> > +
> > if (plane->cursor.base != base ||
> > plane->cursor.size != fbc_ctl ||
> > plane->cursor.cntl != cntl) {
> > @@ -12810,8 +12813,11 @@ static int intel_crtc_atomic_check(struct intel_atomic_state *state,
> >
> > }
> >
> > - if (!mode_changed)
> > - intel_psr2_sel_fetch_update(state, crtc);
> > + if (!mode_changed) {
> > + ret = intel_psr2_sel_fetch_update(state, crtc);
> > + if (ret)
> > + return ret;
> > + }
> >
> > return 0;
> > }
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 9349b15afff6..2138bb0f1587 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -586,6 +586,8 @@ struct intel_plane_state {
> > u32 planar_slave;
> >
> > struct drm_intel_sprite_colorkey ckey;
> > +
> > + struct drm_rect psr2_sel_fetch_area;
> > };
> >
> > struct intel_initial_plane_config {
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 6698d0209879..b60ea133a527 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1173,6 +1173,46 @@ static void psr_force_hw_tracking_exit(struct drm_i915_private *dev_priv)
> > intel_psr_exit(dev_priv);
> > }
> >
> > +void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> > + const struct intel_crtc_state *crtc_state,
> > + const struct intel_plane_state *plane_state,
> > + int color_plane)
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > + const struct drm_rect *clip;
> > + enum pipe pipe = plane->pipe;
> > + u32 val;
> > +
> > + if (!plane_state || !dev_priv->psr.psr2_sel_fetch_enabled)
> > + return;
> > +
> > + /*
> > + * skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0 for gen11+, so
> > + * plane_state->ctl is the right value
> > + */
> > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id), plane_state->ctl);
> > + if (!plane_state->ctl || plane->id == PLANE_CURSOR)
> > + return;
> > +
> > + clip = &plane_state->psr2_sel_fetch_area;
> > +
> > + val = (clip->y1 + plane_state->uapi.crtc_y) << 16;
>
> crtc_x/y are the raw values usrspace gave us. That is definitely not
> what we should be looking at.
plane_state->uapi.dst then? but for what I found crtc_x/y is set from dst.
plane_state->uapi.dst is used in skl_program_plane()
skl_program_plane()
int crtc_x = plane_state->uapi.dst.x1;
int crtc_y = plane_state->uapi.dst.y1;
...
intel_de_write_fw(dev_priv, PLANE_POS(pipe, plane_id), (crtc_y << 16) | crtc_x);
>
> As the first step I think these functions should just program the
> registers with *exactly* the same values as we program into the
> normal plane register. That gets us to the point where we're
> actually programming something into the register without having to
> complicate things with calculating the selective fetch area.
Okay, I can move this to other patch but please check the comment above so we have this agreed for first version of the future patch.
>
> > + val |= plane_state->uapi.crtc_x;
> > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane->id),
> > + val);
> > +
> > + val = (clip->y1 + plane_state->color_plane[color_plane].y) << 16;
> > + val |= plane_state->color_plane[color_plane].x;
> > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane->id),
> > + val);
> > +
> > + /* Sizes are 0 based */
> > + val = (drm_rect_height(clip) - 1) << 16;
> > + val |= (drm_rect_width(&plane_state->uapi.src) >> 16) - 1;
> > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, plane->id),
> > + val);
> > +}
> > +
> > void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_state)
> > {
> > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > @@ -1187,17 +1227,96 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
> > crtc_state->psr2_man_track_ctl);
> > }
> >
> > -void intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> > - struct intel_crtc *crtc)
> > +static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state,
> > + struct drm_rect *clip, bool full_update)
> > +{
> > + u32 val = PSR2_MAN_TRK_CTL_ENABLE;
> > +
> > + if (full_update) {
> > + val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> > + goto exit;
> > + }
> > +
> > + drm_WARN_ON_ONCE(crtc_state->uapi.crtc->dev, clip->y1 == -1);
> > +
> > + val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
> > + val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
> > + val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip->y2, 4) + 1);
> > +exit:
> > + crtc_state->psr2_man_track_ctl = val;
> > +}
> > +
> > +static void clip_area_update(struct drm_rect *overlap_damage_area,
> > + struct drm_rect *damage_area)
> > +{
> > + if (overlap_damage_area->y1 == -1) {
> > + overlap_damage_area->y1 = damage_area->y1;
> > + overlap_damage_area->y2 = damage_area->y2;
> > + return;
> > + }
> > +
> > + if (damage_area->y1 < overlap_damage_area->y1)
> > + overlap_damage_area->y1 = damage_area->y1;
> > +
> > + if (damage_area->y2 > overlap_damage_area->y2)
> > + overlap_damage_area->y2 = damage_area->y2;
> > +}
> > +
> > +int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> > + struct intel_crtc *crtc)
> > {
> > struct intel_crtc_state *crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > + struct intel_plane_state *new_plane_state, *old_plane_state;
> > + struct drm_rect pipe_clip = { .y1 = -1 };
> > + struct intel_plane *plane;
> > + bool full_update = false;
> > + int i, ret;
> >
> > if (!dev_priv->psr.psr2_sel_fetch_enabled)
> > - return;
> > + return 0;
> > +
> > + ret = drm_atomic_add_affected_planes(&state->base, &crtc->base);
> > + if (ret)
> > + return ret;
> > +
> > + for_each_oldnew_intel_plane_in_state(state, plane, old_plane_state,
> > + new_plane_state, i) {
> > + struct drm_rect *plane_sel_fetch_area, temp;
> >
> > - crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE |
> > - PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> > + if (new_plane_state->uapi.crtc != crtc_state->uapi.crtc)
> > + continue;
> > +
> > + /*
> > + * TODO: Not clear how to handle planes with negative position,
> > + * also planes are not updated if they have a negative X
> > + * position so for now doing a full update in this cases
> > + */
> > + if (new_plane_state->uapi.crtc_y < 0 ||
> > + new_plane_state->uapi.crtc_x < 0) {
> > + full_update = true;
> > + break;
> > + }
> > +
> > + if (!new_plane_state->uapi.visible)
> > + continue;
> > +
> > + /*
> > + * For now doing a selective fetch in the whole plane area,
> > + * optimizations will come in the future.
> > + */
> > + plane_sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
> > + plane_sel_fetch_area->y1 = new_plane_state->uapi.src.y1 >> 16;
> > + plane_sel_fetch_area->y2 = new_plane_state->uapi.src.y2 >> 16;
> > +
> > + temp = *plane_sel_fetch_area;
> > + temp.y1 += new_plane_state->uapi.crtc_y;
> > + temp.y2 += new_plane_state->uapi.crtc_y;
> > + clip_area_update(&pipe_clip, &temp);
> > + }
> > +
> > + psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, full_update);
> > + return 0;
> > }
> >
> > /**
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h
> > index 6a83c8e682e6..3eca9dcec3c0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.h
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> > @@ -15,6 +15,8 @@ struct intel_crtc_state;
> > struct intel_dp;
> > struct intel_crtc;
> > struct intel_atomic_state;
> > +struct intel_plane_state;
> > +struct intel_plane;
> >
> > #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv->psr.sink_support)
> > void intel_psr_init_dpcd(struct intel_dp *intel_dp);
> > @@ -45,8 +47,12 @@ void intel_psr_atomic_check(struct drm_connector *connector,
> > struct drm_connector_state *old_state,
> > struct drm_connector_state *new_state);
> > void intel_psr_set_force_mode_changed(struct intel_dp *intel_dp);
> > -void intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> > - struct intel_crtc *crtc);
> > +int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> > + struct intel_crtc *crtc);
> > void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_state);
> > +void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> > + const struct intel_crtc_state *crtc_state,
> > + const struct intel_plane_state *plane_state,
> > + int color_plane);
> >
> > #endif /* __INTEL_PSR_H__ */
> > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> > index 1797a06cfd60..24ee9b08ec4a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > @@ -690,6 +690,9 @@ skl_program_plane(struct intel_plane *plane,
> > intel_de_write_fw(dev_priv, PLANE_AUX_OFFSET(pipe, plane_id),
> > (plane_state->color_plane[1].y << 16) | plane_state->color_plane[1].x);
> >
> > + if (!drm_atomic_crtc_needs_modeset(&crtc_state->uapi))
> > + intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, color_plane);
> > +
> > /*
> > * The control register self-arms if the plane was previously
> > * disabled. Try to make the plane enable atomic by writing
> > --
> > 2.28.0
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix state of PSR2 sub features
2020-09-14 19:57 ` Souza, Jose
@ 2020-09-14 20:30 ` Ville Syrjälä
2020-09-14 20:56 ` Souza, Jose
0 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjälä @ 2020-09-14 20:30 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx
On Mon, Sep 14, 2020 at 07:57:34PM +0000, Souza, Jose wrote:
> On Mon, 2020-09-14 at 17:24 +0300, Ville Syrjälä wrote:
> > On Mon, Aug 31, 2020 at 06:09:22PM -0700, José Roberto de Souza wrote:
> > > In case PSR2 is disabled by debugfs dc3co_enabled and
> > > psr2_sel_fetch_enabled were still being set causing some code paths
> > > to be executed were it should not.
> > > We have tests for PSR1 and PSR2 so keep those features disabled when
> > > PSR1 is active but PSR2 is supported is important.
> > >
> > > 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 | 11 +++++++----
> > > 1 file changed, 7 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 4e09ae61d4aa..6698d0209879 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -962,12 +962,14 @@ static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
> > > dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state);
> > > dev_priv->psr.busy_frontbuffer_bits = 0;
> > > dev_priv->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
> > > - dev_priv->psr.dc3co_enabled = !!crtc_state->dc3co_exitline;
> > > + dev_priv->psr.dc3co_enabled = !!crtc_state->dc3co_exitline &&
> > > + dev_priv->psr.psr2_enabled;
> > > dev_priv->psr.transcoder = crtc_state->cpu_transcoder;
> > > /* DC5/DC6 requires at least 6 idle frames */
> > > val = usecs_to_jiffies(intel_get_frame_time_us(crtc_state) * 6);
> > > dev_priv->psr.dc3co_exit_delay = val;
> > > - dev_priv->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch;
> > > + dev_priv->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch &&
> > > + dev_priv->psr.psr2_enabled;
> > >
> > > /*
> > > * If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
> > > @@ -1178,7 +1180,7 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
> > > struct i915_psr *psr = &dev_priv->psr;
> > >
> > > if (!HAS_PSR2_SEL_FETCH(dev_priv) ||
> > > - !crtc_state->enable_psr2_sel_fetch)
> > > + !dev_priv->psr.psr2_sel_fetch_enabled)
> > > return;
> > >
> > > intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(psr->transcoder),
> > > @@ -1189,8 +1191,9 @@ void intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> > > struct intel_crtc *crtc)
> > > {
> > > struct intel_crtc_state *crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
> > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > >
> > > - if (!crtc_state->enable_psr2_sel_fetch)
> > > + if (!dev_priv->psr.psr2_sel_fetch_enabled)
> >
> > This looks rather sketchy. AFAICS this gets called during atomic_check()
> > so looking at stuff outside the crtc state is very suspicious.
>
> This is called after the functions that change the PSR state so no issues, also we can't really on information in CRTC state, as PSR is only enabled
> if supported by state, i915 PSR parameter and PSR debug fs value.
I see it getting called from intel_crtc_atomic_check(). Confused.
Am I missing some other patches?
>
> >
> > > return;
> > >
> > > crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE |
> > > --
> > > 2.28.0
> >
> >
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix state of PSR2 sub features
2020-09-14 20:30 ` Ville Syrjälä
@ 2020-09-14 20:56 ` Souza, Jose
2020-09-15 11:50 ` Ville Syrjälä
0 siblings, 1 reply; 19+ messages in thread
From: Souza, Jose @ 2020-09-14 20:56 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Mon, 2020-09-14 at 23:30 +0300, Ville Syrjälä wrote:
> On Mon, Sep 14, 2020 at 07:57:34PM +0000, Souza, Jose wrote:
> > On Mon, 2020-09-14 at 17:24 +0300, Ville Syrjälä wrote:
> > > On Mon, Aug 31, 2020 at 06:09:22PM -0700, José Roberto de Souza wrote:
> > > > In case PSR2 is disabled by debugfs dc3co_enabled and
> > > > psr2_sel_fetch_enabled were still being set causing some code paths
> > > > to be executed were it should not.
> > > > We have tests for PSR1 and PSR2 so keep those features disabled when
> > > > PSR1 is active but PSR2 is supported is important.
> > > >
> > > > 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 | 11 +++++++----
> > > > 1 file changed, 7 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index 4e09ae61d4aa..6698d0209879 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -962,12 +962,14 @@ static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
> > > > dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state);
> > > > dev_priv->psr.busy_frontbuffer_bits = 0;
> > > > dev_priv->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
> > > > - dev_priv->psr.dc3co_enabled = !!crtc_state->dc3co_exitline;
> > > > + dev_priv->psr.dc3co_enabled = !!crtc_state->dc3co_exitline &&
> > > > + dev_priv->psr.psr2_enabled;
> > > > dev_priv->psr.transcoder = crtc_state->cpu_transcoder;
> > > > /* DC5/DC6 requires at least 6 idle frames */
> > > > val = usecs_to_jiffies(intel_get_frame_time_us(crtc_state) * 6);
> > > > dev_priv->psr.dc3co_exit_delay = val;
> > > > - dev_priv->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch;
> > > > + dev_priv->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch &&
> > > > + dev_priv->psr.psr2_enabled;
> > > >
> > > > /*
> > > > * If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
> > > > @@ -1178,7 +1180,7 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
> > > > struct i915_psr *psr = &dev_priv->psr;
> > > >
> > > > if (!HAS_PSR2_SEL_FETCH(dev_priv) ||
> > > > - !crtc_state->enable_psr2_sel_fetch)
> > > > + !dev_priv->psr.psr2_sel_fetch_enabled)
> > > > return;
> > > >
> > > > intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(psr->transcoder),
> > > > @@ -1189,8 +1191,9 @@ void intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> > > > struct intel_crtc *crtc)
> > > > {
> > > > struct intel_crtc_state *crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
> > > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > >
> > > > - if (!crtc_state->enable_psr2_sel_fetch)
> > > > + if (!dev_priv->psr.psr2_sel_fetch_enabled)
> > >
> > > This looks rather sketchy. AFAICS this gets called during atomic_check()
> > > so looking at stuff outside the crtc state is very suspicious.
> >
> > This is called after the functions that change the PSR state so no issues, also we can't really on information in CRTC state, as PSR is only enabled
> > if supported by state, i915 PSR parameter and PSR debug fs value.
>
> I see it getting called from intel_crtc_atomic_check(). Confused.
> Am I missing some other patches?
It is set from intel_psr_disable(), intel_psr_enable() and intel_psr_update() all executed before intel_psr2_sel_fetch_update()
intel_enable_ddi()
intel_enable_ddi_dp()
intel_psr_enable()
intel_update_crtc() {
if (!modeset) {
intel_encoders_update_pipe()
encoder->update_pipe() / intel_ddi_update_pipe()
intel_ddi_update_pipe_dp()
intel_psr_update()
}
...
skl_update_planes_on_crtc(state, crtc);
intel_update_plane()
plane->update_plane() / skl_update_plane()
skl_program_plane()
intel_psr2_sel_fetch_update()
}
>
> > > > return;
> > > >
> > > > crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE |
> > > > --
> > > > 2.28.0
> > >
> > >
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix state of PSR2 sub features
2020-09-14 20:56 ` Souza, Jose
@ 2020-09-15 11:50 ` Ville Syrjälä
2020-09-16 2:44 ` Souza, Jose
0 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjälä @ 2020-09-15 11:50 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx
On Mon, Sep 14, 2020 at 08:56:12PM +0000, Souza, Jose wrote:
> On Mon, 2020-09-14 at 23:30 +0300, Ville Syrjälä wrote:
> > On Mon, Sep 14, 2020 at 07:57:34PM +0000, Souza, Jose wrote:
> > > On Mon, 2020-09-14 at 17:24 +0300, Ville Syrjälä wrote:
> > > > On Mon, Aug 31, 2020 at 06:09:22PM -0700, José Roberto de Souza wrote:
> > > > > In case PSR2 is disabled by debugfs dc3co_enabled and
> > > > > psr2_sel_fetch_enabled were still being set causing some code paths
> > > > > to be executed were it should not.
> > > > > We have tests for PSR1 and PSR2 so keep those features disabled when
> > > > > PSR1 is active but PSR2 is supported is important.
> > > > >
> > > > > 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 | 11 +++++++----
> > > > > 1 file changed, 7 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > index 4e09ae61d4aa..6698d0209879 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > @@ -962,12 +962,14 @@ static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
> > > > > dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state);
> > > > > dev_priv->psr.busy_frontbuffer_bits = 0;
> > > > > dev_priv->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
> > > > > - dev_priv->psr.dc3co_enabled = !!crtc_state->dc3co_exitline;
> > > > > + dev_priv->psr.dc3co_enabled = !!crtc_state->dc3co_exitline &&
> > > > > + dev_priv->psr.psr2_enabled;
> > > > > dev_priv->psr.transcoder = crtc_state->cpu_transcoder;
> > > > > /* DC5/DC6 requires at least 6 idle frames */
> > > > > val = usecs_to_jiffies(intel_get_frame_time_us(crtc_state) * 6);
> > > > > dev_priv->psr.dc3co_exit_delay = val;
> > > > > - dev_priv->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch;
> > > > > + dev_priv->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch &&
> > > > > + dev_priv->psr.psr2_enabled;
> > > > >
> > > > > /*
> > > > > * If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
> > > > > @@ -1178,7 +1180,7 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
> > > > > struct i915_psr *psr = &dev_priv->psr;
> > > > >
> > > > > if (!HAS_PSR2_SEL_FETCH(dev_priv) ||
> > > > > - !crtc_state->enable_psr2_sel_fetch)
> > > > > + !dev_priv->psr.psr2_sel_fetch_enabled)
> > > > > return;
> > > > >
> > > > > intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(psr->transcoder),
> > > > > @@ -1189,8 +1191,9 @@ void intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> > > > > struct intel_crtc *crtc)
> > > > > {
> > > > > struct intel_crtc_state *crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
> > > > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > > >
> > > > > - if (!crtc_state->enable_psr2_sel_fetch)
> > > > > + if (!dev_priv->psr.psr2_sel_fetch_enabled)
> > > >
> > > > This looks rather sketchy. AFAICS this gets called during atomic_check()
> > > > so looking at stuff outside the crtc state is very suspicious.
> > >
> > > This is called after the functions that change the PSR state so no issues, also we can't really on information in CRTC state, as PSR is only enabled
> > > if supported by state, i915 PSR parameter and PSR debug fs value.
> >
> > I see it getting called from intel_crtc_atomic_check(). Confused.
> > Am I missing some other patches?
>
> It is set from intel_psr_disable(), intel_psr_enable() and intel_psr_update() all executed before intel_psr2_sel_fetch_update()
>
> intel_enable_ddi()
> intel_enable_ddi_dp()
> intel_psr_enable()
>
> intel_update_crtc() {
> if (!modeset) {
> intel_encoders_update_pipe()
> encoder->update_pipe() / intel_ddi_update_pipe()
> intel_ddi_update_pipe_dp()
> intel_psr_update()
> }
>
> ...
>
> skl_update_planes_on_crtc(state, crtc);
> intel_update_plane()
> plane->update_plane() / skl_update_plane()
> skl_program_plane()
> intel_psr2_sel_fetch_update()
That's not what I see at all. The only caller I see is
intel_crtc_atomic_check().
> }
>
>
> >
> > > > > return;
> > > > >
> > > > > crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE |
And if it would be called from there then this part would be
kinda bad. We should not mutate the state during the commit phase.
> > > > > --
> > > > > 2.28.0
> > > >
> > > >
> >
> >
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915/display: Program PSR2 selective fetch registers
2020-09-14 20:15 ` Souza, Jose
@ 2020-09-15 19:28 ` Mun, Gwan-gyeong
2020-09-15 19:57 ` Souza, Jose
0 siblings, 1 reply; 19+ messages in thread
From: Mun, Gwan-gyeong @ 2020-09-15 19:28 UTC (permalink / raw)
To: ville.syrjala, Souza, Jose; +Cc: intel-gfx
On Mon, 2020-09-14 at 13:15 -0700, Souza, Jose wrote:
> On Mon, 2020-09-14 at 17:28 +0300, Ville Syrjälä wrote:
> > On Mon, Aug 31, 2020 at 06:09:23PM -0700, José Roberto de Souza
> > wrote:
> > > Another step towards PSR2 selective fetch, here programming plane
> > > selective fetch registers and MAN_TRK_CTL enabling selective
> > > fetch but
> > > for now it is fetching the whole area of the planes.
> > > The damaged area calculation will come as next and final step.
> > >
> > > 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_display.c | 10 +-
> > > .../drm/i915/display/intel_display_types.h | 2 +
> > > drivers/gpu/drm/i915/display/intel_psr.c | 129
> > > +++++++++++++++++-
> > > drivers/gpu/drm/i915/display/intel_psr.h | 10 +-
> > > drivers/gpu/drm/i915/display/intel_sprite.c | 3 +
> > > 5 files changed, 145 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index c8b1dd1a9e46..865486e89915 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -11799,6 +11799,9 @@ static void i9xx_update_cursor(struct
> > > intel_plane *plane,
> > > if (INTEL_GEN(dev_priv) >= 9)
> > > skl_write_cursor_wm(plane, crtc_state);
> > >
> > > + if (!needs_modeset(crtc_state))
> > > + intel_psr2_program_plane_sel_fetch(plane, crtc_state,
> > > plane_state, 0);
> > > +
> > > if (plane->cursor.base != base ||
> > > plane->cursor.size != fbc_ctl ||
> > > plane->cursor.cntl != cntl) {
> > > @@ -12810,8 +12813,11 @@ static int
> > > intel_crtc_atomic_check(struct intel_atomic_state *state,
> > >
> > > }
> > >
> > > - if (!mode_changed)
> > > - intel_psr2_sel_fetch_update(state, crtc);
> > > + if (!mode_changed) {
> > > + ret = intel_psr2_sel_fetch_update(state, crtc);
> > > + if (ret)
> > > + return ret;
> > > + }
> > >
> > > return 0;
> > > }
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index 9349b15afff6..2138bb0f1587 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -586,6 +586,8 @@ struct intel_plane_state {
> > > u32 planar_slave;
> > >
> > > struct drm_intel_sprite_colorkey ckey;
> > > +
> > > + struct drm_rect psr2_sel_fetch_area;
> > > };
> > >
> > > struct intel_initial_plane_config {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 6698d0209879..b60ea133a527 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -1173,6 +1173,46 @@ static void
> > > psr_force_hw_tracking_exit(struct drm_i915_private *dev_priv)
> > > intel_psr_exit(dev_priv);
> > > }
> > >
> > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane
> > > *plane,
> > > + const struct intel_crtc_state
> > > *crtc_state,
> > > + const struct intel_plane_state
> > > *plane_state,
> > > + int color_plane)
> > > +{
> > > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > > + const struct drm_rect *clip;
> > > + enum pipe pipe = plane->pipe;
> > > + u32 val;
> > > +
> > > + if (!plane_state || !dev_priv->psr.psr2_sel_fetch_enabled)
> > > + return;
> > > +
> > > + /*
> > > + * skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0 for
> > > gen11+, so
> > > + * plane_state->ctl is the right value
> > > + */
As per Bspec 50420, "SEL_FETCH_PLANE_CTL[31]: Selective Fetch Plane
Enable bit" should be set.
And when "PSR2_MAN_TRK_CTL[1] : SF Partial Frame Enable bit" is enabled
selective fetch will be applied.
> > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane-
> > > >id), plane_state->ctl);
As per
> > > + if (!plane_state->ctl || plane->id == PLANE_CURSOR)
> > > + return;
> > > +
> > > + clip = &plane_state->psr2_sel_fetch_area;
> > > +
> > > + val = (clip->y1 + plane_state->uapi.crtc_y) << 16;
> >
> > crtc_x/y are the raw values usrspace gave us. That is definitely
> > not
> > what we should be looking at.
>
> plane_state->uapi.dst then? but for what I found crtc_x/y is set from
> dst.
>
> plane_state->uapi.dst is used in skl_program_plane()
>
> skl_program_plane()
> int crtc_x = plane_state->uapi.dst.x1;
> int crtc_y = plane_state->uapi.dst.y1;
> ...
> intel_de_write_fw(dev_priv, PLANE_POS(pipe, plane_id), (crtc_y
> << 16) | crtc_x);
>
>
> > As the first step I think these functions should just program the
> > registers with *exactly* the same values as we program into the
> > normal plane register. That gets us to the point where we're
> > actually programming something into the register without having to
> > complicate things with calculating the selective fetch area.
>
> Okay, I can move this to other patch but please check the comment
> above so we have this agreed for first version of the future patch.
>
> > > + val |= plane_state->uapi.crtc_x;
> > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane-
> > > >id),
> > > + val);
> > > +
> > > + val = (clip->y1 + plane_state->color_plane[color_plane].y) <<
> > > 16;
> > > + val |= plane_state->color_plane[color_plane].x;
> > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane-
> > > >id),
> > > + val);
> > > +
PLANE_SEL_FETCH_OFFSET values should be considered tiling information.
this code does not consider aux surfaces and fb offsets.
> > > + /* Sizes are 0 based */
> > > + val = (drm_rect_height(clip) - 1) << 16;
> > > + val |= (drm_rect_width(&plane_state->uapi.src) >> 16) - 1;
> > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, plane-
> > > >id),
> > > + val);
> > > +}
> > > +
> > > void intel_psr2_program_trans_man_trk_ctl(const struct
> > > intel_crtc_state *crtc_state)
> > > {
> > > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > @@ -1187,17 +1227,96 @@ void
> > > intel_psr2_program_trans_man_trk_ctl(const struct
> > > intel_crtc_state *crtc_st
> > > crtc_state->psr2_man_track_ctl);
> > > }
> > >
> > > -void intel_psr2_sel_fetch_update(struct intel_atomic_state
> > > *state,
> > > - struct intel_crtc *crtc)
> > > +static void psr2_man_trk_ctl_calc(struct intel_crtc_state
> > > *crtc_state,
> > > + struct drm_rect *clip, bool
> > > full_update)
> > > +{
> > > + u32 val = PSR2_MAN_TRK_CTL_ENABLE;
> > > +
> > > + if (full_update) {
> > > + val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> > > + goto exit;
> > > + }
> > > +
> > > + drm_WARN_ON_ONCE(crtc_state->uapi.crtc->dev, clip->y1 == -1);
> > > +
> > > + val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
> > > + val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
> > > + val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip-
> > > >y2, 4) + 1);
As per Bspec 50424, " The frame is divided into blocks of four scan
lines each. The blocks are addressed starting from 1 for the first
block of the frame and ending with ROUNDUP[(TRANS_VTOTAL Vertical
Active + 1) / 4]for the last block of the frame. Software must provide
the starting and ending block address of the selective update region.
The SU Region Start Address is programmed to the first block of the
selective update region. The SU Region End Address is programmed to the
final block of the selective update region + 1."
I think it should be like, val |=
PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip->y2 +1, 4) + 1);
> > > +exit:
> > > + crtc_state->psr2_man_track_ctl = val;
> > > +}
> > > +
> > > +static void clip_area_update(struct drm_rect
> > > *overlap_damage_area,
> > > + struct drm_rect *damage_area)
> > > +{
> > > + if (overlap_damage_area->y1 == -1) {
> > > + overlap_damage_area->y1 = damage_area->y1;
> > > + overlap_damage_area->y2 = damage_area->y2;
> > > + return;
> > > + }
> > > +
> > > + if (damage_area->y1 < overlap_damage_area->y1)
> > > + overlap_damage_area->y1 = damage_area->y1;
> > > +
> > > + if (damage_area->y2 > overlap_damage_area->y2)
> > > + overlap_damage_area->y2 = damage_area->y2;
> > > +}
> > > +
> > > +int intel_psr2_sel_fetch_update(struct intel_atomic_state
> > > *state,
> > > + struct intel_crtc *crtc)
> > > {
> > > struct intel_crtc_state *crtc_state =
> > > intel_atomic_get_new_crtc_state(state, crtc);
> > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > + struct intel_plane_state *new_plane_state, *old_plane_state;
> > > + struct drm_rect pipe_clip = { .y1 = -1 };
> > > + struct intel_plane *plane;
> > > + bool full_update = false;
> > > + int i, ret;
> > >
> > > if (!dev_priv->psr.psr2_sel_fetch_enabled)
> > > - return;
> > > + return 0;
> > > +
> > > + ret = drm_atomic_add_affected_planes(&state->base, &crtc-
> > > >base);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + for_each_oldnew_intel_plane_in_state(state, plane,
> > > old_plane_state,
> > > + new_plane_state, i) {
> > > + struct drm_rect *plane_sel_fetch_area, temp;
> > >
> > > - crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE |
> > > - PSR2_MAN_TRK_CTL_SF_SINGLE_FUL
> > > L_FRAME;
> > > + if (new_plane_state->uapi.crtc != crtc_state-
> > > >uapi.crtc)
> > > + continue;
> > > +
> > > + /*
> > > + * TODO: Not clear how to handle planes with negative
> > > position,
> > > + * also planes are not updated if they have a negative
> > > X
> > > + * position so for now doing a full update in this
> > > cases
> > > + */
> > > + if (new_plane_state->uapi.crtc_y < 0 ||
> > > + new_plane_state->uapi.crtc_x < 0) {
> > > + full_update = true;
> > > + break;
> > > + }
> > > +
> > > + if (!new_plane_state->uapi.visible)
> > > + continue;
> > > +
> > > + /*
> > > + * For now doing a selective fetch in the whole plane
> > > area,
> > > + * optimizations will come in the future.
> > > + */
> > > + plane_sel_fetch_area = &new_plane_state-
> > > >psr2_sel_fetch_area;
> > > + plane_sel_fetch_area->y1 = new_plane_state->uapi.src.y1
> > > >> 16;
> > > + plane_sel_fetch_area->y2 = new_plane_state->uapi.src.y2
> > > >> 16;
> > > +
> > > + temp = *plane_sel_fetch_area;
> > > + temp.y1 += new_plane_state->uapi.crtc_y;
> > > + temp.y2 += new_plane_state->uapi.crtc_y;
> > > + clip_area_update(&pipe_clip, &temp);
> > > + }
> > > +
> > > + psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, full_update);
> > > + return 0;
> > > }
> > >
> > > /**
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> > > b/drivers/gpu/drm/i915/display/intel_psr.h
> > > index 6a83c8e682e6..3eca9dcec3c0 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> > > @@ -15,6 +15,8 @@ struct intel_crtc_state;
> > > struct intel_dp;
> > > struct intel_crtc;
> > > struct intel_atomic_state;
> > > +struct intel_plane_state;
> > > +struct intel_plane;
> > >
> > > #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv-
> > > >psr.sink_support)
> > > void intel_psr_init_dpcd(struct intel_dp *intel_dp);
> > > @@ -45,8 +47,12 @@ void intel_psr_atomic_check(struct
> > > drm_connector *connector,
> > > struct drm_connector_state *old_state,
> > > struct drm_connector_state *new_state);
> > > void intel_psr_set_force_mode_changed(struct intel_dp
> > > *intel_dp);
> > > -void intel_psr2_sel_fetch_update(struct intel_atomic_state
> > > *state,
> > > - struct intel_crtc *crtc);
> > > +int intel_psr2_sel_fetch_update(struct intel_atomic_state
> > > *state,
> > > + struct intel_crtc *crtc);
> > > void intel_psr2_program_trans_man_trk_ctl(const struct
> > > intel_crtc_state *crtc_state);
> > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane
> > > *plane,
> > > + const struct intel_crtc_state
> > > *crtc_state,
> > > + const struct intel_plane_state
> > > *plane_state,
> > > + int color_plane);
> > >
> > > #endif /* __INTEL_PSR_H__ */
> > > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > index 1797a06cfd60..24ee9b08ec4a 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > @@ -690,6 +690,9 @@ skl_program_plane(struct intel_plane *plane,
> > > intel_de_write_fw(dev_priv, PLANE_AUX_OFFSET(pipe,
> > > plane_id),
> > > (plane_state->color_plane[1].y << 16)
> > > | plane_state->color_plane[1].x);
> > >
> > > + if (!drm_atomic_crtc_needs_modeset(&crtc_state->uapi))
> > > + intel_psr2_program_plane_sel_fetch(plane, crtc_state,
> > > plane_state, color_plane);
> > > +
> > > /*
> > > * The control register self-arms if the plane was previously
> > > * disabled. Try to make the plane enable atomic by writing
> > > --
> > > 2.28.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915/display: Program PSR2 selective fetch registers
2020-09-15 19:28 ` Mun, Gwan-gyeong
@ 2020-09-15 19:57 ` Souza, Jose
2020-09-17 14:05 ` Mun, Gwan-gyeong
0 siblings, 1 reply; 19+ messages in thread
From: Souza, Jose @ 2020-09-15 19:57 UTC (permalink / raw)
To: ville.syrjala, Mun, Gwan-gyeong; +Cc: intel-gfx
On Tue, 2020-09-15 at 20:28 +0100, Mun, Gwan-gyeong wrote:
> On Mon, 2020-09-14 at 13:15 -0700, Souza, Jose wrote:
> > On Mon, 2020-09-14 at 17:28 +0300, Ville Syrjälä wrote:
> > > On Mon, Aug 31, 2020 at 06:09:23PM -0700, José Roberto de Souza
> > > wrote:
> > > > Another step towards PSR2 selective fetch, here programming plane
> > > > selective fetch registers and MAN_TRK_CTL enabling selective
> > > > fetch but
> > > > for now it is fetching the whole area of the planes.
> > > > The damaged area calculation will come as next and final step.
> > > >
> > > > 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_display.c | 10 +-
> > > > .../drm/i915/display/intel_display_types.h | 2 +
> > > > drivers/gpu/drm/i915/display/intel_psr.c | 129
> > > > +++++++++++++++++-
> > > > drivers/gpu/drm/i915/display/intel_psr.h | 10 +-
> > > > drivers/gpu/drm/i915/display/intel_sprite.c | 3 +
> > > > 5 files changed, 145 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index c8b1dd1a9e46..865486e89915 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -11799,6 +11799,9 @@ static void i9xx_update_cursor(struct
> > > > intel_plane *plane,
> > > > if (INTEL_GEN(dev_priv) >= 9)
> > > > skl_write_cursor_wm(plane, crtc_state);
> > > >
> > > > + if (!needs_modeset(crtc_state))
> > > > + intel_psr2_program_plane_sel_fetch(plane, crtc_state,
> > > > plane_state, 0);
> > > > +
> > > > if (plane->cursor.base != base ||
> > > > plane->cursor.size != fbc_ctl ||
> > > > plane->cursor.cntl != cntl) {
> > > > @@ -12810,8 +12813,11 @@ static int
> > > > intel_crtc_atomic_check(struct intel_atomic_state *state,
> > > >
> > > > }
> > > >
> > > > - if (!mode_changed)
> > > > - intel_psr2_sel_fetch_update(state, crtc);
> > > > + if (!mode_changed) {
> > > > + ret = intel_psr2_sel_fetch_update(state, crtc);
> > > > + if (ret)
> > > > + return ret;
> > > > + }
> > > >
> > > > return 0;
> > > > }
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > index 9349b15afff6..2138bb0f1587 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > @@ -586,6 +586,8 @@ struct intel_plane_state {
> > > > u32 planar_slave;
> > > >
> > > > struct drm_intel_sprite_colorkey ckey;
> > > > +
> > > > + struct drm_rect psr2_sel_fetch_area;
> > > > };
> > > >
> > > > struct intel_initial_plane_config {
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index 6698d0209879..b60ea133a527 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -1173,6 +1173,46 @@ static void
> > > > psr_force_hw_tracking_exit(struct drm_i915_private *dev_priv)
> > > > intel_psr_exit(dev_priv);
> > > > }
> > > >
> > > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane
> > > > *plane,
> > > > + const struct intel_crtc_state
> > > > *crtc_state,
> > > > + const struct intel_plane_state
> > > > *plane_state,
> > > > + int color_plane)
> > > > +{
> > > > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > > > + const struct drm_rect *clip;
> > > > + enum pipe pipe = plane->pipe;
> > > > + u32 val;
> > > > +
> > > > + if (!plane_state || !dev_priv->psr.psr2_sel_fetch_enabled)
> > > > + return;
> > > > +
> > > > + /*
> > > > + * skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0 for
> > > > gen11+, so
> > > > + * plane_state->ctl is the right value
> > > > + */
>
> As per Bspec 50420, "SEL_FETCH_PLANE_CTL[31]: Selective Fetch Plane
> Enable bit" should be set.
> And when "PSR2_MAN_TRK_CTL[1] : SF Partial Frame Enable bit" is enabled
> selective fetch will be applied.
Bit 31 from PLANE_CTL is the enabled bit all the other fields are spare so we can program it without issues.
>
> > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane-
> > > > > id), plane_state->ctl);
>
> As per
> > > > + if (!plane_state->ctl || plane->id == PLANE_CURSOR)
> > > > + return;
> > > > +
> > > > + clip = &plane_state->psr2_sel_fetch_area;
> > > > +
> > > > + val = (clip->y1 + plane_state->uapi.crtc_y) << 16;
> > >
> > > crtc_x/y are the raw values usrspace gave us. That is definitely
> > > not
> > > what we should be looking at.
> >
> > plane_state->uapi.dst then? but for what I found crtc_x/y is set from
> > dst.
> >
> > plane_state->uapi.dst is used in skl_program_plane()
> >
> > skl_program_plane()
> > int crtc_x = plane_state->uapi.dst.x1;
> > int crtc_y = plane_state->uapi.dst.y1;
> > ...
> > intel_de_write_fw(dev_priv, PLANE_POS(pipe, plane_id), (crtc_y
> > << 16) | crtc_x);
> >
> >
> > > As the first step I think these functions should just program the
> > > registers with *exactly* the same values as we program into the
> > > normal plane register. That gets us to the point where we're
> > > actually programming something into the register without having to
> > > complicate things with calculating the selective fetch area.
> >
> > Okay, I can move this to other patch but please check the comment
> > above so we have this agreed for first version of the future patch.
> >
> > > > + val |= plane_state->uapi.crtc_x;
> > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane-
> > > > > id),
> > > >
> > > > + val);
> > > > +
> > > > + val = (clip->y1 + plane_state->color_plane[color_plane].y) <<
> > > > 16;
> > > > + val |= plane_state->color_plane[color_plane].x;
> > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane-
> > > > > id),
> > > >
> > > > + val);
> > > > +
>
> PLANE_SEL_FETCH_OFFSET values should be considered tiling information.
> this code does not consider aux surfaces and fb offsets.
Not familiar with this, could explain more?
plane_state->color_plane[color_plane].x/y are the ones used to program plane_offset, so some calculation will be needed before sum plane_state-
>color_plane[color_plane].y to clip->y1?
> > > > + /* Sizes are 0 based */
> > > > + val = (drm_rect_height(clip) - 1) << 16;
> > > > + val |= (drm_rect_width(&plane_state->uapi.src) >> 16) - 1;
> > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, plane-
> > > > > id),
> > > >
> > > > + val);
> > > > +}
> > > > +
> > > > void intel_psr2_program_trans_man_trk_ctl(const struct
> > > > intel_crtc_state *crtc_state)
> > > > {
> > > > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > > @@ -1187,17 +1227,96 @@ void
> > > > intel_psr2_program_trans_man_trk_ctl(const struct
> > > > intel_crtc_state *crtc_st
> > > > crtc_state->psr2_man_track_ctl);
> > > > }
> > > >
> > > > -void intel_psr2_sel_fetch_update(struct intel_atomic_state
> > > > *state,
> > > > - struct intel_crtc *crtc)
> > > > +static void psr2_man_trk_ctl_calc(struct intel_crtc_state
> > > > *crtc_state,
> > > > + struct drm_rect *clip, bool
> > > > full_update)
> > > > +{
> > > > + u32 val = PSR2_MAN_TRK_CTL_ENABLE;
> > > > +
> > > > + if (full_update) {
> > > > + val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> > > > + goto exit;
> > > > + }
> > > > +
> > > > + drm_WARN_ON_ONCE(crtc_state->uapi.crtc->dev, clip->y1 == -1);
> > > > +
> > > > + val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
> > > > + val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
> > > > + val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip-
> > > > > y2, 4) + 1);
>
> As per Bspec 50424, " The frame is divided into blocks of four scan
> lines each. The blocks are addressed starting from 1 for the first
> block of the frame and ending with ROUNDUP[(TRANS_VTOTAL Vertical
> Active + 1) / 4]for the last block of the frame. Software must provide
> the starting and ending block address of the selective update region.
> The SU Region Start Address is programmed to the first block of the
> selective update region. The SU Region End Address is programmed to the
> final block of the selective update region + 1."
> I think it should be like, val |=
> PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip->y2 +1, 4) + 1);
DIV_ROUND_UP(clip->y2 +1, 4) + 1 do not work for numbers that divide by 4.
clip->y2 = 1079, will result in 271
clip->y2 = 1080, will result in 272, one more block than a 1080 frame have
>
> > > > +exit:
> > > > + crtc_state->psr2_man_track_ctl = val;
> > > > +}
> > > > +
> > > > +static void clip_area_update(struct drm_rect
> > > > *overlap_damage_area,
> > > > + struct drm_rect *damage_area)
> > > > +{
> > > > + if (overlap_damage_area->y1 == -1) {
> > > > + overlap_damage_area->y1 = damage_area->y1;
> > > > + overlap_damage_area->y2 = damage_area->y2;
> > > > + return;
> > > > + }
> > > > +
> > > > + if (damage_area->y1 < overlap_damage_area->y1)
> > > > + overlap_damage_area->y1 = damage_area->y1;
> > > > +
> > > > + if (damage_area->y2 > overlap_damage_area->y2)
> > > > + overlap_damage_area->y2 = damage_area->y2;
> > > > +}
> > > > +
> > > > +int intel_psr2_sel_fetch_update(struct intel_atomic_state
> > > > *state,
> > > > + struct intel_crtc *crtc)
> > > > {
> > > > struct intel_crtc_state *crtc_state =
> > > > intel_atomic_get_new_crtc_state(state, crtc);
> > > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > > + struct intel_plane_state *new_plane_state, *old_plane_state;
> > > > + struct drm_rect pipe_clip = { .y1 = -1 };
> > > > + struct intel_plane *plane;
> > > > + bool full_update = false;
> > > > + int i, ret;
> > > >
> > > > if (!dev_priv->psr.psr2_sel_fetch_enabled)
> > > > - return;
> > > > + return 0;
> > > > +
> > > > + ret = drm_atomic_add_affected_planes(&state->base, &crtc-
> > > > > base);
> > > >
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + for_each_oldnew_intel_plane_in_state(state, plane,
> > > > old_plane_state,
> > > > + new_plane_state, i) {
> > > > + struct drm_rect *plane_sel_fetch_area, temp;
> > > >
> > > > - crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE |
> > > > - PSR2_MAN_TRK_CTL_SF_SINGLE_FUL
> > > > L_FRAME;
> > > > + if (new_plane_state->uapi.crtc != crtc_state-
> > > > > uapi.crtc)
> > > >
> > > > + continue;
> > > > +
> > > > + /*
> > > > + * TODO: Not clear how to handle planes with negative
> > > > position,
> > > > + * also planes are not updated if they have a negative
> > > > X
> > > > + * position so for now doing a full update in this
> > > > cases
> > > > + */
> > > > + if (new_plane_state->uapi.crtc_y < 0 ||
> > > > + new_plane_state->uapi.crtc_x < 0) {
> > > > + full_update = true;
> > > > + break;
> > > > + }
> > > > +
> > > > + if (!new_plane_state->uapi.visible)
> > > > + continue;
> > > > +
> > > > + /*
> > > > + * For now doing a selective fetch in the whole plane
> > > > area,
> > > > + * optimizations will come in the future.
> > > > + */
> > > > + plane_sel_fetch_area = &new_plane_state-
> > > > > psr2_sel_fetch_area;
> > > >
> > > > + plane_sel_fetch_area->y1 = new_plane_state->uapi.src.y1
> > > > > > 16;
> > > >
> > > > + plane_sel_fetch_area->y2 = new_plane_state->uapi.src.y2
> > > > > > 16;
> > > >
> > > > +
> > > > + temp = *plane_sel_fetch_area;
> > > > + temp.y1 += new_plane_state->uapi.crtc_y;
> > > > + temp.y2 += new_plane_state->uapi.crtc_y;
> > > > + clip_area_update(&pipe_clip, &temp);
> > > > + }
> > > > +
> > > > + psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, full_update);
> > > > + return 0;
> > > > }
> > > >
> > > > /**
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> > > > b/drivers/gpu/drm/i915/display/intel_psr.h
> > > > index 6a83c8e682e6..3eca9dcec3c0 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> > > > @@ -15,6 +15,8 @@ struct intel_crtc_state;
> > > > struct intel_dp;
> > > > struct intel_crtc;
> > > > struct intel_atomic_state;
> > > > +struct intel_plane_state;
> > > > +struct intel_plane;
> > > >
> > > > #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv-
> > > > > psr.sink_support)
> > > >
> > > > void intel_psr_init_dpcd(struct intel_dp *intel_dp);
> > > > @@ -45,8 +47,12 @@ void intel_psr_atomic_check(struct
> > > > drm_connector *connector,
> > > > struct drm_connector_state *old_state,
> > > > struct drm_connector_state *new_state);
> > > > void intel_psr_set_force_mode_changed(struct intel_dp
> > > > *intel_dp);
> > > > -void intel_psr2_sel_fetch_update(struct intel_atomic_state
> > > > *state,
> > > > - struct intel_crtc *crtc);
> > > > +int intel_psr2_sel_fetch_update(struct intel_atomic_state
> > > > *state,
> > > > + struct intel_crtc *crtc);
> > > > void intel_psr2_program_trans_man_trk_ctl(const struct
> > > > intel_crtc_state *crtc_state);
> > > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane
> > > > *plane,
> > > > + const struct intel_crtc_state
> > > > *crtc_state,
> > > > + const struct intel_plane_state
> > > > *plane_state,
> > > > + int color_plane);
> > > >
> > > > #endif /* __INTEL_PSR_H__ */
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > > b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > > index 1797a06cfd60..24ee9b08ec4a 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > > @@ -690,6 +690,9 @@ skl_program_plane(struct intel_plane *plane,
> > > > intel_de_write_fw(dev_priv, PLANE_AUX_OFFSET(pipe,
> > > > plane_id),
> > > > (plane_state->color_plane[1].y << 16)
> > > > > plane_state->color_plane[1].x);
> > > >
> > > >
> > > > + if (!drm_atomic_crtc_needs_modeset(&crtc_state->uapi))
> > > > + intel_psr2_program_plane_sel_fetch(plane, crtc_state,
> > > > plane_state, color_plane);
> > > > +
> > > > /*
> > > > * The control register self-arms if the plane was previously
> > > > * disabled. Try to make the plane enable atomic by writing
> > > > --
> > > > 2.28.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix state of PSR2 sub features
2020-09-15 11:50 ` Ville Syrjälä
@ 2020-09-16 2:44 ` Souza, Jose
0 siblings, 0 replies; 19+ messages in thread
From: Souza, Jose @ 2020-09-16 2:44 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Tue, 2020-09-15 at 14:50 +0300, Ville Syrjälä wrote:
> On Mon, Sep 14, 2020 at 08:56:12PM +0000, Souza, Jose wrote:
> > On Mon, 2020-09-14 at 23:30 +0300, Ville Syrjälä wrote:
> > > On Mon, Sep 14, 2020 at 07:57:34PM +0000, Souza, Jose wrote:
> > > > On Mon, 2020-09-14 at 17:24 +0300, Ville Syrjälä wrote:
> > > > > On Mon, Aug 31, 2020 at 06:09:22PM -0700, José Roberto de Souza wrote:
> > > > > > In case PSR2 is disabled by debugfs dc3co_enabled and
> > > > > > psr2_sel_fetch_enabled were still being set causing some code paths
> > > > > > to be executed were it should not.
> > > > > > We have tests for PSR1 and PSR2 so keep those features disabled when
> > > > > > PSR1 is active but PSR2 is supported is important.
> > > > > >
> > > > > > 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 | 11 +++++++----
> > > > > > 1 file changed, 7 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > > index 4e09ae61d4aa..6698d0209879 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > > @@ -962,12 +962,14 @@ static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
> > > > > > dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state);
> > > > > > dev_priv->psr.busy_frontbuffer_bits = 0;
> > > > > > dev_priv->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
> > > > > > - dev_priv->psr.dc3co_enabled = !!crtc_state->dc3co_exitline;
> > > > > > + dev_priv->psr.dc3co_enabled = !!crtc_state->dc3co_exitline &&
> > > > > > + dev_priv->psr.psr2_enabled;
> > > > > > dev_priv->psr.transcoder = crtc_state->cpu_transcoder;
> > > > > > /* DC5/DC6 requires at least 6 idle frames */
> > > > > > val = usecs_to_jiffies(intel_get_frame_time_us(crtc_state) * 6);
> > > > > > dev_priv->psr.dc3co_exit_delay = val;
> > > > > > - dev_priv->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch;
> > > > > > + dev_priv->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch &&
> > > > > > + dev_priv->psr.psr2_enabled;
> > > > > >
> > > > > > /*
> > > > > > * If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
> > > > > > @@ -1178,7 +1180,7 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
> > > > > > struct i915_psr *psr = &dev_priv->psr;
> > > > > >
> > > > > > if (!HAS_PSR2_SEL_FETCH(dev_priv) ||
> > > > > > - !crtc_state->enable_psr2_sel_fetch)
> > > > > > + !dev_priv->psr.psr2_sel_fetch_enabled)
> > > > > > return;
> > > > > >
> > > > > > intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(psr->transcoder),
> > > > > > @@ -1189,8 +1191,9 @@ void intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> > > > > > struct intel_crtc *crtc)
> > > > > > {
> > > > > > struct intel_crtc_state *crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
> > > > > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > > > >
> > > > > > - if (!crtc_state->enable_psr2_sel_fetch)
> > > > > > + if (!dev_priv->psr.psr2_sel_fetch_enabled)
> > > > >
> > > > > This looks rather sketchy. AFAICS this gets called during atomic_check()
> > > > > so looking at stuff outside the crtc state is very suspicious.
> > > >
> > > > This is called after the functions that change the PSR state so no issues, also we can't really on information in CRTC state, as PSR is only enabled
> > > > if supported by state, i915 PSR parameter and PSR debug fs value.
> > >
> > > I see it getting called from intel_crtc_atomic_check(). Confused.
> > > Am I missing some other patches?
> >
> > It is set from intel_psr_disable(), intel_psr_enable() and intel_psr_update() all executed before intel_psr2_sel_fetch_update()
> >
> > intel_enable_ddi()
> > intel_enable_ddi_dp()
> > intel_psr_enable()
> >
> > intel_update_crtc() {
> > if (!modeset) {
> > intel_encoders_update_pipe()
> > encoder->update_pipe() / intel_ddi_update_pipe()
> > intel_ddi_update_pipe_dp()
> > intel_psr_update()
> > }
> >
> > ...
> >
> > skl_update_planes_on_crtc(state, crtc);
> > intel_update_plane()
> > plane->update_plane() / skl_update_plane()
> > skl_program_plane()
> > intel_psr2_sel_fetch_update()
>
> That's not what I see at all. The only caller I see is
> intel_crtc_atomic_check().
Messed it with program function.
Yeah this will not work, so just fixed the has_psr and has_psr2 to match real state, just sent the new patch.
Thanks for the comments.
>
>
> > }
> >
> >
> > > > > > return;
> > > > > >
> > > > > > crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE |
>
> And if it would be called from there then this part would be
> kinda bad. We should not mutate the state during the commit phase.
>
> > > > > > --
> > > > > > 2.28.0
> > > > >
> > > > >
> > >
> > >
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915/display: Program PSR2 selective fetch registers
2020-09-15 19:57 ` Souza, Jose
@ 2020-09-17 14:05 ` Mun, Gwan-gyeong
0 siblings, 0 replies; 19+ messages in thread
From: Mun, Gwan-gyeong @ 2020-09-17 14:05 UTC (permalink / raw)
To: ville.syrjala, Souza, Jose; +Cc: intel-gfx
On Tue, 2020-09-15 at 12:57 -0700, Souza, Jose wrote:
> On Tue, 2020-09-15 at 20:28 +0100, Mun, Gwan-gyeong wrote:
> > On Mon, 2020-09-14 at 13:15 -0700, Souza, Jose wrote:
> > > On Mon, 2020-09-14 at 17:28 +0300, Ville Syrjälä wrote:
> > > > On Mon, Aug 31, 2020 at 06:09:23PM -0700, José Roberto de Souza
> > > > wrote:
> > > > > Another step towards PSR2 selective fetch, here programming
> > > > > plane
> > > > > selective fetch registers and MAN_TRK_CTL enabling selective
> > > > > fetch but
> > > > > for now it is fetching the whole area of the planes.
> > > > > The damaged area calculation will come as next and final
> > > > > step.
> > > > >
> > > > > 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_display.c | 10 +-
> > > > > .../drm/i915/display/intel_display_types.h | 2 +
> > > > > drivers/gpu/drm/i915/display/intel_psr.c | 129
> > > > > +++++++++++++++++-
> > > > > drivers/gpu/drm/i915/display/intel_psr.h | 10 +-
> > > > > drivers/gpu/drm/i915/display/intel_sprite.c | 3 +
> > > > > 5 files changed, 145 insertions(+), 9 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > index c8b1dd1a9e46..865486e89915 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > @@ -11799,6 +11799,9 @@ static void i9xx_update_cursor(struct
> > > > > intel_plane *plane,
> > > > > if (INTEL_GEN(dev_priv) >= 9)
> > > > > skl_write_cursor_wm(plane, crtc_state);
> > > > >
> > > > > + if (!needs_modeset(crtc_state))
> > > > > + intel_psr2_program_plane_sel_fetch(plane,
> > > > > crtc_state,
> > > > > plane_state, 0);
> > > > > +
> > > > > if (plane->cursor.base != base ||
> > > > > plane->cursor.size != fbc_ctl ||
> > > > > plane->cursor.cntl != cntl) {
> > > > > @@ -12810,8 +12813,11 @@ static int
> > > > > intel_crtc_atomic_check(struct intel_atomic_state *state,
> > > > >
> > > > > }
> > > > >
> > > > > - if (!mode_changed)
> > > > > - intel_psr2_sel_fetch_update(state, crtc);
> > > > > + if (!mode_changed) {
> > > > > + ret = intel_psr2_sel_fetch_update(state, crtc);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > + }
> > > > >
> > > > > return 0;
> > > > > }
> > > > > diff --git
> > > > > a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > > index 9349b15afff6..2138bb0f1587 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > > @@ -586,6 +586,8 @@ struct intel_plane_state {
> > > > > u32 planar_slave;
> > > > >
> > > > > struct drm_intel_sprite_colorkey ckey;
> > > > > +
> > > > > + struct drm_rect psr2_sel_fetch_area;
> > > > > };
> > > > >
> > > > > struct intel_initial_plane_config {
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > index 6698d0209879..b60ea133a527 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > @@ -1173,6 +1173,46 @@ static void
> > > > > psr_force_hw_tracking_exit(struct drm_i915_private *dev_priv)
> > > > > intel_psr_exit(dev_priv);
> > > > > }
> > > > >
> > > > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane
> > > > > *plane,
> > > > > + const struct
> > > > > intel_crtc_state
> > > > > *crtc_state,
> > > > > + const struct
> > > > > intel_plane_state
> > > > > *plane_state,
> > > > > + int color_plane)
> > > > > +{
> > > > > + struct drm_i915_private *dev_priv = to_i915(plane-
> > > > > >base.dev);
> > > > > + const struct drm_rect *clip;
> > > > > + enum pipe pipe = plane->pipe;
> > > > > + u32 val;
> > > > > +
> > > > > + if (!plane_state || !dev_priv-
> > > > > >psr.psr2_sel_fetch_enabled)
> > > > > + return;
> > > > > +
> > > > > + /*
> > > > > + * skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0
> > > > > for
> > > > > gen11+, so
> > > > > + * plane_state->ctl is the right value
> > > > > + */
> >
> > As per Bspec 50420, "SEL_FETCH_PLANE_CTL[31]: Selective Fetch
> > Plane
> > Enable bit" should be set.
> > And when "PSR2_MAN_TRK_CTL[1] : SF Partial Frame Enable bit" is
> > enabled
> > selective fetch will be applied.
>
> Bit 31 from PLANE_CTL is the enabled bit all the other fields are
> spare so we can program it without issues.
>
> > > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe,
> > > > > plane-
> > > > > > id), plane_state->ctl);
> >
> > As per
> > > > > + if (!plane_state->ctl || plane->id == PLANE_CURSOR)
> > > > > + return;
> > > > > +
> > > > > + clip = &plane_state->psr2_sel_fetch_area;
> > > > > +
> > > > > + val = (clip->y1 + plane_state->uapi.crtc_y) << 16;
> > > >
> > > > crtc_x/y are the raw values usrspace gave us. That is
> > > > definitely
> > > > not
> > > > what we should be looking at.
> > >
> > > plane_state->uapi.dst then? but for what I found crtc_x/y is set
> > > from
> > > dst.
> > >
> > > plane_state->uapi.dst is used in skl_program_plane()
> > >
> > > skl_program_plane()
> > > int crtc_x = plane_state->uapi.dst.x1;
> > > int crtc_y = plane_state->uapi.dst.y1;
> > > ...
> > > intel_de_write_fw(dev_priv, PLANE_POS(pipe, plane_id), (crtc_y
> > > << 16) | crtc_x);
> > >
> > >
> > > > As the first step I think these functions should just program
> > > > the
> > > > registers with *exactly* the same values as we program into the
> > > > normal plane register. That gets us to the point where we're
> > > > actually programming something into the register without having
> > > > to
> > > > complicate things with calculating the selective fetch area.
> > >
> > > Okay, I can move this to other patch but please check the comment
> > > above so we have this agreed for first version of the future
> > > patch.
> > >
> > > > > + val |= plane_state->uapi.crtc_x;
> > > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe,
> > > > > plane-
> > > > > > id),
> > > > >
> > > > > + val);
> > > > > +
> > > > > + val = (clip->y1 + plane_state-
> > > > > >color_plane[color_plane].y) <<
> > > > > 16;
> > > > > + val |= plane_state->color_plane[color_plane].x;
> > > > > + intel_de_write_fw(dev_priv,
> > > > > PLANE_SEL_FETCH_OFFSET(pipe, plane-
> > > > > > id),
> > > > >
> > > > > + val);
> > > > > +
> >
> > PLANE_SEL_FETCH_OFFSET values should be considered tiling
> > information.
> > this code does not consider aux surfaces and fb offsets.
>
> Not familiar with this, could explain more?
As per Bspec: 55229,
Sel_Fetch Plane Y Offset = Plane Y offset + Update offset
the Plane Y offset is calculated with tiling information for non linear
surface. (linear surface also needs to calculate with the pitch ...)
therefore it seems that we need to separately calculate for Sel_Fetch
Plane Y Offset
refer. intel_adjust_aligned_offset()
> plane_state->color_plane[color_plane].x/y are the ones used to
> program plane_offset, so some calculation will be needed before sum
> plane_state-
> > color_plane[color_plane].y to clip->y1?
> > > > > + /* Sizes are 0 based */
> > > > > + val = (drm_rect_height(clip) - 1) << 16;
> > > > > + val |= (drm_rect_width(&plane_state->uapi.src) >> 16) -
> > > > > 1;
> > > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe,
> > > > > plane-
> > > > > > id),
> > > > >
> > > > > + val);
> > > > > +}
> > > > > +
> > > > > void intel_psr2_program_trans_man_trk_ctl(const struct
> > > > > intel_crtc_state *crtc_state)
> > > > > {
> > > > > struct intel_crtc *crtc = to_intel_crtc(crtc_state-
> > > > > >uapi.crtc);
> > > > > @@ -1187,17 +1227,96 @@ void
> > > > > intel_psr2_program_trans_man_trk_ctl(const struct
> > > > > intel_crtc_state *crtc_st
> > > > > crtc_state->psr2_man_track_ctl);
> > > > > }
> > > > >
> > > > > -void intel_psr2_sel_fetch_update(struct intel_atomic_state
> > > > > *state,
> > > > > - struct intel_crtc *crtc)
> > > > > +static void psr2_man_trk_ctl_calc(struct intel_crtc_state
> > > > > *crtc_state,
> > > > > + struct drm_rect *clip, bool
> > > > > full_update)
> > > > > +{
> > > > > + u32 val = PSR2_MAN_TRK_CTL_ENABLE;
> > > > > +
> > > > > + if (full_update) {
> > > > > + val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> > > > > + goto exit;
> > > > > + }
> > > > > +
> > > > > + drm_WARN_ON_ONCE(crtc_state->uapi.crtc->dev, clip->y1
> > > > > == -1);
> > > > > +
> > > > > + val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
> > > > > + val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 /
> > > > > 4 + 1);
> > > > > + val |=
> > > > > PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip-
> > > > > > y2, 4) + 1);
> >
> > As per Bspec 50424, " The frame is divided into blocks of four scan
> > lines each. The blocks are addressed starting from 1 for the first
> > block of the frame and ending with ROUNDUP[(TRANS_VTOTAL Vertical
> > Active + 1) / 4]for the last block of the frame. Software must
> > provide
> > the starting and ending block address of the selective update
> > region.
> > The SU Region Start Address is programmed to the first block of the
> > selective update region. The SU Region End Address is programmed to
> > the
> > final block of the selective update region + 1."
> > I think it should be like, val |=
> > PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip->y2 +1, 4) +
> > 1);
>
> DIV_ROUND_UP(clip->y2 +1, 4) + 1 do not work for numbers that divide
> by 4.
> clip->y2 = 1079, will result in 271
> clip->y2 = 1080, will result in 272, one more block than a 1080 frame
> have
>
> > > > > +exit:
> > > > > + crtc_state->psr2_man_track_ctl = val;
> > > > > +}
> > > > > +
> > > > > +static void clip_area_update(struct drm_rect
> > > > > *overlap_damage_area,
> > > > > + struct drm_rect *damage_area)
> > > > > +{
> > > > > + if (overlap_damage_area->y1 == -1) {
> > > > > + overlap_damage_area->y1 = damage_area->y1;
> > > > > + overlap_damage_area->y2 = damage_area->y2;
> > > > > + return;
> > > > > + }
> > > > > +
> > > > > + if (damage_area->y1 < overlap_damage_area->y1)
> > > > > + overlap_damage_area->y1 = damage_area->y1;
> > > > > +
> > > > > + if (damage_area->y2 > overlap_damage_area->y2)
> > > > > + overlap_damage_area->y2 = damage_area->y2;
> > > > > +}
> > > > > +
> > > > > +int intel_psr2_sel_fetch_update(struct intel_atomic_state
> > > > > *state,
> > > > > + struct intel_crtc *crtc)
> > > > > {
> > > > > struct intel_crtc_state *crtc_state =
> > > > > intel_atomic_get_new_crtc_state(state, crtc);
> > > > > struct drm_i915_private *dev_priv = to_i915(crtc-
> > > > > >base.dev);
> > > > > + struct intel_plane_state *new_plane_state,
> > > > > *old_plane_state;
> > > > > + struct drm_rect pipe_clip = { .y1 = -1 };
> > > > > + struct intel_plane *plane;
> > > > > + bool full_update = false;
> > > > > + int i, ret;
> > > > >
> > > > > if (!dev_priv->psr.psr2_sel_fetch_enabled)
> > > > > - return;
> > > > > + return 0;
> > > > > +
> > > > > + ret = drm_atomic_add_affected_planes(&state->base,
> > > > > &crtc-
> > > > > > base);
> > > > >
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + for_each_oldnew_intel_plane_in_state(state, plane,
> > > > > old_plane_state,
> > > > > + new_plane_state,
> > > > > i) {
> > > > > + struct drm_rect *plane_sel_fetch_area, temp;
> > > > >
> > > > > - crtc_state->psr2_man_track_ctl =
> > > > > PSR2_MAN_TRK_CTL_ENABLE |
> > > > > - PSR2_MAN_TRK_CTL_SF_SI
> > > > > NGLE_FUL
> > > > > L_FRAME;
> > > > > + if (new_plane_state->uapi.crtc != crtc_state-
> > > > > > uapi.crtc)
> > > > >
> > > > > + continue;
> > > > > +
> > > > > + /*
> > > > > + * TODO: Not clear how to handle planes with
> > > > > negative
> > > > > position,
> > > > > + * also planes are not updated if they have a
> > > > > negative
> > > > > X
> > > > > + * position so for now doing a full update in
> > > > > this
> > > > > cases
> > > > > + */
> > > > > + if (new_plane_state->uapi.crtc_y < 0 ||
> > > > > + new_plane_state->uapi.crtc_x < 0) {
> > > > > + full_update = true;
> > > > > + break;
> > > > > + }
> > > > > +
> > > > > + if (!new_plane_state->uapi.visible)
> > > > > + continue;
> > > > > +
> > > > > + /*
> > > > > + * For now doing a selective fetch in the whole
> > > > > plane
> > > > > area,
> > > > > + * optimizations will come in the future.
> > > > > + */
> > > > > + plane_sel_fetch_area = &new_plane_state-
> > > > > > psr2_sel_fetch_area;
> > > > >
> > > > > + plane_sel_fetch_area->y1 = new_plane_state-
> > > > > >uapi.src.y1
> > > > > > > 16;
> > > > >
> > > > > + plane_sel_fetch_area->y2 = new_plane_state-
> > > > > >uapi.src.y2
> > > > > > > 16;
> > > > >
> > > > > +
> > > > > + temp = *plane_sel_fetch_area;
> > > > > + temp.y1 += new_plane_state->uapi.crtc_y;
> > > > > + temp.y2 += new_plane_state->uapi.crtc_y;
> > > > > + clip_area_update(&pipe_clip, &temp);
> > > > > + }
> > > > > +
> > > > > + psr2_man_trk_ctl_calc(crtc_state, &pipe_clip,
> > > > > full_update);
> > > > > + return 0;
> > > > > }
> > > > >
> > > > > /**
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> > > > > b/drivers/gpu/drm/i915/display/intel_psr.h
> > > > > index 6a83c8e682e6..3eca9dcec3c0 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.h
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> > > > > @@ -15,6 +15,8 @@ struct intel_crtc_state;
> > > > > struct intel_dp;
> > > > > struct intel_crtc;
> > > > > struct intel_atomic_state;
> > > > > +struct intel_plane_state;
> > > > > +struct intel_plane;
> > > > >
> > > > > #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv-
> > > > > > psr.sink_support)
> > > > >
> > > > > void intel_psr_init_dpcd(struct intel_dp *intel_dp);
> > > > > @@ -45,8 +47,12 @@ void intel_psr_atomic_check(struct
> > > > > drm_connector *connector,
> > > > > struct drm_connector_state
> > > > > *old_state,
> > > > > struct drm_connector_state
> > > > > *new_state);
> > > > > void intel_psr_set_force_mode_changed(struct intel_dp
> > > > > *intel_dp);
> > > > > -void intel_psr2_sel_fetch_update(struct intel_atomic_state
> > > > > *state,
> > > > > - struct intel_crtc *crtc);
> > > > > +int intel_psr2_sel_fetch_update(struct intel_atomic_state
> > > > > *state,
> > > > > + struct intel_crtc *crtc);
> > > > > void intel_psr2_program_trans_man_trk_ctl(const struct
> > > > > intel_crtc_state *crtc_state);
> > > > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane
> > > > > *plane,
> > > > > + const struct
> > > > > intel_crtc_state
> > > > > *crtc_state,
> > > > > + const struct
> > > > > intel_plane_state
> > > > > *plane_state,
> > > > > + int color_plane);
> > > > >
> > > > > #endif /* __INTEL_PSR_H__ */
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > > > b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > > > index 1797a06cfd60..24ee9b08ec4a 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > > > @@ -690,6 +690,9 @@ skl_program_plane(struct intel_plane
> > > > > *plane,
> > > > > intel_de_write_fw(dev_priv,
> > > > > PLANE_AUX_OFFSET(pipe,
> > > > > plane_id),
> > > > > (plane_state-
> > > > > >color_plane[1].y << 16)
> > > > > > plane_state->color_plane[1].x);
> > > > >
> > > > >
> > > > > + if (!drm_atomic_crtc_needs_modeset(&crtc_state->uapi))
> > > > > + intel_psr2_program_plane_sel_fetch(plane,
> > > > > crtc_state,
> > > > > plane_state, color_plane);
> > > > > +
> > > > > /*
> > > > > * The control register self-arms if the plane was
> > > > > previously
> > > > > * disabled. Try to make the plane enable atomic by
> > > > > writing
> > > > > --
> > > > > 2.28.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2020-09-17 14:07 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-01 1:09 [Intel-gfx] [PATCH 1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch José Roberto de Souza
2020-09-01 1:09 ` [Intel-gfx] [PATCH 2/4] drm/i915/display: Fix state of PSR2 sub features José Roberto de Souza
2020-09-14 14:24 ` Ville Syrjälä
2020-09-14 19:57 ` Souza, Jose
2020-09-14 20:30 ` Ville Syrjälä
2020-09-14 20:56 ` Souza, Jose
2020-09-15 11:50 ` Ville Syrjälä
2020-09-16 2:44 ` Souza, Jose
2020-09-01 1:09 ` [Intel-gfx] [PATCH 3/4] drm/i915/display: Program PSR2 selective fetch registers José Roberto de Souza
2020-09-14 14:28 ` Ville Syrjälä
2020-09-14 20:15 ` Souza, Jose
2020-09-15 19:28 ` Mun, Gwan-gyeong
2020-09-15 19:57 ` Souza, Jose
2020-09-17 14:05 ` Mun, Gwan-gyeong
2020-09-01 1:09 ` [Intel-gfx] [PATCH 4/4] HAX/DO_NOT_MERGE_IT: drm/i915/display: Enable PSR2 selective fetch for testing José Roberto de Souza
2020-09-01 1:46 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch Patchwork
2020-09-01 9:59 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2020-09-01 16:49 ` Souza, Jose
2020-09-01 19:30 ` [Intel-gfx] [PATCH 1/4] " Rodrigo Vivi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.