* [Intel-gfx] [PATCH 1/3] drm/i915/display: Move dc3co_exitline variable to struct intel_psr
@ 2021-03-03 16:41 Gwan-gyeong Mun
2021-03-03 16:42 ` [Intel-gfx] [PATCH 2/3] drm/i915/display: Remove a redundant function argument from intel_psr_enable_source() Gwan-gyeong Mun
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Gwan-gyeong Mun @ 2021-03-03 16:41 UTC (permalink / raw)
To: intel-gfx
dc3co_exitline is indirectly called by intel_psr_compute_config().
And it will not be changed until the next calling of
intel_psr_compute_config(). So in order to use dc3co_exitline without
intel_crtc_state on other psr internal function, it moves dc3co_exitline
variable to struct intel_psr.
And it removes a dc3co_enabled variable from struct intel_psr.
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Anshuman Gupta <anshuman.gupta@intel.com>
Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
drivers/gpu/drm/i915/display/intel_display_types.h | 3 +--
drivers/gpu/drm/i915/display/intel_psr.c | 11 +++++------
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 1a76e1d9de7a..f69bd1caebbf 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1002,7 +1002,6 @@ struct intel_crtc_state {
bool has_psr;
bool has_psr2;
bool enable_psr2_sel_fetch;
- u32 dc3co_exitline;
/*
* Frequence the dpll for the port should run at. Differs from the
@@ -1453,7 +1452,7 @@ struct intel_psr {
bool sink_not_reliable;
bool irq_aux_error;
u16 su_x_granularity;
- bool dc3co_enabled;
+ u32 dc3co_exitline;
u32 dc3co_exit_delay;
struct delayed_work dc3co_work;
struct drm_dp_vsc_sdp vsc;
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index cd434285e3b7..976826653143 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -637,7 +637,7 @@ static void tgl_dc3co_disable_work(struct work_struct *work)
static void tgl_disallow_dc3co_on_psr2_exit(struct intel_dp *intel_dp)
{
- if (!intel_dp->psr.dc3co_enabled)
+ if (!intel_dp->psr.dc3co_exitline)
return;
cancel_delayed_work(&intel_dp->psr.dc3co_work);
@@ -679,7 +679,7 @@ tgl_dc3co_exitline_compute_config(struct intel_dp *intel_dp,
if (drm_WARN_ON(&dev_priv->drm, exit_scanlines > crtc_vdisplay))
return;
- crtc_state->dc3co_exitline = crtc_vdisplay - exit_scanlines;
+ intel_dp->psr.dc3co_exitline = crtc_vdisplay - exit_scanlines;
}
static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
@@ -938,7 +938,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
psr_irq_control(intel_dp);
- if (crtc_state->dc3co_exitline) {
+ if (intel_dp->psr.dc3co_exitline) {
u32 val;
/*
@@ -947,7 +947,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
*/
val = intel_de_read(dev_priv, EXITLINE(cpu_transcoder));
val &= ~EXITLINE_MASK;
- val |= crtc_state->dc3co_exitline << EXITLINE_SHIFT;
+ val |= intel_dp->psr.dc3co_exitline << EXITLINE_SHIFT;
val |= EXITLINE_ENABLE;
intel_de_write(dev_priv, EXITLINE(cpu_transcoder), val);
}
@@ -972,7 +972,6 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp,
intel_dp->psr.psr2_enabled = crtc_state->has_psr2;
intel_dp->psr.busy_frontbuffer_bits = 0;
intel_dp->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
- intel_dp->psr.dc3co_enabled = !!crtc_state->dc3co_exitline;
intel_dp->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);
@@ -1761,7 +1760,7 @@ tgl_dc3co_flush(struct intel_dp *intel_dp, unsigned int frontbuffer_bits,
{
mutex_lock(&intel_dp->psr.lock);
- if (!intel_dp->psr.dc3co_enabled)
+ if (!intel_dp->psr.dc3co_exitline)
goto unlock;
if (!intel_dp->psr.psr2_enabled || !intel_dp->psr.active)
--
2.30.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Intel-gfx] [PATCH 2/3] drm/i915/display: Remove a redundant function argument from intel_psr_enable_source()
2021-03-03 16:41 [Intel-gfx] [PATCH 1/3] drm/i915/display: Move dc3co_exitline variable to struct intel_psr Gwan-gyeong Mun
@ 2021-03-03 16:42 ` Gwan-gyeong Mun
2021-03-03 17:51 ` Souza, Jose
2021-03-03 16:42 ` [Intel-gfx] [PATCH 3/3] drm/i915/display: Introduce new intel_psr_pause/resume function Gwan-gyeong Mun
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Gwan-gyeong Mun @ 2021-03-03 16:42 UTC (permalink / raw)
To: intel-gfx
It removes intel_crtc_state from function argument of
intel_psr_enable_source() in order to use intel_psr_enable_source()
without intel_crtc_state on other psr internal functions.
And we can get cpu_trancoder from intel_psr, therefore we don't need to
pass intel_crtc_state to this function.
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
drivers/gpu/drm/i915/display/intel_psr.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 976826653143..ea8f9598e6a3 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -896,11 +896,10 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
intel_dp->psr.active = true;
}
-static void intel_psr_enable_source(struct intel_dp *intel_dp,
- const struct intel_crtc_state *crtc_state)
+static void intel_psr_enable_source(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
- enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+ enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
u32 mask;
/* Only HSW and BDW have PSR AUX registers that need to be setup. SKL+
@@ -1007,7 +1006,7 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp,
&intel_dp->psr.vsc);
intel_write_dp_vsc_sdp(encoder, crtc_state, &intel_dp->psr.vsc);
intel_psr_enable_sink(intel_dp);
- intel_psr_enable_source(intel_dp, crtc_state);
+ intel_psr_enable_source(intel_dp);
intel_dp->psr.enabled = true;
intel_psr_activate(intel_dp);
--
2.30.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Intel-gfx] [PATCH 3/3] drm/i915/display: Introduce new intel_psr_pause/resume function
2021-03-03 16:41 [Intel-gfx] [PATCH 1/3] drm/i915/display: Move dc3co_exitline variable to struct intel_psr Gwan-gyeong Mun
2021-03-03 16:42 ` [Intel-gfx] [PATCH 2/3] drm/i915/display: Remove a redundant function argument from intel_psr_enable_source() Gwan-gyeong Mun
@ 2021-03-03 16:42 ` Gwan-gyeong Mun
2021-03-03 18:05 ` Souza, Jose
2021-03-03 16:54 ` [Intel-gfx] [PATCH 1/3] drm/i915/display: Move dc3co_exitline variable to struct intel_psr Ville Syrjälä
2021-03-03 19:27 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] " Patchwork
3 siblings, 1 reply; 9+ messages in thread
From: Gwan-gyeong Mun @ 2021-03-03 16:42 UTC (permalink / raw)
To: intel-gfx
This introduces the following function that can enable and disable psr
without intel_crtc_state when intel_psr is already enabled with current
intel_crtc_state information.
- intel_psr_pause(): Pause current PSR. it deactivates current psr state.
- intel_psr_resume(): Resume paused PSR without intel_crtc_state.
It activates paused psr state.
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
.../drm/i915/display/intel_display_types.h | 1 +
drivers/gpu/drm/i915/display/intel_psr.c | 111 +++++++++++++++---
drivers/gpu/drm/i915/display/intel_psr.h | 2 +
3 files changed, 97 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index f69bd1caebbf..d49b79a0691a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1454,6 +1454,7 @@ struct intel_psr {
u16 su_x_granularity;
u32 dc3co_exitline;
u32 dc3co_exit_delay;
+ bool paused;
struct delayed_work dc3co_work;
struct drm_dp_vsc_sdp vsc;
};
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index ea8f9598e6a3..533fc21f4352 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -957,26 +957,11 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp)
IGNORE_PSR2_HW_TRACKING : 0);
}
-static void intel_psr_enable_locked(struct intel_dp *intel_dp,
- const struct intel_crtc_state *crtc_state,
- const struct drm_connector_state *conn_state)
+static bool psr_interrupt_error_check(struct intel_dp *intel_dp)
{
- struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
- struct intel_encoder *encoder = &dig_port->base;
u32 val;
- drm_WARN_ON(&dev_priv->drm, intel_dp->psr.enabled);
-
- intel_dp->psr.psr2_enabled = crtc_state->has_psr2;
- intel_dp->psr.busy_frontbuffer_bits = 0;
- intel_dp->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
- intel_dp->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);
- intel_dp->psr.dc3co_exit_delay = val;
- intel_dp->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch;
-
/*
* If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
* will still keep the error set even after the reset done in the
@@ -997,9 +982,36 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp,
intel_dp->psr.sink_not_reliable = true;
drm_dbg_kms(&dev_priv->drm,
"PSR interruption error set, not enabling PSR\n");
- return;
+ return false;
}
+ return true;
+}
+
+static void intel_psr_enable_locked(struct intel_dp *intel_dp,
+ const struct intel_crtc_state *crtc_state,
+ const struct drm_connector_state *conn_state)
+{
+ struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+ struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+ struct intel_encoder *encoder = &dig_port->base;
+ u32 val;
+
+ drm_WARN_ON(&dev_priv->drm, intel_dp->psr.enabled);
+
+ intel_dp->psr.psr2_enabled = crtc_state->has_psr2;
+ intel_dp->psr.busy_frontbuffer_bits = 0;
+ intel_dp->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
+ intel_dp->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);
+ intel_dp->psr.dc3co_exit_delay = val;
+ intel_dp->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch;
+ intel_dp->psr.paused = false;
+
+ if (!psr_interrupt_error_check(intel_dp))
+ return;
+
drm_dbg_kms(&dev_priv->drm, "Enabling PSR%s\n",
intel_dp->psr.psr2_enabled ? "2" : "1");
intel_dp_compute_psr_vsc_sdp(intel_dp, crtc_state, conn_state,
@@ -1149,6 +1161,71 @@ void intel_psr_disable(struct intel_dp *intel_dp,
cancel_delayed_work_sync(&intel_dp->psr.dc3co_work);
}
+/**
+ * intel_psr_pause - Pause PSR
+ * @intel_dp: Intel DP
+ *
+ * This function need to be called after enabling psr.
+ */
+void intel_psr_pause(struct intel_dp *intel_dp)
+{
+ struct intel_psr *psr = &intel_dp->psr;
+
+ if (!CAN_PSR(intel_dp))
+ return;
+
+ mutex_lock(&psr->lock);
+
+ if (!psr->enabled || psr->paused) {
+ mutex_unlock(&psr->lock);
+ return;
+ }
+
+ intel_psr_disable_locked(intel_dp);
+ psr->paused = true;
+
+ mutex_unlock(&psr->lock);
+
+ cancel_work_sync(&psr->work);
+ cancel_delayed_work_sync(&psr->dc3co_work);
+}
+
+/**
+ * intel_psr_resume - Resume PSR
+ * @intel_dp: Intel DP
+ *
+ * This function need to be called after pausing psr.
+ */
+void intel_psr_resume(struct intel_dp *intel_dp)
+{
+ struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+ struct intel_psr *psr = &intel_dp->psr;
+
+ if (!CAN_PSR(intel_dp))
+ return;
+
+ mutex_lock(&psr->lock);
+
+ if (psr->enabled || !psr->paused)
+ goto unlock;
+
+ psr->paused = false;
+
+ if (!psr_interrupt_error_check(intel_dp))
+ goto unlock;
+
+ drm_dbg_kms(&dev_priv->drm, "Enabling PSR%s\n",
+ psr->psr2_enabled ? "2" : "1");
+ intel_psr_enable_sink(intel_dp);
+ intel_psr_enable_source(intel_dp);
+ intel_dp->psr.enabled = true;
+
+ intel_psr_activate(intel_dp);
+
+unlock:
+ mutex_unlock(&psr->lock);
+}
+
static void psr_force_hw_tracking_exit(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h
index 0491a49ffd50..8cc5e78fb1d2 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.h
+++ b/drivers/gpu/drm/i915/display/intel_psr.h
@@ -48,5 +48,7 @@ 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);
+void intel_psr_pause(struct intel_dp *intel_dp);
+void intel_psr_resume(struct intel_dp *intel_dp);
#endif /* __INTEL_PSR_H__ */
--
2.30.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH 1/3] drm/i915/display: Move dc3co_exitline variable to struct intel_psr
2021-03-03 16:41 [Intel-gfx] [PATCH 1/3] drm/i915/display: Move dc3co_exitline variable to struct intel_psr Gwan-gyeong Mun
2021-03-03 16:42 ` [Intel-gfx] [PATCH 2/3] drm/i915/display: Remove a redundant function argument from intel_psr_enable_source() Gwan-gyeong Mun
2021-03-03 16:42 ` [Intel-gfx] [PATCH 3/3] drm/i915/display: Introduce new intel_psr_pause/resume function Gwan-gyeong Mun
@ 2021-03-03 16:54 ` Ville Syrjälä
2021-03-05 20:00 ` Mun, Gwan-gyeong
2021-03-03 19:27 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] " Patchwork
3 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2021-03-03 16:54 UTC (permalink / raw)
To: Gwan-gyeong Mun; +Cc: intel-gfx
On Wed, Mar 03, 2021 at 06:41:59PM +0200, Gwan-gyeong Mun wrote:
> dc3co_exitline is indirectly called by intel_psr_compute_config().
> And it will not be changed until the next calling of
> intel_psr_compute_config(). So in order to use dc3co_exitline without
> intel_crtc_state on other psr internal function, it moves dc3co_exitline
> variable to struct intel_psr.
> And it removes a dc3co_enabled variable from struct intel_psr.
>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display_types.h | 3 +--
> drivers/gpu/drm/i915/display/intel_psr.c | 11 +++++------
> 2 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 1a76e1d9de7a..f69bd1caebbf 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1002,7 +1002,6 @@ struct intel_crtc_state {
> bool has_psr;
> bool has_psr2;
> bool enable_psr2_sel_fetch;
> - u32 dc3co_exitline;
>
> /*
> * Frequence the dpll for the port should run at. Differs from the
> @@ -1453,7 +1452,7 @@ struct intel_psr {
> bool sink_not_reliable;
> bool irq_aux_error;
> u16 su_x_granularity;
> - bool dc3co_enabled;
> + u32 dc3co_exitline;
> u32 dc3co_exit_delay;
> struct delayed_work dc3co_work;
> struct drm_dp_vsc_sdp vsc;
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index cd434285e3b7..976826653143 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -637,7 +637,7 @@ static void tgl_dc3co_disable_work(struct work_struct *work)
>
> static void tgl_disallow_dc3co_on_psr2_exit(struct intel_dp *intel_dp)
> {
> - if (!intel_dp->psr.dc3co_enabled)
> + if (!intel_dp->psr.dc3co_exitline)
> return;
>
> cancel_delayed_work(&intel_dp->psr.dc3co_work);
> @@ -679,7 +679,7 @@ tgl_dc3co_exitline_compute_config(struct intel_dp *intel_dp,
> if (drm_WARN_ON(&dev_priv->drm, exit_scanlines > crtc_vdisplay))
> return;
>
> - crtc_state->dc3co_exitline = crtc_vdisplay - exit_scanlines;
> + intel_dp->psr.dc3co_exitline = crtc_vdisplay - exit_scanlines;
Thou shalt not mutate externally visible state in .compute_config().
You either want to make a copy of it or just compute it on the spot in
the psr_enable(). The first option is a good choice when you can also
hook into into the readout+state checker.
> }
>
> static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
> @@ -938,7 +938,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
>
> psr_irq_control(intel_dp);
>
> - if (crtc_state->dc3co_exitline) {
> + if (intel_dp->psr.dc3co_exitline) {
> u32 val;
>
> /*
> @@ -947,7 +947,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
> */
> val = intel_de_read(dev_priv, EXITLINE(cpu_transcoder));
> val &= ~EXITLINE_MASK;
> - val |= crtc_state->dc3co_exitline << EXITLINE_SHIFT;
> + val |= intel_dp->psr.dc3co_exitline << EXITLINE_SHIFT;
> val |= EXITLINE_ENABLE;
> intel_de_write(dev_priv, EXITLINE(cpu_transcoder), val);
> }
> @@ -972,7 +972,6 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp,
> intel_dp->psr.psr2_enabled = crtc_state->has_psr2;
> intel_dp->psr.busy_frontbuffer_bits = 0;
> intel_dp->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
> - intel_dp->psr.dc3co_enabled = !!crtc_state->dc3co_exitline;
> intel_dp->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);
> @@ -1761,7 +1760,7 @@ tgl_dc3co_flush(struct intel_dp *intel_dp, unsigned int frontbuffer_bits,
> {
> mutex_lock(&intel_dp->psr.lock);
>
> - if (!intel_dp->psr.dc3co_enabled)
> + if (!intel_dp->psr.dc3co_exitline)
> goto unlock;
>
> if (!intel_dp->psr.psr2_enabled || !intel_dp->psr.active)
> --
> 2.30.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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] 9+ messages in thread
* Re: [Intel-gfx] [PATCH 2/3] drm/i915/display: Remove a redundant function argument from intel_psr_enable_source()
2021-03-03 16:42 ` [Intel-gfx] [PATCH 2/3] drm/i915/display: Remove a redundant function argument from intel_psr_enable_source() Gwan-gyeong Mun
@ 2021-03-03 17:51 ` Souza, Jose
0 siblings, 0 replies; 9+ messages in thread
From: Souza, Jose @ 2021-03-03 17:51 UTC (permalink / raw)
To: Mun, Gwan-gyeong, intel-gfx
On Wed, 2021-03-03 at 18:42 +0200, Gwan-gyeong Mun wrote:
> It removes intel_crtc_state from function argument of
> intel_psr_enable_source() in order to use intel_psr_enable_source()
> without intel_crtc_state on other psr internal functions.
> And we can get cpu_trancoder from intel_psr, therefore we don't need to
> pass intel_crtc_state to this function.
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 976826653143..ea8f9598e6a3 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -896,11 +896,10 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
> intel_dp->psr.active = true;
> }
>
>
>
>
> -static void intel_psr_enable_source(struct intel_dp *intel_dp,
> - const struct intel_crtc_state *crtc_state)
> +static void intel_psr_enable_source(struct intel_dp *intel_dp)
> {
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> - enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> + enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
> u32 mask;
>
>
>
>
> /* Only HSW and BDW have PSR AUX registers that need to be setup. SKL+
> @@ -1007,7 +1006,7 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp,
> &intel_dp->psr.vsc);
> intel_write_dp_vsc_sdp(encoder, crtc_state, &intel_dp->psr.vsc);
> intel_psr_enable_sink(intel_dp);
> - intel_psr_enable_source(intel_dp, crtc_state);
> + intel_psr_enable_source(intel_dp);
> intel_dp->psr.enabled = true;
>
>
>
>
> intel_psr_activate(intel_dp);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH 3/3] drm/i915/display: Introduce new intel_psr_pause/resume function
2021-03-03 16:42 ` [Intel-gfx] [PATCH 3/3] drm/i915/display: Introduce new intel_psr_pause/resume function Gwan-gyeong Mun
@ 2021-03-03 18:05 ` Souza, Jose
2021-03-05 20:01 ` Mun, Gwan-gyeong
0 siblings, 1 reply; 9+ messages in thread
From: Souza, Jose @ 2021-03-03 18:05 UTC (permalink / raw)
To: Mun, Gwan-gyeong, intel-gfx
patch 1 is a nack for the reasons that Ville explained.
This one could be simplified even more.
intel_psr_enable_locked() should have all the dev_priv->psr.* initialization from crtc_state + intel_dp_compute_psr_vsc_sdp().
Then add a function(_intel_psr_enable_locked() or other better name that you can think) with the error checking + intel_write_dp_vsc_sdp() +
intel_psr_enable_sink() + intel_psr_enable_source() + intel_psr_activate()...
intel_psr_resume()
take loock
checks
_intel_psr_enable_locked()
unlock()
On Wed, 2021-03-03 at 18:42 +0200, Gwan-gyeong Mun wrote:
> This introduces the following function that can enable and disable psr
> without intel_crtc_state when intel_psr is already enabled with current
> intel_crtc_state information.
>
> - intel_psr_pause(): Pause current PSR. it deactivates current psr state.
> - intel_psr_resume(): Resume paused PSR without intel_crtc_state.
> It activates paused psr state.
>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> ---
> .../drm/i915/display/intel_display_types.h | 1 +
> drivers/gpu/drm/i915/display/intel_psr.c | 111 +++++++++++++++---
> drivers/gpu/drm/i915/display/intel_psr.h | 2 +
> 3 files changed, 97 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index f69bd1caebbf..d49b79a0691a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1454,6 +1454,7 @@ struct intel_psr {
> u16 su_x_granularity;
> u32 dc3co_exitline;
> u32 dc3co_exit_delay;
> + bool paused;
> struct delayed_work dc3co_work;
> struct drm_dp_vsc_sdp vsc;
> };
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index ea8f9598e6a3..533fc21f4352 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -957,26 +957,11 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp)
> IGNORE_PSR2_HW_TRACKING : 0);
> }
>
>
> -static void intel_psr_enable_locked(struct intel_dp *intel_dp,
> - const struct intel_crtc_state *crtc_state,
> - const struct drm_connector_state *conn_state)
> +static bool psr_interrupt_error_check(struct intel_dp *intel_dp)
> {
> - struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> - struct intel_encoder *encoder = &dig_port->base;
> u32 val;
>
>
> - drm_WARN_ON(&dev_priv->drm, intel_dp->psr.enabled);
> -
> - intel_dp->psr.psr2_enabled = crtc_state->has_psr2;
> - intel_dp->psr.busy_frontbuffer_bits = 0;
> - intel_dp->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
> - intel_dp->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);
> - intel_dp->psr.dc3co_exit_delay = val;
> - intel_dp->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch;
> -
> /*
> * If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR
> * will still keep the error set even after the reset done in the
> @@ -997,9 +982,36 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp,
> intel_dp->psr.sink_not_reliable = true;
> drm_dbg_kms(&dev_priv->drm,
> "PSR interruption error set, not enabling PSR\n");
> - return;
> + return false;
> }
>
>
> + return true;
> +}
> +
> +static void intel_psr_enable_locked(struct intel_dp *intel_dp,
> + const struct intel_crtc_state *crtc_state,
> + const struct drm_connector_state *conn_state)
> +{
> + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> + struct intel_encoder *encoder = &dig_port->base;
> + u32 val;
> +
> + drm_WARN_ON(&dev_priv->drm, intel_dp->psr.enabled);
> +
> + intel_dp->psr.psr2_enabled = crtc_state->has_psr2;
> + intel_dp->psr.busy_frontbuffer_bits = 0;
> + intel_dp->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
> + intel_dp->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);
> + intel_dp->psr.dc3co_exit_delay = val;
> + intel_dp->psr.psr2_sel_fetch_enabled = crtc_state->enable_psr2_sel_fetch;
> + intel_dp->psr.paused = false;
> +
> + if (!psr_interrupt_error_check(intel_dp))
> + return;
> +
> drm_dbg_kms(&dev_priv->drm, "Enabling PSR%s\n",
> intel_dp->psr.psr2_enabled ? "2" : "1");
> intel_dp_compute_psr_vsc_sdp(intel_dp, crtc_state, conn_state,
> @@ -1149,6 +1161,71 @@ void intel_psr_disable(struct intel_dp *intel_dp,
> cancel_delayed_work_sync(&intel_dp->psr.dc3co_work);
> }
>
>
> +/**
> + * intel_psr_pause - Pause PSR
> + * @intel_dp: Intel DP
> + *
> + * This function need to be called after enabling psr.
> + */
> +void intel_psr_pause(struct intel_dp *intel_dp)
> +{
> + struct intel_psr *psr = &intel_dp->psr;
> +
> + if (!CAN_PSR(intel_dp))
> + return;
> +
> + mutex_lock(&psr->lock);
> +
> + if (!psr->enabled || psr->paused) {
> + mutex_unlock(&psr->lock);
> + return;
> + }
> +
> + intel_psr_disable_locked(intel_dp);
> + psr->paused = true;
> +
> + mutex_unlock(&psr->lock);
> +
> + cancel_work_sync(&psr->work);
> + cancel_delayed_work_sync(&psr->dc3co_work);
> +}
> +
> +/**
> + * intel_psr_resume - Resume PSR
> + * @intel_dp: Intel DP
> + *
> + * This function need to be called after pausing psr.
> + */
> +void intel_psr_resume(struct intel_dp *intel_dp)
> +{
> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> + struct intel_psr *psr = &intel_dp->psr;
> +
> + if (!CAN_PSR(intel_dp))
> + return;
> +
> + mutex_lock(&psr->lock);
> +
> + if (psr->enabled || !psr->paused)
> + goto unlock;
> +
> + psr->paused = false;
> +
> + if (!psr_interrupt_error_check(intel_dp))
> + goto unlock;
> +
> + drm_dbg_kms(&dev_priv->drm, "Enabling PSR%s\n",
> + psr->psr2_enabled ? "2" : "1");
> + intel_psr_enable_sink(intel_dp);
> + intel_psr_enable_source(intel_dp);
> + intel_dp->psr.enabled = true;
> +
> + intel_psr_activate(intel_dp);
> +
> +unlock:
> + mutex_unlock(&psr->lock);
> +}
> +
> static void psr_force_hw_tracking_exit(struct intel_dp *intel_dp)
> {
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h
> index 0491a49ffd50..8cc5e78fb1d2 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> @@ -48,5 +48,7 @@ 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);
> +void intel_psr_pause(struct intel_dp *intel_dp);
> +void intel_psr_resume(struct intel_dp *intel_dp);
>
>
> #endif /* __INTEL_PSR_H__ */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915/display: Move dc3co_exitline variable to struct intel_psr
2021-03-03 16:41 [Intel-gfx] [PATCH 1/3] drm/i915/display: Move dc3co_exitline variable to struct intel_psr Gwan-gyeong Mun
` (2 preceding siblings ...)
2021-03-03 16:54 ` [Intel-gfx] [PATCH 1/3] drm/i915/display: Move dc3co_exitline variable to struct intel_psr Ville Syrjälä
@ 2021-03-03 19:27 ` Patchwork
3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2021-03-03 19:27 UTC (permalink / raw)
To: Gwan-gyeong Mun; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 5181 bytes --]
== Series Details ==
Series: series starting with [1/3] drm/i915/display: Move dc3co_exitline variable to struct intel_psr
URL : https://patchwork.freedesktop.org/series/87596/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_9824 -> Patchwork_19751
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_19751 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_19751, 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_19751/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_19751:
### IGT changes ###
#### Possible regressions ####
* igt@gem_wait@busy@all:
- fi-bsw-nick: [PASS][1] -> [FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9824/fi-bsw-nick/igt@gem_wait@busy@all.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19751/fi-bsw-nick/igt@gem_wait@busy@all.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@kms_frontbuffer_tracking@basic:
- {fi-rkl-11500t}: [PASS][3] -> [SKIP][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9824/fi-rkl-11500t/igt@kms_frontbuffer_tracking@basic.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19751/fi-rkl-11500t/igt@kms_frontbuffer_tracking@basic.html
Known issues
------------
Here are the changes found in Patchwork_19751 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@amdgpu/amd_cs_nop@sync-fork-compute0:
- fi-snb-2600: NOTRUN -> [SKIP][5] ([fdo#109271]) +17 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19751/fi-snb-2600/igt@amdgpu/amd_cs_nop@sync-fork-compute0.html
* igt@gem_linear_blits@basic:
- fi-kbl-8809g: [PASS][6] -> [TIMEOUT][7] ([i915#2502])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9824/fi-kbl-8809g/igt@gem_linear_blits@basic.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19751/fi-kbl-8809g/igt@gem_linear_blits@basic.html
* igt@gem_tiled_fence_blits@basic:
- fi-kbl-8809g: [PASS][8] -> [TIMEOUT][9] ([i915#3145])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9824/fi-kbl-8809g/igt@gem_tiled_fence_blits@basic.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19751/fi-kbl-8809g/igt@gem_tiled_fence_blits@basic.html
#### Possible fixes ####
* igt@gem_exec_gttfill@basic:
- fi-kbl-8809g: [TIMEOUT][10] ([i915#3145]) -> [PASS][11]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9824/fi-kbl-8809g/igt@gem_exec_gttfill@basic.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19751/fi-kbl-8809g/igt@gem_exec_gttfill@basic.html
* igt@i915_selftest@live@hangcheck:
- fi-snb-2600: [INCOMPLETE][12] ([i915#2782]) -> [PASS][13]
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9824/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19751/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
* igt@kms_chamelium@dp-crc-fast:
- fi-kbl-7500u: [FAIL][14] ([i915#1372]) -> [PASS][15]
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9824/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19751/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[i915#1372]: https://gitlab.freedesktop.org/drm/intel/issues/1372
[i915#2502]: https://gitlab.freedesktop.org/drm/intel/issues/2502
[i915#2782]: https://gitlab.freedesktop.org/drm/intel/issues/2782
[i915#3145]: https://gitlab.freedesktop.org/drm/intel/issues/3145
Participating hosts (42 -> 37)
------------------------------
Missing (5): fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_9824 -> Patchwork_19751
CI-20190529: 20190529
CI_DRM_9824: 6b21aa2a30637b56282297d09a9fbfd1954cc7b0 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_6020: 8382d9e87bba39ecc6fa879b2491e28c7fb2e3c7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_19751: 81b7380be9637dce2e4685b4cfcfe465c692b9db @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
81b7380be963 drm/i915/display: Introduce new intel_psr_pause/resume function
027c24831154 drm/i915/display: Remove a redundant function argument from intel_psr_enable_source()
64a484d3e434 drm/i915/display: Move dc3co_exitline variable to struct intel_psr
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19751/index.html
[-- Attachment #1.2: Type: text/html, Size: 6089 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] 9+ messages in thread
* Re: [Intel-gfx] [PATCH 1/3] drm/i915/display: Move dc3co_exitline variable to struct intel_psr
2021-03-03 16:54 ` [Intel-gfx] [PATCH 1/3] drm/i915/display: Move dc3co_exitline variable to struct intel_psr Ville Syrjälä
@ 2021-03-05 20:00 ` Mun, Gwan-gyeong
0 siblings, 0 replies; 9+ messages in thread
From: Mun, Gwan-gyeong @ 2021-03-05 20:00 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Wed, 2021-03-03 at 18:54 +0200, Ville Syrjälä wrote:
> On Wed, Mar 03, 2021 at 06:41:59PM +0200, Gwan-gyeong Mun wrote:
> > dc3co_exitline is indirectly called by intel_psr_compute_config().
> > And it will not be changed until the next calling of
> > intel_psr_compute_config(). So in order to use dc3co_exitline without
> > intel_crtc_state on other psr internal function, it moves
> > dc3co_exitline
> > variable to struct intel_psr.
> > And it removes a dc3co_enabled variable from struct intel_psr.
> >
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_display_types.h | 3 +--
> > drivers/gpu/drm/i915/display/intel_psr.c | 11 +++++------
> > 2 files changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 1a76e1d9de7a..f69bd1caebbf 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1002,7 +1002,6 @@ struct intel_crtc_state {
> > bool has_psr;
> > bool has_psr2;
> > bool enable_psr2_sel_fetch;
> > - u32 dc3co_exitline;
> >
> > /*
> > * Frequence the dpll for the port should run at. Differs
> > from the
> > @@ -1453,7 +1452,7 @@ struct intel_psr {
> > bool sink_not_reliable;
> > bool irq_aux_error;
> > u16 su_x_granularity;
> > - bool dc3co_enabled;
> > + u32 dc3co_exitline;
> > u32 dc3co_exit_delay;
> > struct delayed_work dc3co_work;
> > struct drm_dp_vsc_sdp vsc;
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index cd434285e3b7..976826653143 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -637,7 +637,7 @@ static void tgl_dc3co_disable_work(struct
> > work_struct *work)
> >
> > static void tgl_disallow_dc3co_on_psr2_exit(struct intel_dp
> > *intel_dp)
> > {
> > - if (!intel_dp->psr.dc3co_enabled)
> > + if (!intel_dp->psr.dc3co_exitline)
> > return;
> >
> > cancel_delayed_work(&intel_dp->psr.dc3co_work);
> > @@ -679,7 +679,7 @@ tgl_dc3co_exitline_compute_config(struct intel_dp
> > *intel_dp,
> > if (drm_WARN_ON(&dev_priv->drm, exit_scanlines >
> > crtc_vdisplay))
> > return;
> >
> > - crtc_state->dc3co_exitline = crtc_vdisplay - exit_scanlines;
> > + intel_dp->psr.dc3co_exitline = crtc_vdisplay -
> > exit_scanlines;
>
> Thou shalt not mutate externally visible state in .compute_config().
> You either want to make a copy of it or just compute it on the spot in
> the psr_enable(). The first option is a good choice when you can also
> hook into into the readout+state checker.
>
Hi, thanks for pointing out where I was approached incorrectly.
I'll float new patch that makes copy crtc_state's dc3co_exitline to
intel_psr.dc3co_exitline.
> > }
> >
> > static bool intel_psr2_sel_fetch_config_valid(struct intel_dp
> > *intel_dp,
> > @@ -938,7 +938,7 @@ static void intel_psr_enable_source(struct
> > intel_dp *intel_dp,
> >
> > psr_irq_control(intel_dp);
> >
> > - if (crtc_state->dc3co_exitline) {
> > + if (intel_dp->psr.dc3co_exitline) {
> > u32 val;
> >
> > /*
> > @@ -947,7 +947,7 @@ static void intel_psr_enable_source(struct
> > intel_dp *intel_dp,
> > */
> > val = intel_de_read(dev_priv,
> > EXITLINE(cpu_transcoder));
> > val &= ~EXITLINE_MASK;
> > - val |= crtc_state->dc3co_exitline << EXITLINE_SHIFT;
> > + val |= intel_dp->psr.dc3co_exitline <<
> > EXITLINE_SHIFT;
> > val |= EXITLINE_ENABLE;
> > intel_de_write(dev_priv, EXITLINE(cpu_transcoder),
> > val);
> > }
> > @@ -972,7 +972,6 @@ static void intel_psr_enable_locked(struct
> > intel_dp *intel_dp,
> > intel_dp->psr.psr2_enabled = crtc_state->has_psr2;
> > intel_dp->psr.busy_frontbuffer_bits = 0;
> > intel_dp->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)-
> > >pipe;
> > - intel_dp->psr.dc3co_enabled = !!crtc_state->dc3co_exitline;
> > intel_dp->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);
> > @@ -1761,7 +1760,7 @@ tgl_dc3co_flush(struct intel_dp *intel_dp,
> > unsigned int frontbuffer_bits,
> > {
> > mutex_lock(&intel_dp->psr.lock);
> >
> > - if (!intel_dp->psr.dc3co_enabled)
> > + if (!intel_dp->psr.dc3co_exitline)
> > goto unlock;
> >
> > if (!intel_dp->psr.psr2_enabled || !intel_dp->psr.active)
> > --
> > 2.30.1
> >
> > _______________________________________________
> > 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] 9+ messages in thread
* Re: [Intel-gfx] [PATCH 3/3] drm/i915/display: Introduce new intel_psr_pause/resume function
2021-03-03 18:05 ` Souza, Jose
@ 2021-03-05 20:01 ` Mun, Gwan-gyeong
0 siblings, 0 replies; 9+ messages in thread
From: Mun, Gwan-gyeong @ 2021-03-05 20:01 UTC (permalink / raw)
To: intel-gfx, Souza, Jose
On Wed, 2021-03-03 at 10:05 -0800, Souza, Jose wrote:
> patch 1 is a nack for the reasons that Ville explained.
>
> This one could be simplified even more.
>
> intel_psr_enable_locked() should have all the dev_priv->psr.*
> initialization from crtc_state + intel_dp_compute_psr_vsc_sdp().
> Then add a function(_intel_psr_enable_locked() or other better name
> that you can think) with the error checking + intel_write_dp_vsc_sdp()
> +
> intel_psr_enable_sink() + intel_psr_enable_source() +
> intel_psr_activate()...
>
> intel_psr_resume()
> take loock
> checks
> _intel_psr_enable_locked()
> unlock()
>
>
Hi, thanks for checking it.
I'll float a new patch that addresses your comments.
>
> On Wed, 2021-03-03 at 18:42 +0200, Gwan-gyeong Mun wrote:
> > This introduces the following function that can enable and disable
> > psr
> > without intel_crtc_state when intel_psr is already enabled with
> > current
> > intel_crtc_state information.
> >
> > - intel_psr_pause(): Pause current PSR. it deactivates current psr
> > state.
> > - intel_psr_resume(): Resume paused PSR without intel_crtc_state.
> > It activates paused psr state.
> >
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > ---
> > .../drm/i915/display/intel_display_types.h | 1 +
> > drivers/gpu/drm/i915/display/intel_psr.c | 111 +++++++++++++++-
> > --
> > drivers/gpu/drm/i915/display/intel_psr.h | 2 +
> > 3 files changed, 97 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index f69bd1caebbf..d49b79a0691a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1454,6 +1454,7 @@ struct intel_psr {
> > u16 su_x_granularity;
> > u32 dc3co_exitline;
> > u32 dc3co_exit_delay;
> > + bool paused;
> > struct delayed_work dc3co_work;
> > struct drm_dp_vsc_sdp vsc;
> > };
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index ea8f9598e6a3..533fc21f4352 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -957,26 +957,11 @@ static void intel_psr_enable_source(struct
> > intel_dp *intel_dp)
> > IGNORE_PSR2_HW_TRACKING : 0);
> > }
> >
> >
> > -static void intel_psr_enable_locked(struct intel_dp *intel_dp,
> > - const struct intel_crtc_state
> > *crtc_state,
> > - const struct drm_connector_state
> > *conn_state)
> > +static bool psr_interrupt_error_check(struct intel_dp *intel_dp)
> > {
> > - struct intel_digital_port *dig_port =
> > dp_to_dig_port(intel_dp);
> > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > - struct intel_encoder *encoder = &dig_port->base;
> > u32 val;
> >
> >
> > - drm_WARN_ON(&dev_priv->drm, intel_dp->psr.enabled);
> > -
> > - intel_dp->psr.psr2_enabled = crtc_state->has_psr2;
> > - intel_dp->psr.busy_frontbuffer_bits = 0;
> > - intel_dp->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)-
> > >pipe;
> > - intel_dp->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);
> > - intel_dp->psr.dc3co_exit_delay = val;
> > - intel_dp->psr.psr2_sel_fetch_enabled = crtc_state-
> > >enable_psr2_sel_fetch;
> > -
> > /*
> > * If a PSR error happened and the driver is reloaded, the
> > EDP_PSR_IIR
> > * will still keep the error set even after the reset done in
> > the
> > @@ -997,9 +982,36 @@ static void intel_psr_enable_locked(struct
> > intel_dp *intel_dp,
> > intel_dp->psr.sink_not_reliable = true;
> > drm_dbg_kms(&dev_priv->drm,
> > "PSR interruption error set, not enabling
> > PSR\n");
> > - return;
> > + return false;
> > }
> >
> >
> > + return true;
> > +}
> > +
> > +static void intel_psr_enable_locked(struct intel_dp *intel_dp,
> > + const struct intel_crtc_state
> > *crtc_state,
> > + const struct drm_connector_state
> > *conn_state)
> > +{
> > + struct intel_digital_port *dig_port =
> > dp_to_dig_port(intel_dp);
> > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > + struct intel_encoder *encoder = &dig_port->base;
> > + u32 val;
> > +
> > + drm_WARN_ON(&dev_priv->drm, intel_dp->psr.enabled);
> > +
> > + intel_dp->psr.psr2_enabled = crtc_state->has_psr2;
> > + intel_dp->psr.busy_frontbuffer_bits = 0;
> > + intel_dp->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)-
> > >pipe;
> > + intel_dp->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);
> > + intel_dp->psr.dc3co_exit_delay = val;
> > + intel_dp->psr.psr2_sel_fetch_enabled = crtc_state-
> > >enable_psr2_sel_fetch;
> > + intel_dp->psr.paused = false;
> > +
> > + if (!psr_interrupt_error_check(intel_dp))
> > + return;
> > +
> > drm_dbg_kms(&dev_priv->drm, "Enabling PSR%s\n",
> > intel_dp->psr.psr2_enabled ? "2" : "1");
> > intel_dp_compute_psr_vsc_sdp(intel_dp, crtc_state,
> > conn_state,
> > @@ -1149,6 +1161,71 @@ void intel_psr_disable(struct intel_dp
> > *intel_dp,
> > cancel_delayed_work_sync(&intel_dp->psr.dc3co_work);
> > }
> >
> >
> > +/**
> > + * intel_psr_pause - Pause PSR
> > + * @intel_dp: Intel DP
> > + *
> > + * This function need to be called after enabling psr.
> > + */
> > +void intel_psr_pause(struct intel_dp *intel_dp)
> > +{
> > + struct intel_psr *psr = &intel_dp->psr;
> > +
> > + if (!CAN_PSR(intel_dp))
> > + return;
> > +
> > + mutex_lock(&psr->lock);
> > +
> > + if (!psr->enabled || psr->paused) {
> > + mutex_unlock(&psr->lock);
> > + return;
> > + }
> > +
> > + intel_psr_disable_locked(intel_dp);
> > + psr->paused = true;
> > +
> > + mutex_unlock(&psr->lock);
> > +
> > + cancel_work_sync(&psr->work);
> > + cancel_delayed_work_sync(&psr->dc3co_work);
> > +}
> > +
> > +/**
> > + * intel_psr_resume - Resume PSR
> > + * @intel_dp: Intel DP
> > + *
> > + * This function need to be called after pausing psr.
> > + */
> > +void intel_psr_resume(struct intel_dp *intel_dp)
> > +{
> > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > + struct intel_psr *psr = &intel_dp->psr;
> > +
> > + if (!CAN_PSR(intel_dp))
> > + return;
> > +
> > + mutex_lock(&psr->lock);
> > +
> > + if (psr->enabled || !psr->paused)
> > + goto unlock;
> > +
> > + psr->paused = false;
> > +
> > + if (!psr_interrupt_error_check(intel_dp))
> > + goto unlock;
> > +
> > + drm_dbg_kms(&dev_priv->drm, "Enabling PSR%s\n",
> > + psr->psr2_enabled ? "2" : "1");
> > + intel_psr_enable_sink(intel_dp);
> > + intel_psr_enable_source(intel_dp);
> > + intel_dp->psr.enabled = true;
> > +
> > + intel_psr_activate(intel_dp);
> > +
> > +unlock:
> > + mutex_unlock(&psr->lock);
> > +}
> > +
> > static void psr_force_hw_tracking_exit(struct intel_dp *intel_dp)
> > {
> > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> > b/drivers/gpu/drm/i915/display/intel_psr.h
> > index 0491a49ffd50..8cc5e78fb1d2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.h
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> > @@ -48,5 +48,7 @@ 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);
> > +void intel_psr_pause(struct intel_dp *intel_dp);
> > +void intel_psr_resume(struct intel_dp *intel_dp);
> >
> >
> > #endif /* __INTEL_PSR_H__ */
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-03-05 20:02 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-03 16:41 [Intel-gfx] [PATCH 1/3] drm/i915/display: Move dc3co_exitline variable to struct intel_psr Gwan-gyeong Mun
2021-03-03 16:42 ` [Intel-gfx] [PATCH 2/3] drm/i915/display: Remove a redundant function argument from intel_psr_enable_source() Gwan-gyeong Mun
2021-03-03 17:51 ` Souza, Jose
2021-03-03 16:42 ` [Intel-gfx] [PATCH 3/3] drm/i915/display: Introduce new intel_psr_pause/resume function Gwan-gyeong Mun
2021-03-03 18:05 ` Souza, Jose
2021-03-05 20:01 ` Mun, Gwan-gyeong
2021-03-03 16:54 ` [Intel-gfx] [PATCH 1/3] drm/i915/display: Move dc3co_exitline variable to struct intel_psr Ville Syrjälä
2021-03-05 20:00 ` Mun, Gwan-gyeong
2021-03-03 19:27 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] " Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).