Re: [PATCH 08/20] drm/i915/fbc: Track FBC usage per-plane

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Ville
> Syrjala
> Sent: Wednesday, November 24, 2021 1:37 PM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject:  [PATCH 08/20] drm/i915/fbc: Track FBC usage per-plane
> 
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> 
> In the future we may have multiple planes on the same pipe capable of using
> FBC. Prepare for that by tracking FBC usage per-plane rather than per-crtc.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

Reviewed-by: Mika Kahola <mika.kahola@xxxxxxxxx>

> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 224 +++++++++++------------
>  drivers/gpu/drm/i915/i915_drv.h          |   2 +-
>  drivers/gpu/drm/i915/i915_trace.h        |  18 +-
>  3 files changed, 123 insertions(+), 121 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
> b/drivers/gpu/drm/i915/display/intel_fbc.c
> index db390c29c665..cf7fc0de6081 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -583,7 +583,7 @@ static bool intel_fbc_hw_is_active(struct intel_fbc *fbc)
> 
>  static void intel_fbc_hw_activate(struct intel_fbc *fbc)  {
> -	trace_intel_fbc_activate(fbc->crtc);
> +	trace_intel_fbc_activate(fbc->plane);
> 
>  	fbc->active = true;
>  	fbc->activated = true;
> @@ -593,7 +593,7 @@ static void intel_fbc_hw_activate(struct intel_fbc *fbc)
> 
>  static void intel_fbc_hw_deactivate(struct intel_fbc *fbc)  {
> -	trace_intel_fbc_deactivate(fbc->crtc);
> +	trace_intel_fbc_deactivate(fbc->plane);
> 
>  	fbc->active = false;
> 
> @@ -607,7 +607,7 @@ bool intel_fbc_is_compressing(struct intel_fbc *fbc)
> 
>  static void intel_fbc_nuke(struct intel_fbc *fbc)  {
> -	trace_intel_fbc_nuke(fbc->crtc);
> +	trace_intel_fbc_nuke(fbc->plane);
> 
>  	fbc->funcs->nuke(fbc);
>  }
> @@ -1154,8 +1154,7 @@ static bool intel_fbc_can_activate(struct intel_fbc
> *fbc)
>  	return true;
>  }
> 
> -static void intel_fbc_get_reg_params(struct intel_fbc *fbc,
> -				     struct intel_crtc *crtc)
> +static void intel_fbc_get_reg_params(struct intel_fbc *fbc)
>  {
>  	const struct intel_fbc_state *cache = &fbc->state_cache;
>  	struct intel_fbc_state *params = &fbc->params; @@ -1213,30 +1212,19
> @@ static bool intel_fbc_can_flip_nuke(struct intel_atomic_state *state,
>  	return true;
>  }
> 
> -bool intel_fbc_pre_update(struct intel_atomic_state *state,
> -			  struct intel_crtc *crtc)
> +static bool __intel_fbc_pre_update(struct intel_atomic_state *state,
> +				   struct intel_crtc *crtc,
> +				   struct intel_plane *plane)
>  {
> -	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
> -	const struct intel_plane_state *plane_state =
> -		intel_atomic_get_new_plane_state(state, plane);
> -	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	struct drm_i915_private *i915 = to_i915(state->base.dev);
>  	struct intel_fbc *fbc = plane->fbc;
> -	const char *reason = "update pending";
>  	bool need_vblank_wait = false;
> 
> -	if (!fbc || !plane_state)
> -		return need_vblank_wait;
> -
> -	mutex_lock(&fbc->lock);
> -
> -	if (fbc->crtc != crtc)
> -		goto unlock;
> -
>  	intel_fbc_update_state_cache(state, crtc, plane);
>  	fbc->flip_pending = true;
> 
>  	if (!intel_fbc_can_flip_nuke(state, crtc, plane)) {
> -		intel_fbc_deactivate(fbc, reason);
> +		intel_fbc_deactivate(fbc, "update pending");
> 
>  		/*
>  		 * Display WA #1198: glk+
> @@ -1256,8 +1244,31 @@ bool intel_fbc_pre_update(struct intel_atomic_state
> *state,
>  			need_vblank_wait = true;
>  		fbc->activated = false;
>  	}
> -unlock:
> -	mutex_unlock(&fbc->lock);
> +
> +	return need_vblank_wait;
> +}
> +
> +bool intel_fbc_pre_update(struct intel_atomic_state *state,
> +			  struct intel_crtc *crtc)
> +{
> +	const struct intel_plane_state *plane_state;
> +	bool need_vblank_wait = false;
> +	struct intel_plane *plane;
> +	int i;
> +
> +	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
> +		struct intel_fbc *fbc = plane->fbc;
> +
> +		if (!fbc || plane->pipe != crtc->pipe)
> +			continue;
> +
> +		mutex_lock(&fbc->lock);
> +
> +		if (fbc->plane == plane)
> +			need_vblank_wait |= __intel_fbc_pre_update(state,
> crtc, plane);
> +
> +		mutex_unlock(&fbc->lock);
> +	}
> 
>  	return need_vblank_wait;
>  }
> @@ -1265,18 +1276,18 @@ bool intel_fbc_pre_update(struct
> intel_atomic_state *state,  static void __intel_fbc_disable(struct intel_fbc *fbc)
> {
>  	struct drm_i915_private *i915 = fbc->i915;
> -	struct intel_crtc *crtc = fbc->crtc;
> +	struct intel_plane *plane = fbc->plane;
> 
>  	drm_WARN_ON(&i915->drm, !mutex_is_locked(&fbc->lock));
> -	drm_WARN_ON(&i915->drm, !fbc->crtc);
> +	drm_WARN_ON(&i915->drm, !fbc->plane);
>  	drm_WARN_ON(&i915->drm, fbc->active);
> 
> -	drm_dbg_kms(&i915->drm, "Disabling FBC on pipe %c\n",
> -		    pipe_name(crtc->pipe));
> +	drm_dbg_kms(&i915->drm, "Disabling FBC on [PLANE:%d:%s]\n",
> +		    plane->base.base.id, plane->base.name);
> 
>  	__intel_fbc_cleanup_cfb(fbc);
> 
> -	fbc->crtc = NULL;
> +	fbc->plane = NULL;
>  }
> 
>  static void __intel_fbc_post_update(struct intel_fbc *fbc) @@ -1304,27
> +1315,32 @@ static void __intel_fbc_post_update(struct intel_fbc *fbc)  void
> intel_fbc_post_update(struct intel_atomic_state *state,
>  			   struct intel_crtc *crtc)
>  {
> -	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
> -	const struct intel_plane_state *plane_state =
> -		intel_atomic_get_new_plane_state(state, plane);
> -	struct intel_fbc *fbc = plane->fbc;
> -
> -	if (!fbc || !plane_state)
> -		return;
> -
> -	mutex_lock(&fbc->lock);
> -	if (fbc->crtc == crtc) {
> -		fbc->flip_pending = false;
> -		intel_fbc_get_reg_params(fbc, crtc);
> -		__intel_fbc_post_update(fbc);
> +	const struct intel_plane_state *plane_state;
> +	struct intel_plane *plane;
> +	int i;
> +
> +	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
> +		struct intel_fbc *fbc = plane->fbc;
> +
> +		if (!fbc || plane->pipe != crtc->pipe)
> +			continue;
> +
> +		mutex_lock(&fbc->lock);
> +
> +		if (fbc->plane == plane) {
> +			fbc->flip_pending = false;
> +			intel_fbc_get_reg_params(fbc);
> +			__intel_fbc_post_update(fbc);
> +		}
> +
> +		mutex_unlock(&fbc->lock);
>  	}
> -	mutex_unlock(&fbc->lock);
>  }
> 
>  static unsigned int intel_fbc_get_frontbuffer_bit(struct intel_fbc *fbc)  {
> -	if (fbc->crtc)
> -		return to_intel_plane(fbc->crtc->base.primary)-
> >frontbuffer_bit;
> +	if (fbc->plane)
> +		return fbc->plane->frontbuffer_bit;
>  	else
>  		return fbc->possible_framebuffer_bits;  } @@ -1345,7 +1361,7
> @@ void intel_fbc_invalidate(struct drm_i915_private *i915,
> 
>  	fbc->busy_bits |= intel_fbc_get_frontbuffer_bit(fbc) & frontbuffer_bits;
> 
> -	if (fbc->crtc && fbc->busy_bits)
> +	if (fbc->plane && fbc->busy_bits)
>  		intel_fbc_deactivate(fbc, "frontbuffer write");
> 
>  	mutex_unlock(&fbc->lock);
> @@ -1366,7 +1382,7 @@ void intel_fbc_flush(struct drm_i915_private *i915,
>  	if (origin == ORIGIN_FLIP || origin == ORIGIN_CURSOR_UPDATE)
>  		goto out;
> 
> -	if (!fbc->busy_bits && fbc->crtc &&
> +	if (!fbc->busy_bits && fbc->plane &&
>  	    (frontbuffer_bits & intel_fbc_get_frontbuffer_bit(fbc))) {
>  		if (fbc->active)
>  			intel_fbc_nuke(fbc);
> @@ -1395,43 +1411,24 @@ int intel_fbc_atomic_check(struct
> intel_atomic_state *state)
>  	return 0;
>  }
> 
> -/**
> - * intel_fbc_enable: tries to enable FBC on the CRTC
> - * @crtc: the CRTC
> - * @state: corresponding &drm_crtc_state for @crtc
> - *
> - * This function checks if the given CRTC was chosen for FBC, then enables it if
> - * possible. Notice that it doesn't activate FBC. It is valid to call
> - * intel_fbc_enable multiple times for the same pipe without an
> - * intel_fbc_disable in the middle, as long as it is deactivated.
> - */
> -static void intel_fbc_enable(struct intel_atomic_state *state,
> -			     struct intel_crtc *crtc)
> +static void __intel_fbc_enable(struct intel_atomic_state *state,
> +			       struct intel_crtc *crtc,
> +			       struct intel_plane *plane)
>  {
> -	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> -	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
> +	struct drm_i915_private *i915 = to_i915(state->base.dev);
>  	const struct intel_plane_state *plane_state =
>  		intel_atomic_get_new_plane_state(state, plane);
>  	struct intel_fbc *fbc = plane->fbc;
> -	struct intel_fbc_state *cache;
> -	int min_limit;
> +	struct intel_fbc_state *cache = &fbc->state_cache;
> +	int min_limit = intel_fbc_min_limit(plane_state);
> 
> -	if (!fbc || !plane_state)
> -		return;
> -
> -	cache = &fbc->state_cache;
> -
> -	min_limit = intel_fbc_min_limit(plane_state);
> -
> -	mutex_lock(&fbc->lock);
> -
> -	if (fbc->crtc) {
> -		if (fbc->crtc != crtc)
> -			goto out;
> +	if (fbc->plane) {
> +		if (fbc->plane != plane)
> +			return;
> 
>  		if (fbc->limit >= min_limit &&
>  		    !intel_fbc_cfb_size_changed(fbc))
> -			goto out;
> +			return;
> 
>  		__intel_fbc_disable(fbc);
>  	}
> @@ -1441,22 +1438,20 @@ static void intel_fbc_enable(struct
> intel_atomic_state *state,
>  	intel_fbc_update_state_cache(state, crtc, plane);
> 
>  	if (cache->no_fbc_reason)
> -		goto out;
> +		return;
> 
>  	if (intel_fbc_alloc_cfb(fbc, intel_fbc_cfb_size(plane_state), min_limit)) {
>  		fbc->no_fbc_reason = "not enough stolen memory";
> -		goto out;
> +		return;
>  	}
> 
> -	drm_dbg_kms(&i915->drm, "Enabling FBC on pipe %c\n",
> -		    pipe_name(crtc->pipe));
> +	drm_dbg_kms(&i915->drm, "Enabling FBC on [PLANE:%d:%s]\n",
> +		    plane->base.base.id, plane->base.name);
>  	fbc->no_fbc_reason = "FBC enabled but not active yet\n";
> 
> -	fbc->crtc = crtc;
> +	fbc->plane = plane;
> 
>  	intel_fbc_program_cfb(fbc);
> -out:
> -	mutex_unlock(&fbc->lock);
>  }
> 
>  /**
> @@ -1467,45 +1462,48 @@ static void intel_fbc_enable(struct
> intel_atomic_state *state,
>   */
>  void intel_fbc_disable(struct intel_crtc *crtc)  {
> -	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
> -	struct intel_fbc *fbc = plane->fbc;
> +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	struct intel_plane *plane;
> 
> -	if (!fbc)
> -		return;
> +	for_each_intel_plane(&i915->drm, plane) {
> +		struct intel_fbc *fbc = plane->fbc;
> 
> -	mutex_lock(&fbc->lock);
> -	if (fbc->crtc == crtc)
> -		__intel_fbc_disable(fbc);
> -	mutex_unlock(&fbc->lock);
> +		if (!fbc || plane->pipe != crtc->pipe)
> +			continue;
> +
> +		mutex_lock(&fbc->lock);
> +		if (fbc->plane == plane)
> +			__intel_fbc_disable(fbc);
> +		mutex_unlock(&fbc->lock);
> +	}
>  }
> 
> -/**
> - * intel_fbc_update: enable/disable FBC on the CRTC
> - * @state: atomic state
> - * @crtc: the CRTC
> - *
> - * This function checks if the given CRTC was chosen for FBC, then enables it if
> - * possible. Notice that it doesn't activate FBC. It is valid to call
> - * intel_fbc_update multiple times for the same pipe without an
> - * intel_fbc_disable in the middle.
> - */
>  void intel_fbc_update(struct intel_atomic_state *state,
>  		      struct intel_crtc *crtc)
>  {
> -	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
>  	const struct intel_crtc_state *crtc_state =
>  		intel_atomic_get_new_crtc_state(state, crtc);
> -	const struct intel_plane_state *plane_state =
> -		intel_atomic_get_new_plane_state(state, plane);
> -	struct intel_fbc *fbc = plane->fbc;
> +	const struct intel_plane_state *plane_state;
> +	struct intel_plane *plane;
> +	int i;
> 
> -	if (!fbc || !plane_state)
> -		return;
> +	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
> +		struct intel_fbc *fbc = plane->fbc;
> 
> -	if (crtc_state->update_pipe && plane_state->no_fbc_reason)
> -		intel_fbc_disable(crtc);
> -	else
> -		intel_fbc_enable(state, crtc);
> +		if (!fbc || plane->pipe != crtc->pipe)
> +			continue;
> +
> +		mutex_lock(&fbc->lock);
> +
> +		if (crtc_state->update_pipe && plane_state->no_fbc_reason) {
> +			if (fbc->plane == plane)
> +				__intel_fbc_disable(fbc);
> +		} else {
> +			__intel_fbc_enable(state, crtc, plane);
> +		}
> +
> +		mutex_unlock(&fbc->lock);
> +	}
>  }
> 
>  /**
> @@ -1522,10 +1520,8 @@ void intel_fbc_global_disable(struct
> drm_i915_private *i915)
>  		return;
> 
>  	mutex_lock(&fbc->lock);
> -	if (fbc->crtc) {
> -		drm_WARN_ON(&i915->drm, fbc->crtc->active);
> +	if (fbc->plane)
>  		__intel_fbc_disable(fbc);
> -	}
>  	mutex_unlock(&fbc->lock);
>  }
> 
> @@ -1538,7 +1534,7 @@ static void intel_fbc_underrun_work_fn(struct
> work_struct *work)
>  	mutex_lock(&fbc->lock);
> 
>  	/* Maybe we were scheduled twice. */
> -	if (fbc->underrun_detected || !fbc->crtc)
> +	if (fbc->underrun_detected || !fbc->plane)
>  		goto out;
> 
>  	drm_dbg_kms(&i915->drm, "Disabling FBC due to FIFO underrun.\n");
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a737fa483cf3..f632b026ce34 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -423,7 +423,7 @@ struct intel_fbc {
>  	struct mutex lock;
>  	unsigned int possible_framebuffer_bits;
>  	unsigned int busy_bits;
> -	struct intel_crtc *crtc;
> +	struct intel_plane *plane;
> 
>  	struct drm_mm_node compressed_fb;
>  	struct drm_mm_node compressed_llb;
> diff --git a/drivers/gpu/drm/i915/i915_trace.h
> b/drivers/gpu/drm/i915/i915_trace.h
> index 6b8fb6ffe8da..7d6e638dcccb 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -372,8 +372,8 @@ TRACE_EVENT(intel_plane_disable_arm,
>  /* fbc */
> 
>  TRACE_EVENT(intel_fbc_activate,
> -	    TP_PROTO(struct intel_crtc *crtc),
> -	    TP_ARGS(crtc),
> +	    TP_PROTO(struct intel_plane *plane),
> +	    TP_ARGS(plane),
> 
>  	    TP_STRUCT__entry(
>  			     __field(enum pipe, pipe)
> @@ -382,6 +382,8 @@ TRACE_EVENT(intel_fbc_activate,
>  			     ),
> 
>  	    TP_fast_assign(
> +			   struct intel_crtc *crtc =
> intel_get_crtc_for_pipe(to_i915(plane->base.dev),
> +
> plane->pipe);
>  			   __entry->pipe = crtc->pipe;
>  			   __entry->frame =
> intel_crtc_get_vblank_counter(crtc);
>  			   __entry->scanline = intel_get_crtc_scanline(crtc); @@
> -392,8 +394,8 @@ TRACE_EVENT(intel_fbc_activate,  );
> 
>  TRACE_EVENT(intel_fbc_deactivate,
> -	    TP_PROTO(struct intel_crtc *crtc),
> -	    TP_ARGS(crtc),
> +	    TP_PROTO(struct intel_plane *plane),
> +	    TP_ARGS(plane),
> 
>  	    TP_STRUCT__entry(
>  			     __field(enum pipe, pipe)
> @@ -402,6 +404,8 @@ TRACE_EVENT(intel_fbc_deactivate,
>  			     ),
> 
>  	    TP_fast_assign(
> +			   struct intel_crtc *crtc =
> intel_get_crtc_for_pipe(to_i915(plane->base.dev),
> +
> plane->pipe);
>  			   __entry->pipe = crtc->pipe;
>  			   __entry->frame =
> intel_crtc_get_vblank_counter(crtc);
>  			   __entry->scanline = intel_get_crtc_scanline(crtc); @@
> -412,8 +416,8 @@ TRACE_EVENT(intel_fbc_deactivate,  );
> 
>  TRACE_EVENT(intel_fbc_nuke,
> -	    TP_PROTO(struct intel_crtc *crtc),
> -	    TP_ARGS(crtc),
> +	    TP_PROTO(struct intel_plane *plane),
> +	    TP_ARGS(plane),
> 
>  	    TP_STRUCT__entry(
>  			     __field(enum pipe, pipe)
> @@ -422,6 +426,8 @@ TRACE_EVENT(intel_fbc_nuke,
>  			     ),
> 
>  	    TP_fast_assign(
> +			   struct intel_crtc *crtc =
> intel_get_crtc_for_pipe(to_i915(plane->base.dev),
> +
> plane->pipe);
>  			   __entry->pipe = crtc->pipe;
>  			   __entry->frame =
> intel_crtc_get_vblank_counter(crtc);
>  			   __entry->scanline = intel_get_crtc_scanline(crtc);
> --
> 2.32.0





[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux