Re: [Intel-gfx] [PATCH v13 1/2] drm/i915/display: Support PSR Multiple Instances
On Wed, 2021-01-27 at 19:23 +0200, Gwan-gyeong Mun wrote:
> It is a preliminary work for supporting multiple EDP PSR and
> DP PanelReplay. And it refactors singleton PSR to Multi Transcoder
> supportable PSR.
> And this moves and renames the i915_psr structure of drm_i915_private's to
> intel_dp's intel_psr structure.
> It also causes changes in PSR interrupt handling routine for supporting
> multiple transcoders. But it does not change the scenario and timing of
> enabling and disabling PSR. And it not support multiple pipes with
> a single transcoder PSR case yet.
>
> v2: Fix indentation and add comments
> v3: Remove Blank line
> v4: Rebased
> v5: Rebased and Addressed Anshuman's review comment.
> - Move calling of intel_psr_init() to intel_dp_init_connector()
> v6: Address Anshuman's review comments
>- Remove wrong comments and add comments for a limit of supporting of
> a single pipe PSR
> v7: Update intel_psr_compute_config() for supporting multiple transcoder
> PSR on BDW+
> v8: Address Anshuman's review comments
>- Replace DRM_DEBUG_KMS with drm_dbg_kms() / DRM_WARN with drm_warn()
> v9: Fix commit message
> v10: Rebased
> v11: Address Jose's review comment.
> - Reorder calling order of intel_psr2_program_trans_man_trk_ctl().
> - In order to reduce changes keep the old name for drm_i915_private.
> - Change restrictions of multiple instances of PSR.
> v12: Address Jose's review comment.
> - Change the calling of intel_psr2_program_trans_man_trk_ctl() into
> commit_pipe_config().
> - Change a checking order of CAN_PSR() and connector_status to original
> on i915_psr_sink_status_show().
> - Drop unneeded intel_dp_update_pipe() function.
> - In order to wait a specific encoder which belong to crtc_state on
> intel_psr_wait_for_idle(), add checking of encoder.
> - Add an whitespace to comments.
> v13: Rebased and Address Jose's review comment.
> - Add and use for_each_intel_psr_enabled_encoder() macro.
> - In order to use correct frontbuffer_bit for each pipe,
> fix intel_psr_invalidate() and intel_psr_flush().
> - Remove redundant or unneeded codes.
> - Update comments.
>
> Signed-off-by: Gwan-gyeong Mun
> Cc: José Roberto de Souza
> Cc: Juha-Pekka Heikkila
> Cc: Anshuman Gupta
> Reviewed-by: Anshuman Gupta
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 2 -
> drivers/gpu/drm/i915/display/intel_display.h | 8 +
> .../drm/i915/display/intel_display_debugfs.c | 105 +++-
> .../drm/i915/display/intel_display_types.h| 46 ++
> drivers/gpu/drm/i915/display/intel_dp.c | 10 +-
> drivers/gpu/drm/i915/display/intel_psr.c | 576 ++
> drivers/gpu/drm/i915/display/intel_psr.h | 11 +-
> drivers/gpu/drm/i915/display/intel_sprite.c | 6 +-
> drivers/gpu/drm/i915/i915_drv.h | 38 --
> drivers/gpu/drm/i915/i915_irq.c | 48 +-
> 10 files changed, 485 insertions(+), 365 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 667d941878a3..24df9dfb57fb 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14135,8 +14135,6 @@ static void intel_setup_outputs(struct
> drm_i915_private *dev_priv)
> intel_dvo_init(dev_priv);
> }
>
>
> - intel_psr_init(dev_priv);
> -
> for_each_intel_encoder(&dev_priv->drm, encoder) {
> encoder->base.possible_crtcs =
> intel_encoder_possible_crtcs(encoder);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h
> b/drivers/gpu/drm/i915/display/intel_display.h
> index 64ffa34544a7..fc41d0d9e5a5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -417,6 +417,14 @@ enum phy_fia {
> for_each_if((encoder_mask) &\
> drm_encoder_mask(&intel_encoder->base))
>
>
> +#define for_each_intel_encoder_is_psr_enabled(dev, intel_encoder,
> encoder_mask) \
I'm not a native speaker but "is" sounds wrong, maybe
for_each_intel_encoder_with_psr_enabled()?
> + list_for_each_entry(intel_encoder, \
> + &(dev)->mode_config.encoder_list, \
> + base.head) \
> + for_each_if(((encoder_mask) & \
> + drm_encoder_mask(&intel_encoder->base)) && \
> + intel_encoder_is_psr_enabled(intel_encoder))
Looks like style is wrong here but bot will tell you if it is.
> +
> #define for_each_intel_dp(dev, intel_encoder)\
> for_each_intel_encoder(dev, intel_encoder) \
> for_each_if(intel_encoder_is_dp(intel_encoder))
> diff --git a/drivers/gpu/drm/i915/display/intel_
[Intel-gfx] [PATCH v13 1/2] drm/i915/display: Support PSR Multiple Instances
It is a preliminary work for supporting multiple EDP PSR and
DP PanelReplay. And it refactors singleton PSR to Multi Transcoder
supportable PSR.
And this moves and renames the i915_psr structure of drm_i915_private's to
intel_dp's intel_psr structure.
It also causes changes in PSR interrupt handling routine for supporting
multiple transcoders. But it does not change the scenario and timing of
enabling and disabling PSR. And it not support multiple pipes with
a single transcoder PSR case yet.
v2: Fix indentation and add comments
v3: Remove Blank line
v4: Rebased
v5: Rebased and Addressed Anshuman's review comment.
- Move calling of intel_psr_init() to intel_dp_init_connector()
v6: Address Anshuman's review comments
- Remove wrong comments and add comments for a limit of supporting of
a single pipe PSR
v7: Update intel_psr_compute_config() for supporting multiple transcoder
PSR on BDW+
v8: Address Anshuman's review comments
- Replace DRM_DEBUG_KMS with drm_dbg_kms() / DRM_WARN with drm_warn()
v9: Fix commit message
v10: Rebased
v11: Address Jose's review comment.
- Reorder calling order of intel_psr2_program_trans_man_trk_ctl().
- In order to reduce changes keep the old name for drm_i915_private.
- Change restrictions of multiple instances of PSR.
v12: Address Jose's review comment.
- Change the calling of intel_psr2_program_trans_man_trk_ctl() into
commit_pipe_config().
- Change a checking order of CAN_PSR() and connector_status to original
on i915_psr_sink_status_show().
- Drop unneeded intel_dp_update_pipe() function.
- In order to wait a specific encoder which belong to crtc_state on
intel_psr_wait_for_idle(), add checking of encoder.
- Add an whitespace to comments.
v13: Rebased and Address Jose's review comment.
- Add and use for_each_intel_psr_enabled_encoder() macro.
- In order to use correct frontbuffer_bit for each pipe,
fix intel_psr_invalidate() and intel_psr_flush().
- Remove redundant or unneeded codes.
- Update comments.
Signed-off-by: Gwan-gyeong Mun
Cc: José Roberto de Souza
Cc: Juha-Pekka Heikkila
Cc: Anshuman Gupta
Reviewed-by: Anshuman Gupta
---
drivers/gpu/drm/i915/display/intel_display.c | 2 -
drivers/gpu/drm/i915/display/intel_display.h | 8 +
.../drm/i915/display/intel_display_debugfs.c | 105 +++-
.../drm/i915/display/intel_display_types.h| 46 ++
drivers/gpu/drm/i915/display/intel_dp.c | 10 +-
drivers/gpu/drm/i915/display/intel_psr.c | 576 ++
drivers/gpu/drm/i915/display/intel_psr.h | 11 +-
drivers/gpu/drm/i915/display/intel_sprite.c | 6 +-
drivers/gpu/drm/i915/i915_drv.h | 38 --
drivers/gpu/drm/i915/i915_irq.c | 48 +-
10 files changed, 485 insertions(+), 365 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 667d941878a3..24df9dfb57fb 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14135,8 +14135,6 @@ static void intel_setup_outputs(struct drm_i915_private
*dev_priv)
intel_dvo_init(dev_priv);
}
- intel_psr_init(dev_priv);
-
for_each_intel_encoder(&dev_priv->drm, encoder) {
encoder->base.possible_crtcs =
intel_encoder_possible_crtcs(encoder);
diff --git a/drivers/gpu/drm/i915/display/intel_display.h
b/drivers/gpu/drm/i915/display/intel_display.h
index 64ffa34544a7..fc41d0d9e5a5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -417,6 +417,14 @@ enum phy_fia {
for_each_if((encoder_mask) &\
drm_encoder_mask(&intel_encoder->base))
+#define for_each_intel_encoder_is_psr_enabled(dev, intel_encoder,
encoder_mask) \
+ list_for_each_entry(intel_encoder, \
+ &(dev)->mode_config.encoder_list, \
+ base.head) \
+ for_each_if(((encoder_mask) & \
+drm_encoder_mask(&intel_encoder->base)) && \
+ intel_encoder_is_psr_enabled(intel_encoder))
+
#define for_each_intel_dp(dev, intel_encoder) \
for_each_intel_encoder(dev, intel_encoder) \
for_each_if(intel_encoder_is_dp(intel_encoder))
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index d62b18d5ecd8..9868253df61f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -249,12 +249,11 @@ static int i915_psr_sink_status_show(struct seq_file *m,
void *data)
"sink internal error",
};
struct drm_connector
