Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1455388 > unrolled thread

[PATCH v5 5/6] drm/i915: Move CRTC updating in atomic_commit into it's own hook

Started byLyude <cpaul@redhat.com>
First post2016-08-02 21:00 +0200
Last post2016-08-03 00:20 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH v5 5/6] drm/i915: Move CRTC updating in atomic_commit into it's own hook Lyude <cpaul@redhat.com> - 2016-08-02 21:00 +0200
    Re: [PATCH v5 5/6] drm/i915: Move CRTC updating in atomic_commit  into it's own hook Matt Roper <matthew.d.roper@intel.com> - 2016-08-03 00:20 +0200

#1455388 — [PATCH v5 5/6] drm/i915: Move CRTC updating in atomic_commit into it's own hook

FromLyude <cpaul@redhat.com>
Date2016-08-02 21:00 +0200
Subject[PATCH v5 5/6] drm/i915: Move CRTC updating in atomic_commit into it's own hook
Message-ID<s1N4Z-8oW-5@gated-at.bofh.it>
Since we have to write ddb allocations at the same time as we do other
plane updates, we're going to need to be able to control the order in
which we execute modesets on each pipe. The easiest way to do this is to
just factor this section of intel_atomic_commit_tail()
(intel_atomic_commit() for stable branches) into it's own function, and
add an appropriate display function hook for it.

Based off of Matt Rope's suggestions

Signed-off-by: Lyude <cpaul@redhat.com>
[omitting CC for stable, since this patch will need to be changed for
such backports first]
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  2 +
 drivers/gpu/drm/i915/intel_display.c | 74 +++++++++++++++++++++++++-----------
 2 files changed, 54 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 87018d3..4964e3f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -632,6 +632,8 @@ struct drm_i915_display_funcs {
 				  struct intel_crtc_state *crtc_state);
 	void (*crtc_enable)(struct drm_crtc *crtc);
 	void (*crtc_disable)(struct drm_crtc *crtc);
+	void (*update_crtcs)(struct drm_atomic_state *state,
+			     unsigned int *crtc_vblank_mask);
 	void (*audio_codec_enable)(struct drm_connector *connector,
 				   struct intel_encoder *encoder,
 				   const struct drm_display_mode *adjusted_mode);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 79d146c..d985b5b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13624,6 +13624,52 @@ static bool needs_vblank_wait(struct intel_crtc_state *crtc_state)
 	return false;
 }
 
+static void intel_update_crtc(struct drm_crtc *crtc,
+			      struct drm_atomic_state *state,
+			      struct drm_crtc_state *old_crtc_state,
+			      unsigned int *crtc_vblank_mask)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc->state);
+	bool modeset = needs_modeset(crtc->state);
+
+	if (modeset) {
+		update_scanline_offset(intel_crtc);
+		dev_priv->display.crtc_enable(crtc);
+	} else {
+		intel_pre_plane_update(to_intel_crtc_state(old_crtc_state));
+	}
+
+	if (drm_atomic_get_existing_plane_state(state, crtc->primary)) {
+		intel_fbc_enable(
+		    intel_crtc, pipe_config,
+		    to_intel_plane_state(crtc->primary->state));
+	}
+
+	drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
+
+	if (pipe_config->base.active && needs_vblank_wait(pipe_config))
+		*crtc_vblank_mask |= drm_crtc_mask(crtc);
+}
+
+static void intel_update_crtcs(struct drm_atomic_state *state,
+			       unsigned int *crtc_vblank_mask)
+{
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *old_crtc_state;
+	int i;
+
+	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
+		if (!crtc->state->active)
+			continue;
+
+		intel_update_crtc(crtc, state, old_crtc_state,
+				  crtc_vblank_mask);
+	}
+}
+
 static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 {
 	struct drm_device *dev = state->dev;
@@ -13715,17 +13761,9 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 		intel_modeset_verify_disabled(dev);
 	}
 
-	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
+	/* Complete the events for pipes that have now been disabled */
 	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
-		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 		bool modeset = needs_modeset(crtc->state);
-		struct intel_crtc_state *pipe_config =
-			to_intel_crtc_state(crtc->state);
-
-		if (modeset && crtc->state->active) {
-			update_scanline_offset(to_intel_crtc(crtc));
-			dev_priv->display.crtc_enable(crtc);
-		}
 
 		/* Complete events for now disable pipes here. */
 		if (modeset && !crtc->state->active && crtc->state->event) {
@@ -13735,21 +13773,11 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 
 			crtc->state->event = NULL;
 		}
-
-		if (!modeset)
-			intel_pre_plane_update(to_intel_crtc_state(old_crtc_state));
-
-		if (crtc->state->active &&
-		    drm_atomic_get_existing_plane_state(state, crtc->primary))
-			intel_fbc_enable(intel_crtc, pipe_config, to_intel_plane_state(crtc->primary->state));
-
-		if (crtc->state->active)
-			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
-
-		if (pipe_config->base.active && needs_vblank_wait(pipe_config))
-			crtc_vblank_mask |= 1 << i;
 	}
 
+	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
+	dev_priv->display.update_crtcs(state, &crtc_vblank_mask);
+
 	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
 	 * already, but still need the state for the delayed optimization. To
 	 * fix this:
@@ -15202,6 +15230,8 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
 	}
 
+	dev_priv->display.update_crtcs = intel_update_crtcs;
+
 	/* Returns the core display clock speed */
 	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
 		dev_priv->display.get_display_clock_speed =
-- 
2.7.4

[toc] | [next] | [standalone]


#1455503 — Re: [PATCH v5 5/6] drm/i915: Move CRTC updating in atomic_commit into it's own hook

FromMatt Roper <matthew.d.roper@intel.com>
Date2016-08-03 00:20 +0200
SubjectRe: [PATCH v5 5/6] drm/i915: Move CRTC updating in atomic_commit into it's own hook
Message-ID<s1Qcz-2bw-53@gated-at.bofh.it>
In reply to#1455388
On Tue, Aug 02, 2016 at 02:52:53PM -0400, Lyude wrote:
> Since we have to write ddb allocations at the same time as we do other
> plane updates, we're going to need to be able to control the order in
> which we execute modesets on each pipe. The easiest way to do this is to
> just factor this section of intel_atomic_commit_tail()
> (intel_atomic_commit() for stable branches) into it's own function, and
> add an appropriate display function hook for it.
> 
> Based off of Matt Rope's suggestions
> 
> Signed-off-by: Lyude <cpaul@redhat.com>
> [omitting CC for stable, since this patch will need to be changed for
> such backports first]
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 +
>  drivers/gpu/drm/i915/intel_display.c | 74 +++++++++++++++++++++++++-----------
>  2 files changed, 54 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 87018d3..4964e3f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -632,6 +632,8 @@ struct drm_i915_display_funcs {
>  				  struct intel_crtc_state *crtc_state);
>  	void (*crtc_enable)(struct drm_crtc *crtc);
>  	void (*crtc_disable)(struct drm_crtc *crtc);
> +	void (*update_crtcs)(struct drm_atomic_state *state,
> +			     unsigned int *crtc_vblank_mask);
>  	void (*audio_codec_enable)(struct drm_connector *connector,
>  				   struct intel_encoder *encoder,
>  				   const struct drm_display_mode *adjusted_mode);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 79d146c..d985b5b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13624,6 +13624,52 @@ static bool needs_vblank_wait(struct intel_crtc_state *crtc_state)
>  	return false;
>  }
>  
> +static void intel_update_crtc(struct drm_crtc *crtc,
> +			      struct drm_atomic_state *state,
> +			      struct drm_crtc_state *old_crtc_state,
> +			      unsigned int *crtc_vblank_mask)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc->state);
> +	bool modeset = needs_modeset(crtc->state);
> +
> +	if (modeset) {
> +		update_scanline_offset(intel_crtc);
> +		dev_priv->display.crtc_enable(crtc);
> +	} else {
> +		intel_pre_plane_update(to_intel_crtc_state(old_crtc_state));
> +	}
> +
> +	if (drm_atomic_get_existing_plane_state(state, crtc->primary)) {
> +		intel_fbc_enable(
> +		    intel_crtc, pipe_config,
> +		    to_intel_plane_state(crtc->primary->state));
> +	}
> +
> +	drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
> +
> +	if (pipe_config->base.active && needs_vblank_wait(pipe_config))

You can drop the pipe_config->base.active since pipe_config->base.active
is just another way of writing crtc->state->active, which you tested
before calling this function.

Other than that,

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> +		*crtc_vblank_mask |= drm_crtc_mask(crtc);
> +}
> +
> +static void intel_update_crtcs(struct drm_atomic_state *state,
> +			       unsigned int *crtc_vblank_mask)
> +{
> +	struct drm_crtc *crtc;
> +	struct drm_crtc_state *old_crtc_state;
> +	int i;
> +
> +	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
> +		if (!crtc->state->active)
> +			continue;
> +
> +		intel_update_crtc(crtc, state, old_crtc_state,
> +				  crtc_vblank_mask);
> +	}
> +}
> +
>  static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>  {
>  	struct drm_device *dev = state->dev;
> @@ -13715,17 +13761,9 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>  		intel_modeset_verify_disabled(dev);
>  	}
>  
> -	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
> +	/* Complete the events for pipes that have now been disabled */
>  	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
> -		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  		bool modeset = needs_modeset(crtc->state);
> -		struct intel_crtc_state *pipe_config =
> -			to_intel_crtc_state(crtc->state);
> -
> -		if (modeset && crtc->state->active) {
> -			update_scanline_offset(to_intel_crtc(crtc));
> -			dev_priv->display.crtc_enable(crtc);
> -		}
>  
>  		/* Complete events for now disable pipes here. */
>  		if (modeset && !crtc->state->active && crtc->state->event) {
> @@ -13735,21 +13773,11 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>  
>  			crtc->state->event = NULL;
>  		}
> -
> -		if (!modeset)
> -			intel_pre_plane_update(to_intel_crtc_state(old_crtc_state));
> -
> -		if (crtc->state->active &&
> -		    drm_atomic_get_existing_plane_state(state, crtc->primary))
> -			intel_fbc_enable(intel_crtc, pipe_config, to_intel_plane_state(crtc->primary->state));
> -
> -		if (crtc->state->active)
> -			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
> -
> -		if (pipe_config->base.active && needs_vblank_wait(pipe_config))
> -			crtc_vblank_mask |= 1 << i;
>  	}
>  
> +	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
> +	dev_priv->display.update_crtcs(state, &crtc_vblank_mask);
> +
>  	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
>  	 * already, but still need the state for the delayed optimization. To
>  	 * fix this:
> @@ -15202,6 +15230,8 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
>  	}
>  
> +	dev_priv->display.update_crtcs = intel_update_crtcs;
> +
>  	/* Returns the core display clock speed */
>  	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
>  		dev_priv->display.get_display_clock_speed =
> -- 
> 2.7.4
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web