Re: [PATCH] drm/i915: Add debugfs file to clear FIFO underruns.

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 28 Mar 2018, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
> Adding a i915_fifo_underrun_reset debugfs file will make it possible
> for IGT tests to clear FIFO underrun fallout at the start of each
> subtest, and make re-enable FBC so tests always have maximum exposure
> to features used by IGT. FIFO underruns and FBC bugs will no longer
> hide when an earlier subtests disables both.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105685
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105681

FWIW, ack on the idea, didn't look at the implementation.

BR,
Jani.

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  | 62 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++-------
>  drivers/gpu/drm/i915/intel_drv.h     |  3 ++
>  drivers/gpu/drm/i915/intel_fbc.c     | 26 +++++++++++++++
>  4 files changed, 109 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 7816cd53100a..27188de2c32b 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4731,6 +4731,67 @@ static int i915_drrs_ctl_set(void *data, u64 val)
>  
>  DEFINE_SIMPLE_ATTRIBUTE(i915_drrs_ctl_fops, NULL, i915_drrs_ctl_set, "%llu\n");
>  
> +static ssize_t
> +i915_fifo_underrun_reset_write(struct file *filp,
> +			       const char __user *ubuf,
> +			       size_t cnt, loff_t *ppos)
> +{
> +	struct drm_i915_private *dev_priv = filp->private_data;
> +	struct intel_crtc *intel_crtc;
> +	struct drm_device *dev = &dev_priv->drm;
> +	int ret;
> +	bool reset;
> +
> +	ret = kstrtobool_from_user(ubuf, cnt, &reset);
> +	if (ret)
> +		return ret;
> +
> +	if (!reset)
> +		return cnt;
> +
> +	for_each_intel_crtc(dev, intel_crtc) {
> +		struct drm_crtc_commit *commit;
> +		struct intel_crtc_state *crtc_state;
> +
> +		ret = drm_modeset_lock_single_interruptible(&intel_crtc->base.mutex);
> +		if (ret)
> +			return ret;
> +
> +		crtc_state = to_intel_crtc_state(intel_crtc->base.state);
> +		commit = crtc_state->base.commit;
> +		if (commit) {
> +			ret = wait_for_completion_interruptible(&commit->hw_done);
> +			if (!ret)
> +				ret = wait_for_completion_interruptible(&commit->flip_done);
> +		}
> +
> +		if (!ret && crtc_state->base.active) {
> +			DRM_DEBUG_KMS("Re-arming FIFO underruns on pipe %c\n",
> +				      pipe_name(intel_crtc->pipe));
> +
> +			intel_crtc_arm_fifo_underrun(intel_crtc, crtc_state);
> +		}
> +
> +		drm_modeset_unlock(&intel_crtc->base.mutex);
> +
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = intel_fbc_reset_underrun(dev_priv);
> +	if (ret)
> +		return ret;
> +
> +	return cnt;
> +}
> +
> +static const struct file_operations i915_fifo_underrun_reset_ops = {
> +	.owner = THIS_MODULE,
> +	.open = simple_open,
> +	.write = i915_fifo_underrun_reset_write,
> +	.llseek = default_llseek,
> +};
> +
>  static const struct drm_info_list i915_debugfs_list[] = {
>  	{"i915_capabilities", i915_capabilities, 0},
>  	{"i915_gem_objects", i915_gem_object_info, 0},
> @@ -4798,6 +4859,7 @@ static const struct i915_debugfs_files {
>  	{"i915_error_state", &i915_error_state_fops},
>  	{"i915_gpu_info", &i915_gpu_info_fops},
>  #endif
> +	{"i915_fifo_underrun_reset", &i915_fifo_underrun_reset_ops},
>  	{"i915_next_seqno", &i915_next_seqno_fops},
>  	{"i915_display_crc_ctl", &i915_display_crc_ctl_fops},
>  	{"i915_pri_wm_latency", &i915_pri_wm_latency_fops},
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4c30c7c04f9c..3df0e8193b83 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12960,10 +12960,25 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
>  							   intel_cstate);
>  }
>  
> +void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,
> +				  struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +
> +	if (!IS_GEN2(dev_priv))
> +		intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true);
> +
> +	if (crtc_state->has_pch_encoder) {
> +		enum pipe pch_transcoder =
> +			intel_crtc_pch_transcoder(crtc);
> +
> +		intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder, true);
> +	}
> +}
> +
>  static void intel_finish_crtc_commit(struct drm_crtc *crtc,
>  				     struct drm_crtc_state *old_crtc_state)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct intel_atomic_state *old_intel_state =
>  		to_intel_atomic_state(old_crtc_state->state);
> @@ -12974,17 +12989,8 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc,
>  
>  	if (new_crtc_state->update_pipe &&
>  	    !needs_modeset(&new_crtc_state->base) &&
> -	    old_crtc_state->mode.private_flags & I915_MODE_FLAG_INHERITED) {
> -		if (!IS_GEN2(dev_priv))
> -			intel_set_cpu_fifo_underrun_reporting(dev_priv, intel_crtc->pipe, true);
> -
> -		if (new_crtc_state->has_pch_encoder) {
> -			enum pipe pch_transcoder =
> -				intel_crtc_pch_transcoder(intel_crtc);
> -
> -			intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder, true);
> -		}
> -	}
> +	    old_crtc_state->mode.private_flags & I915_MODE_FLAG_INHERITED)
> +		intel_crtc_arm_fifo_underrun(intel_crtc, new_crtc_state);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d1452fd2a58d..ca005c989e9f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1592,6 +1592,8 @@ void hsw_disable_ips(const struct intel_crtc_state *crtc_state);
>  enum intel_display_power_domain intel_port_to_power_domain(enum port port);
>  void intel_mode_from_pipe_config(struct drm_display_mode *mode,
>  				 struct intel_crtc_state *pipe_config);
> +void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,
> +				  struct intel_crtc_state *crtc_state);
>  
>  int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
>  int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
> @@ -1777,6 +1779,7 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
>  		     unsigned int frontbuffer_bits, enum fb_op_origin origin);
>  void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv);
>  void intel_fbc_handle_fifo_underrun_irq(struct drm_i915_private *dev_priv);
> +int intel_fbc_reset_underrun(struct drm_i915_private *dev_priv);
>  
>  /* intel_hdmi.c */
>  void intel_hdmi_init(struct drm_i915_private *dev_priv, i915_reg_t hdmi_reg,
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 707d49c12638..308bdd136d33 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -1272,6 +1272,32 @@ static void intel_fbc_underrun_work_fn(struct work_struct *work)
>  	mutex_unlock(&fbc->lock);
>  }
>  
> +/*
> + * intel_fbc_reset_underrun - reset FBC fifo underrun status.
> + * @dev_priv: i915 device instance
> + *
> + * See intel_fbc_handle_fifo_underrun_irq(). For automated testing we
> + * want to re-enable FBC after an underrun to increase test coverage.
> + */
> +int intel_fbc_reset_underrun(struct drm_i915_private *dev_priv)
> +{
> +	int ret;
> +
> +	cancel_work_sync(&dev_priv->fbc.underrun_work);
> +
> +	ret = mutex_lock_interruptible(&dev_priv->fbc.lock);
> +	if (ret)
> +		return ret;
> +
> +	if (dev_priv->fbc.underrun_detected)
> +		DRM_DEBUG_KMS("Re-allowing FBC after fifo underrun\n");
> +
> +	dev_priv->fbc.underrun_detected = false;
> +	mutex_unlock(&dev_priv->fbc.lock);
> +
> +	return 0;
> +}
> +
>  /**
>   * intel_fbc_handle_fifo_underrun_irq - disable FBC when we get a FIFO underrun
>   * @dev_priv: i915 device instance

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx