Hi Rodrigo,
On 5 November 2015 at 18:49, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> wrote:
> diff --git a/drivers/gpu/drm/i915/intel_ips.c b/drivers/gpu/drm/i915/intel_ips.c
> index b867aba..6bc5c55 100644
> --- a/drivers/gpu/drm/i915/intel_ips.c
> +++ b/drivers/gpu/drm/i915/intel_ips.c
> @@ -105,18 +105,21 @@ bool intel_ips_ready(struct intel_crtc *crtc,
> * This function is called to enable IPS on certain pipe.
> * All needed conditions should've checked already by intel_ips_ready.
> */
> -void intel_ips_enable(struct intel_crtc *crtc)
> +int intel_ips_enable(struct intel_crtc *crtc)
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> + int ret = 0;
>
> if (!crtc->config->ips_ready)
> - return;
> + return -EINVAL;
>
> mutex_lock(&dev_priv->display_ips.lock);
>
> - if (dev_priv->display_ips.enabled)
> + if (dev_priv->display_ips.enabled) {
> + ret = -EALREADY;
> goto out;
> + }
>
> /*
> * We can only enable IPS after we enable a plane
> @@ -147,6 +150,7 @@ void intel_ips_enable(struct intel_crtc *crtc)
> */
> if (wait_for(I915_READ_NOTRACE(IPS_CTL) & IPS_ENABLE, 50)) {
> DRM_ERROR("Timed out waiting for IPS enable\n");
> + ret = -ETIMEDOUT;
> goto out;
> }
> }
> @@ -154,6 +158,7 @@ void intel_ips_enable(struct intel_crtc *crtc)
> dev_priv->display_ips.enabled = true;
> out:
> mutex_unlock(&dev_priv->display_ips.lock);
> + return ret;
> }
>
> /**
> @@ -162,16 +167,22 @@ out:
> *
> * This function is called to disable IPS on certain pipe whenever it is needed
> * to disable IPS on the pipe.
> + *
> + * Returns:
> + * 0 on success and -errno otherwise.
> */
> -void intel_ips_disable(struct intel_crtc *crtc)
> +int intel_ips_disable(struct intel_crtc *crtc)
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> + int ret = 0;
>
> mutex_lock(&dev_priv->display_ips.lock);
>
> - if (!dev_priv->display_ips.enabled)
> + if (!dev_priv->display_ips.enabled) {
> + ret = -EALREADY;
> goto out;
> + }
>
> assert_plane_enabled(dev_priv, crtc->plane);
> if (IS_BROADWELL(dev)) {
> @@ -196,6 +207,7 @@ void intel_ips_disable(struct intel_crtc *crtc)
> dev_priv->display_ips.enabled = false;
> out:
> mutex_unlock(&dev_priv->display_ips.lock);
> + return ret;
> }
It would be nice to have these from the beginning, rather than
modifying them part-way through.
> @@ -206,6 +218,9 @@ out:
> * It checks if there is any other plane enabled on the pipe when primary is
> * going to be disabled. In this case IPS can continue enabled, but it needs
> * to be disabled otherwise.
> + *
> + * Returns:
> + * 0 on success and -errno otherwise.
> */
> void intel_ips_disable_if_alone(struct intel_crtc *crtc)
> {
... this one is still left void.
Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
