On Fri,  2 Nov 2012 19:55:06 +0100
Daniel Vetter <daniel.vet...@ffwll.ch> wrote:

> Unfortunately this makes it clearer that our fbc code is ... somewhat hackish
> and racy.
> 
> Signed-off-by: Daniel Vetter <daniel.vet...@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c    |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h        | 24 ++++++++++-------
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 14 +++++-----
>  drivers/gpu/drm/i915/intel_display.c   |  6 ++---
>  drivers/gpu/drm/i915/intel_pm.c        | 48 
> +++++++++++++++++-----------------
>  5 files changed, 49 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 598987f..ea9dfb8 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1184,7 +1184,7 @@ static int i915_fbc_status(struct seq_file *m, void 
> *unused)
>               seq_printf(m, "FBC enabled\n");
>       } else {
>               seq_printf(m, "FBC disabled: ");
> -             switch (dev_priv->no_fbc_reason) {
> +             switch (dev_priv->fbc.no_fbc_reason) {
>               case FBC_NO_OUTPUT:
>                       seq_printf(m, "no outputs");
>                       break;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 98b52e7..07da179 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -599,6 +599,19 @@ struct i915_dri1_state {
>       uint32_t counter;
>  };
>  
> +struct intel_fbc_state {
> +     enum no_fbc_reason no_fbc_reason;
> +
> +     struct drm_mm_node *stolen_mem_cfb;
> +     struct drm_mm_node *stolen_mem_llb;
> +
> +     unsigned long size;
> +     unsigned int fb;
> +     enum plane plane;
> +     int y;
> +     struct intel_fbc_work *work;
> +};
> +
>  typedef struct drm_i915_private {
>       struct drm_device *dev;
>  
> @@ -665,12 +678,6 @@ typedef struct drm_i915_private {
>  
>       unsigned int stop_rings;
>  
> -     unsigned long cfb_size;
> -     unsigned int cfb_fb;
> -     enum plane cfb_plane;
> -     int cfb_y;
> -     struct intel_fbc_work *fbc_work;
> -
>       struct intel_opregion opregion;
>  
>       /* overlay */
> @@ -872,10 +879,7 @@ typedef struct drm_i915_private {
>        * mchdev_lock in intel_pm.c */
>       struct intel_ilk_power_mgmt ips;
>  
> -     enum no_fbc_reason no_fbc_reason;
> -
> -     struct drm_mm_node *compressed_fb;
> -     struct drm_mm_node *compressed_llb;
> +     struct intel_fbc_state fbc;
>  
>       unsigned long last_gpu_reset;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 8e91083..9f67aef 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -134,9 +134,9 @@ static void i915_setup_compression(struct drm_device 
> *dev, int size)
>                       goto err_llb;
>       }
>  
> -     dev_priv->cfb_size = size;
> +     dev_priv->fbc.size = size;
>  
> -     dev_priv->compressed_fb = compressed_fb;
> +     dev_priv->fbc.stolen_mem_cfb = compressed_fb;
>       if (HAS_PCH_SPLIT(dev))
>               I915_WRITE(ILK_DPFC_CB_BASE, compressed_fb->start);
>       else if (IS_GM45(dev)) {
> @@ -144,7 +144,7 @@ static void i915_setup_compression(struct drm_device 
> *dev, int size)
>       } else {
>               I915_WRITE(FBC_CFB_BASE, cfb_base);
>               I915_WRITE(FBC_LL_BASE, ll_base);
> -             dev_priv->compressed_llb = compressed_llb;
> +             dev_priv->fbc.stolen_mem_llb = compressed_llb;
>       }
>  
>       DRM_DEBUG_KMS("FBC base 0x%08lx, ll base 0x%08lx, size %dM\n",
> @@ -156,7 +156,7 @@ err_llb:
>  err_fb:
>       drm_mm_put_block(compressed_fb);
>  err:
> -     dev_priv->no_fbc_reason = FBC_STOLEN_TOO_SMALL;
> +     dev_priv->fbc.no_fbc_reason = FBC_STOLEN_TOO_SMALL;
>       i915_warn_stolen(dev);
>  }
>  
> @@ -164,9 +164,9 @@ static void i915_cleanup_compression(struct drm_device 
> *dev)
>  {
>       struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -     drm_mm_put_block(dev_priv->compressed_fb);
> -     if (dev_priv->compressed_llb)
> -             drm_mm_put_block(dev_priv->compressed_llb);
> +     drm_mm_put_block(dev_priv->fbc.stolen_mem_cfb);
> +     if (dev_priv->fbc.stolen_mem_llb)
> +             drm_mm_put_block(dev_priv->fbc.stolen_mem_llb);
>  }
>  
>  void i915_gem_cleanup_stolen(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 9540d72..e6459ca 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3541,7 +3541,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>  
>       intel_disable_plane(dev_priv, plane, pipe);
>  
> -     if (dev_priv->cfb_plane == plane)
> +     if (dev_priv->fbc.plane == plane)
>               intel_disable_fbc(dev);
>  
>       intel_disable_pipe(dev_priv, pipe);
> @@ -3623,7 +3623,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>  
>       intel_disable_plane(dev_priv, plane, pipe);
>  
> -     if (dev_priv->cfb_plane == plane)
> +     if (dev_priv->fbc.plane == plane)
>               intel_disable_fbc(dev);
>  
>       intel_disable_pipe(dev_priv, pipe);
> @@ -3743,7 +3743,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>       intel_crtc_dpms_overlay(intel_crtc, false);
>       intel_crtc_update_cursor(crtc, false);
>  
> -     if (dev_priv->cfb_plane == plane)
> +     if (dev_priv->fbc.plane == plane)
>               intel_disable_fbc(dev);
>  
>       intel_disable_plane(dev_priv, plane, pipe);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e5cff68..3870220 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -78,7 +78,7 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc, unsigned 
> long interval)
>       int plane, i;
>       u32 fbc_ctl, fbc_ctl2;
>  
> -     cfb_pitch = dev_priv->cfb_size / FBC_LL_SIZE;
> +     cfb_pitch = dev_priv->fbc.size / FBC_LL_SIZE;
>       if (fb->pitches[0] < cfb_pitch)
>               cfb_pitch = fb->pitches[0];
>  
> @@ -264,7 +264,7 @@ static void intel_fbc_work_fn(struct work_struct *__work)
>       struct drm_i915_private *dev_priv = dev->dev_private;
>  
>       mutex_lock(&dev->struct_mutex);
> -     if (work == dev_priv->fbc_work) {
> +     if (work == dev_priv->fbc.work) {
>               /* Double check that we haven't switched fb without cancelling
>                * the prior work.
>                */
> @@ -272,12 +272,12 @@ static void intel_fbc_work_fn(struct work_struct 
> *__work)
>                       dev_priv->display.enable_fbc(work->crtc,
>                                                    work->interval);
>  
> -                     dev_priv->cfb_plane = to_intel_crtc(work->crtc)->plane;
> -                     dev_priv->cfb_fb = work->crtc->fb->base.id;
> -                     dev_priv->cfb_y = work->crtc->y;
> +                     dev_priv->fbc.plane = to_intel_crtc(work->crtc)->plane;
> +                     dev_priv->fbc.fb = work->crtc->fb->base.id;
> +                     dev_priv->fbc.y = work->crtc->y;
>               }
>  
> -             dev_priv->fbc_work = NULL;
> +             dev_priv->fbc.work = NULL;
>       }
>       mutex_unlock(&dev->struct_mutex);
>  
> @@ -286,7 +286,7 @@ static void intel_fbc_work_fn(struct work_struct *__work)
>  
>  static void intel_cancel_fbc_work(struct drm_i915_private *dev_priv)
>  {
> -     if (dev_priv->fbc_work == NULL)
> +     if (dev_priv->fbc.work == NULL)
>               return;
>  
>       DRM_DEBUG_KMS("cancelling pending FBC enable\n");
> @@ -295,16 +295,16 @@ static void intel_cancel_fbc_work(struct 
> drm_i915_private *dev_priv)
>        * dev_priv->fbc_work, so we can perform the cancellation
>        * entirely asynchronously.
>        */
> -     if (cancel_delayed_work(&dev_priv->fbc_work->work))
> +     if (cancel_delayed_work(&dev_priv->fbc.work->work))
>               /* tasklet was killed before being run, clean up */
> -             kfree(dev_priv->fbc_work);
> +             kfree(dev_priv->fbc.work);
>  
>       /* Mark the work as no longer wanted so that if it does
>        * wake-up (because the work was already running and waiting
>        * for our mutex), it will discover that is no longer
>        * necessary to run.
>        */
> -     dev_priv->fbc_work = NULL;
> +     dev_priv->fbc.work = NULL;
>  }
>  
>  void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
> @@ -329,7 +329,7 @@ void intel_enable_fbc(struct drm_crtc *crtc, unsigned 
> long interval)
>       work->interval = interval;
>       INIT_DELAYED_WORK(&work->work, intel_fbc_work_fn);
>  
> -     dev_priv->fbc_work = work;
> +     dev_priv->fbc.work = work;
>  
>       DRM_DEBUG_KMS("scheduling delayed FBC enable\n");
>  
> @@ -357,7 +357,7 @@ void intel_disable_fbc(struct drm_device *dev)
>               return;
>  
>       dev_priv->display.disable_fbc(dev);
> -     dev_priv->cfb_plane = -1;
> +     dev_priv->fbc.plane = -1;
>  }
>  
>  /**
> @@ -410,7 +410,7 @@ void intel_update_fbc(struct drm_device *dev)
>                   tmp_crtc->fb) {
>                       if (crtc) {
>                               DRM_DEBUG_KMS("more than one pipe active, 
> disabling compression\n");
> -                             dev_priv->no_fbc_reason = FBC_MULTIPLE_PIPES;
> +                             dev_priv->fbc.no_fbc_reason = 
> FBC_MULTIPLE_PIPES;
>                               goto out_disable;
>                       }
>                       crtc = tmp_crtc;
> @@ -419,7 +419,7 @@ void intel_update_fbc(struct drm_device *dev)
>  
>       if (!crtc || crtc->fb == NULL) {
>               DRM_DEBUG_KMS("no output, disabling\n");
> -             dev_priv->no_fbc_reason = FBC_NO_OUTPUT;
> +             dev_priv->fbc.no_fbc_reason = FBC_NO_OUTPUT;
>               goto out_disable;
>       }
>  
> @@ -437,31 +437,31 @@ void intel_update_fbc(struct drm_device *dev)
>       }
>       if (!enable_fbc) {
>               DRM_DEBUG_KMS("fbc disabled per module param\n");
> -             dev_priv->no_fbc_reason = FBC_MODULE_PARAM;
> +             dev_priv->fbc.no_fbc_reason = FBC_MODULE_PARAM;
>               goto out_disable;
>       }
> -     if (intel_fb->obj->base.size > dev_priv->cfb_size) {
> +     if (intel_fb->obj->base.size > dev_priv->fbc.size) {
>               DRM_DEBUG_KMS("framebuffer too large, disabling "
>                             "compression\n");
> -             dev_priv->no_fbc_reason = FBC_STOLEN_TOO_SMALL;
> +             dev_priv->fbc.no_fbc_reason = FBC_STOLEN_TOO_SMALL;
>               goto out_disable;
>       }
>       if ((crtc->mode.flags & DRM_MODE_FLAG_INTERLACE) ||
>           (crtc->mode.flags & DRM_MODE_FLAG_DBLSCAN)) {
>               DRM_DEBUG_KMS("mode incompatible with compression, "
>                             "disabling\n");
> -             dev_priv->no_fbc_reason = FBC_UNSUPPORTED_MODE;
> +             dev_priv->fbc.no_fbc_reason = FBC_UNSUPPORTED_MODE;
>               goto out_disable;
>       }
>       if ((crtc->mode.hdisplay > 2048) ||
>           (crtc->mode.vdisplay > 1536)) {
>               DRM_DEBUG_KMS("mode too large for compression, disabling\n");
> -             dev_priv->no_fbc_reason = FBC_MODE_TOO_LARGE;
> +             dev_priv->fbc.no_fbc_reason = FBC_MODE_TOO_LARGE;
>               goto out_disable;
>       }
>       if ((IS_I915GM(dev) || IS_I945GM(dev)) && intel_crtc->plane != 0) {
>               DRM_DEBUG_KMS("plane not 0, disabling compression\n");
> -             dev_priv->no_fbc_reason = FBC_BAD_PLANE;
> +             dev_priv->fbc.no_fbc_reason = FBC_BAD_PLANE;
>               goto out_disable;
>       }
>  
> @@ -471,7 +471,7 @@ void intel_update_fbc(struct drm_device *dev)
>       if (obj->tiling_mode != I915_TILING_X ||
>           obj->fence_reg == I915_FENCE_REG_NONE) {
>               DRM_DEBUG_KMS("framebuffer not tiled or fenced, disabling 
> compression\n");
> -             dev_priv->no_fbc_reason = FBC_NOT_TILED;
> +             dev_priv->fbc.no_fbc_reason = FBC_NOT_TILED;
>               goto out_disable;
>       }
>  
> @@ -484,9 +484,9 @@ void intel_update_fbc(struct drm_device *dev)
>        * cannot be unpinned (and have its GTT offset and fence revoked)
>        * without first being decoupled from the scanout and FBC disabled.
>        */
> -     if (dev_priv->cfb_plane == intel_crtc->plane &&
> -         dev_priv->cfb_fb == fb->base.id &&
> -         dev_priv->cfb_y == crtc->y)
> +     if (dev_priv->fbc.plane == intel_crtc->plane &&
> +         dev_priv->fbc.fb == fb->base.id &&
> +         dev_priv->fbc.y == crtc->y)
>               return;
>  
>       if (intel_fbc_enabled(dev)) {

Reviewed-by: Jesse Barnes <jbar...@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to