Re: [PATCH 2/3] drm/i915/fbc: Loop through FBC instances in various places

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

 



On Thu, 09 Dec 2021, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Convert i915->fbc into an array in preparation for
> multiple FBC instances, and loop through all instances
> in all places where the caller does not know which
> instance(s) (if any) are relevant. This is the case
> for eg. frontbuffer tracking and FIFO underrun hadling.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>

> ---
>  drivers/gpu/drm/i915/display/i9xx_plane.c     |   2 +-
>  drivers/gpu/drm/i915/display/intel_fbc.c      | 166 +++++++++++-------
>  .../drm/i915/display/skl_universal_plane.c    |   2 +-
>  drivers/gpu/drm/i915/i915_drv.h               |   3 +-
>  4 files changed, 104 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c b/drivers/gpu/drm/i915/display/i9xx_plane.c
> index 85950ff67609..731f446bdf20 100644
> --- a/drivers/gpu/drm/i915/display/i9xx_plane.c
> +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
> @@ -125,7 +125,7 @@ static struct intel_fbc *i9xx_plane_fbc(struct drm_i915_private *dev_priv,
>  					enum i9xx_plane_id i9xx_plane)
>  {
>  	if (i9xx_plane_has_fbc(dev_priv, i9xx_plane))
> -		return dev_priv->fbc;
> +		return dev_priv->fbc[FBC_A];
>  	else
>  		return NULL;
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 8376f819071e..2f1a72f98c4b 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -49,6 +49,13 @@
>  #include "intel_fbc.h"
>  #include "intel_frontbuffer.h"
>  
> +#define for_each_fbc_id(__fbc_id) \
> +	for ((__fbc_id) = FBC_A; (__fbc_id) < I915_MAX_FBCS; (__fbc_id)++)
> +
> +#define for_each_intel_fbc(__dev_priv, __fbc, __fbc_id) \
> +	for_each_fbc_id(__fbc_id) \
> +		for_each_if((__fbc) = (__dev_priv)->fbc[(__fbc_id)])
> +
>  struct intel_fbc_funcs {
>  	void (*activate)(struct intel_fbc *fbc);
>  	void (*deactivate)(struct intel_fbc *fbc);
> @@ -812,16 +819,16 @@ static void __intel_fbc_cleanup_cfb(struct intel_fbc *fbc)
>  
>  void intel_fbc_cleanup(struct drm_i915_private *i915)
>  {
> -	struct intel_fbc *fbc = i915->fbc;
> +	struct intel_fbc *fbc;
> +	enum fbc_id fbc_id;
>  
> -	if (!fbc)
> -		return;
> +	for_each_intel_fbc(i915, fbc, fbc_id) {
> +		mutex_lock(&fbc->lock);
> +		__intel_fbc_cleanup_cfb(fbc);
> +		mutex_unlock(&fbc->lock);
>  
> -	mutex_lock(&fbc->lock);
> -	__intel_fbc_cleanup_cfb(fbc);
> -	mutex_unlock(&fbc->lock);
> -
> -	kfree(fbc);
> +		kfree(fbc);
> +	}
>  }
>  
>  static bool stride_is_valid(const struct intel_plane_state *plane_state)
> @@ -1307,36 +1314,39 @@ static unsigned int intel_fbc_get_frontbuffer_bit(struct intel_fbc *fbc)
>  		return fbc->possible_framebuffer_bits;
>  }
>  
> +static void __intel_fbc_invalidate(struct intel_fbc *fbc,
> +				   unsigned int frontbuffer_bits,
> +				   enum fb_op_origin origin)
> +{
> +	if (origin == ORIGIN_FLIP || origin == ORIGIN_CURSOR_UPDATE)
> +		return;
> +
> +	mutex_lock(&fbc->lock);
> +
> +	fbc->busy_bits |= intel_fbc_get_frontbuffer_bit(fbc) & frontbuffer_bits;
> +
> +	if (fbc->state.plane && fbc->busy_bits)
> +		intel_fbc_deactivate(fbc, "frontbuffer write");
> +
> +	mutex_unlock(&fbc->lock);
> +}
> +
>  void intel_fbc_invalidate(struct drm_i915_private *i915,
>  			  unsigned int frontbuffer_bits,
>  			  enum fb_op_origin origin)
>  {
> -	struct intel_fbc *fbc = i915->fbc;
> +	struct intel_fbc *fbc;
> +	enum fbc_id fbc_id;
>  
> -	if (!fbc)
> -		return;
> +	for_each_intel_fbc(i915, fbc, fbc_id)
> +		__intel_fbc_invalidate(fbc, frontbuffer_bits, origin);
>  
> -	if (origin == ORIGIN_FLIP || origin == ORIGIN_CURSOR_UPDATE)
> -		return;
> -
> -	mutex_lock(&fbc->lock);
> -
> -	fbc->busy_bits |= intel_fbc_get_frontbuffer_bit(fbc) & frontbuffer_bits;
> -
> -	if (fbc->state.plane && fbc->busy_bits)
> -		intel_fbc_deactivate(fbc, "frontbuffer write");
> -
> -	mutex_unlock(&fbc->lock);
>  }
>  
> -void intel_fbc_flush(struct drm_i915_private *i915,
> -		     unsigned int frontbuffer_bits, enum fb_op_origin origin)
> +static void __intel_fbc_flush(struct intel_fbc *fbc,
> +			      unsigned int frontbuffer_bits,
> +			      enum fb_op_origin origin)
>  {
> -	struct intel_fbc *fbc = i915->fbc;
> -
> -	if (!fbc)
> -		return;
> -
>  	mutex_lock(&fbc->lock);
>  
>  	fbc->busy_bits &= ~frontbuffer_bits;
> @@ -1356,6 +1366,17 @@ void intel_fbc_flush(struct drm_i915_private *i915,
>  	mutex_unlock(&fbc->lock);
>  }
>  
> +void intel_fbc_flush(struct drm_i915_private *i915,
> +		     unsigned int frontbuffer_bits,
> +		     enum fb_op_origin origin)
> +{
> +	struct intel_fbc *fbc;
> +	enum fbc_id fbc_id;
> +
> +	for_each_intel_fbc(i915, fbc, fbc_id)
> +		__intel_fbc_flush(fbc, frontbuffer_bits, origin);
> +}
> +
>  int intel_fbc_atomic_check(struct intel_atomic_state *state)
>  {
>  	struct intel_plane_state *plane_state;
> @@ -1483,15 +1504,15 @@ void intel_fbc_update(struct intel_atomic_state *state,
>   */
>  void intel_fbc_global_disable(struct drm_i915_private *i915)
>  {
> -	struct intel_fbc *fbc = i915->fbc;
> +	struct intel_fbc *fbc;
> +	enum fbc_id fbc_id;
>  
> -	if (!fbc)
> -		return;
> -
> -	mutex_lock(&fbc->lock);
> -	if (fbc->state.plane)
> -		__intel_fbc_disable(fbc);
> -	mutex_unlock(&fbc->lock);
> +	for_each_intel_fbc(i915, fbc, fbc_id) {
> +		mutex_lock(&fbc->lock);
> +		if (fbc->state.plane)
> +			__intel_fbc_disable(fbc);
> +		mutex_unlock(&fbc->lock);
> +	}
>  }
>  
>  static void intel_fbc_underrun_work_fn(struct work_struct *work)
> @@ -1516,19 +1537,9 @@ static void intel_fbc_underrun_work_fn(struct work_struct *work)
>  	mutex_unlock(&fbc->lock);
>  }
>  
> -/*
> - * intel_fbc_reset_underrun - reset FBC fifo underrun status.
> - * @i915: the i915 device
> - *
> - * See intel_fbc_handle_fifo_underrun_irq(). For automated testing we
> - * want to re-enable FBC after an underrun to increase test coverage.
> - */
> -void intel_fbc_reset_underrun(struct drm_i915_private *i915)
> +static void __intel_fbc_reset_underrun(struct intel_fbc *fbc)
>  {
> -	struct intel_fbc *fbc = i915->fbc;
> -
> -	if (!fbc)
> -		return;
> +	struct drm_i915_private *i915 = fbc->i915;
>  
>  	cancel_work_sync(&fbc->underrun_work);
>  
> @@ -1544,6 +1555,38 @@ void intel_fbc_reset_underrun(struct drm_i915_private *i915)
>  	mutex_unlock(&fbc->lock);
>  }
>  
> +/*
> + * intel_fbc_reset_underrun - reset FBC fifo underrun status.
> + * @i915: the i915 device
> + *
> + * See intel_fbc_handle_fifo_underrun_irq(). For automated testing we
> + * want to re-enable FBC after an underrun to increase test coverage.
> + */
> +void intel_fbc_reset_underrun(struct drm_i915_private *i915)
> +{
> +	struct intel_fbc *fbc;
> +	enum fbc_id fbc_id;
> +
> +	for_each_intel_fbc(i915, fbc, fbc_id)
> +		__intel_fbc_reset_underrun(fbc);
> +}
> +
> +static void __intel_fbc_handle_fifo_underrun_irq(struct intel_fbc *fbc)
> +{
> +	/*
> +	 * There's no guarantee that underrun_detected won't be set to true
> +	 * right after this check and before the work is scheduled, but that's
> +	 * not a problem since we'll check it again under the work function
> +	 * while FBC is locked. This check here is just to prevent us from
> +	 * unnecessarily scheduling the work, and it relies on the fact that we
> +	 * never switch underrun_detect back to false after it's true.
> +	 */
> +	if (READ_ONCE(fbc->underrun_detected))
> +		return;
> +
> +	schedule_work(&fbc->underrun_work);
> +}
> +
>  /**
>   * intel_fbc_handle_fifo_underrun_irq - disable FBC when we get a FIFO underrun
>   * @i915: i915 device
> @@ -1560,21 +1603,11 @@ void intel_fbc_reset_underrun(struct drm_i915_private *i915)
>   */
>  void intel_fbc_handle_fifo_underrun_irq(struct drm_i915_private *i915)
>  {
> -	struct intel_fbc *fbc = i915->fbc;
> +	struct intel_fbc *fbc;
> +	enum fbc_id fbc_id;
>  
> -	if (!fbc)
> -		return;
> -
> -	/* There's no guarantee that underrun_detected won't be set to true
> -	 * right after this check and before the work is scheduled, but that's
> -	 * not a problem since we'll check it again under the work function
> -	 * while FBC is locked. This check here is just to prevent us from
> -	 * unnecessarily scheduling the work, and it relies on the fact that we
> -	 * never switch underrun_detect back to false after it's true. */
> -	if (READ_ONCE(fbc->underrun_detected))
> -		return;
> -
> -	schedule_work(&fbc->underrun_work);
> +	for_each_intel_fbc(i915, fbc, fbc_id)
> +		__intel_fbc_handle_fifo_underrun_irq(fbc);
>  }
>  
>  /*
> @@ -1685,7 +1718,7 @@ void intel_fbc_init(struct drm_i915_private *i915)
>  	if (intel_fbc_hw_is_active(fbc))
>  		intel_fbc_hw_deactivate(fbc);
>  
> -	i915->fbc = fbc;
> +	i915->fbc[fbc->id] = fbc;
>  }
>  
>  static int intel_fbc_debugfs_status_show(struct seq_file *m, void *unused)
> @@ -1778,8 +1811,9 @@ static void intel_fbc_debugfs_add(struct intel_fbc *fbc)
>  
>  void intel_fbc_debugfs_register(struct drm_i915_private *i915)
>  {
> -	struct intel_fbc *fbc = i915->fbc;
> +	struct intel_fbc *fbc;
> +	enum fbc_id fbc_id;
>  
> -	if (fbc)
> +	for_each_intel_fbc(i915, fbc, fbc_id)
>  		intel_fbc_debugfs_add(fbc);
>  }
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index d5359cf3d270..9e31eb54b9f4 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1829,7 +1829,7 @@ static struct intel_fbc *skl_plane_fbc(struct drm_i915_private *dev_priv,
>  				       enum pipe pipe, enum plane_id plane_id)
>  {
>  	if (skl_plane_has_fbc(dev_priv, pipe, plane_id))
> -		return dev_priv->fbc;
> +		return dev_priv->fbc[FBC_A];
>  	else
>  		return NULL;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a0f54a69b11d..7ae62e8e6d02 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -70,6 +70,7 @@
>  #include "display/intel_dmc.h"
>  #include "display/intel_dpll_mgr.h"
>  #include "display/intel_dsb.h"
> +#include "display/intel_fbc.h"
>  #include "display/intel_frontbuffer.h"
>  #include "display/intel_global_state.h"
>  #include "display/intel_gmbus.h"
> @@ -749,7 +750,7 @@ struct drm_i915_private {
>  	u32 pipestat_irq_mask[I915_MAX_PIPES];
>  
>  	struct i915_hotplug hotplug;
> -	struct intel_fbc *fbc;
> +	struct intel_fbc *fbc[I915_MAX_FBCS];
>  	struct i915_drrs drrs;
>  	struct intel_opregion opregion;
>  	struct intel_vbt_data vbt;

-- 
Jani Nikula, Intel Open Source Graphics Center




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

  Powered by Linux