diff mbox series

[2/2] drm/i915: Extract intel_post_plane_update_after_readout()

Message ID 20240916152958.17332-3-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Some modeset refactoring | expand

Commit Message

Ville Syrjälä Sept. 16, 2024, 3:29 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Clean up the main commit_tail() codepath a bit by pulling
the post plane update steps that need to performed after
readout into their own little function
(intel_post_plane_update_after_readout()).

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 25 +++++++++++++-------
 1 file changed, 17 insertions(+), 8 deletions(-)

Comments

Luca Coelho Sept. 19, 2024, 7:53 a.m. UTC | #1 
On Mon, 2024-09-16 at 18:29 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Clean up the main commit_tail() codepath a bit by pulling
> the post plane update steps that need to performed after
> readout into their own little function
> (intel_post_plane_update_after_readout()).
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---

This looks fine, but what is the exact motivation here? Is it just to
reduce the size of intel_atomic_commit_tail()?

Regardless:

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.
Ville Syrjälä Sept. 19, 2024, 11:17 a.m. UTC | #2 
On Thu, Sep 19, 2024 at 10:53:15AM +0300, Luca Coelho wrote:
> On Mon, 2024-09-16 at 18:29 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Clean up the main commit_tail() codepath a bit by pulling
> > the post plane update steps that need to performed after
> > readout into their own little function
> > (intel_post_plane_update_after_readout()).
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> 
> This looks fine, but what is the exact motivation here? Is it just to
> reduce the size of intel_atomic_commit_tail()?

That, and I guess some kind of attempt to keep the pre vs. post
update code mostly in sync (might be easier to remember to update
both sides if they are named somewhat consistently).

> 
> Regardless:
> 
> Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

Ta.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 591fe3b4ace8..5776f9514346 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1117,6 +1117,22 @@  static void intel_post_plane_update(struct intel_atomic_state *state,
 		intel_encoders_audio_enable(state, crtc);
 }
 
+static void intel_post_plane_update_after_readout(struct intel_atomic_state *state,
+						  struct intel_crtc *crtc)
+{
+	const struct intel_crtc_state *new_crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
+
+	/* Must be done after gamma readout due to HSW split gamma vs. IPS w/a */
+	hsw_ips_post_update(state, crtc);
+
+	/*
+	 * Activate DRRS after state readout to avoid
+	 * dp_m_n vs. dp_m2_n2 confusion on BDW+.
+	 */
+	intel_drrs_activate(new_crtc_state);
+}
+
 static void intel_crtc_enable_flip_done(struct intel_atomic_state *state,
 					struct intel_crtc *crtc)
 {
@@ -7479,14 +7495,7 @@  static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 
 		intel_modeset_verify_crtc(state, crtc);
 
-		/* Must be done after gamma readout due to HSW split gamma vs. IPS w/a */
-		hsw_ips_post_update(state, crtc);
-
-		/*
-		 * Activate DRRS after state readout to avoid
-		 * dp_m_n vs. dp_m2_n2 confusion on BDW+.
-		 */
-		intel_drrs_activate(new_crtc_state);
+		intel_post_plane_update_after_readout(state, crtc);
 
 		/*
 		 * DSB cleanup is done in cleanup_work aligning with framebuffer