Re: [PATCH 20/66] drm/i915: Move fbc members out of line

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Jun 27, 2013 at 04:30:21PM -0700, Ben Widawsky wrote:
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

Merged patches 16-20, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c    |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h        | 48 +++++++++++++++++++--------------
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 20 +++++++-------
>  drivers/gpu/drm/i915/intel_display.c   |  6 ++---
>  drivers/gpu/drm/i915/intel_drv.h       |  7 -----
>  drivers/gpu/drm/i915/intel_pm.c        | 49 +++++++++++++++++-----------------
>  6 files changed, 67 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index d4e78b6..e654bf4 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1444,7 +1444,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 efd244d..21cf593 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -534,17 +534,35 @@ struct i915_hw_context {
>  	struct i915_hw_ppgtt ppgtt;
>  };
>  
> -enum no_fbc_reason {
> -	FBC_NO_OUTPUT, /* no outputs enabled to compress */
> -	FBC_STOLEN_TOO_SMALL, /* not enough space to hold compressed buffers */
> -	FBC_UNSUPPORTED_MODE, /* interlace or doublescanned mode */
> -	FBC_MODE_TOO_LARGE, /* mode too large for compression */
> -	FBC_BAD_PLANE, /* fbc not supported on plane */
> -	FBC_NOT_TILED, /* buffer not tiled */
> -	FBC_MULTIPLE_PIPES, /* more than one pipe active */
> -	FBC_MODULE_PARAM,
> +struct i915_fbc {
> +	unsigned long size;
> +	unsigned int fb_id;
> +	enum plane plane;
> +	int y;
> +
> +	struct drm_mm_node *compressed_fb;
> +	struct drm_mm_node *compressed_llb;
> +
> +	struct intel_fbc_work {
> +		struct delayed_work work;
> +		struct drm_crtc *crtc;
> +		struct drm_framebuffer *fb;
> +		int interval;
> +	} *fbc_work;
> +
> +	enum {
> +		FBC_NO_OUTPUT, /* no outputs enabled to compress */
> +		FBC_STOLEN_TOO_SMALL, /* not enough space for buffers */
> +		FBC_UNSUPPORTED_MODE, /* interlace or doublescanned mode */
> +		FBC_MODE_TOO_LARGE, /* mode too large for compression */
> +		FBC_BAD_PLANE, /* fbc not supported on plane */
> +		FBC_NOT_TILED, /* buffer not tiled */
> +		FBC_MULTIPLE_PIPES, /* more than one pipe active */
> +		FBC_MODULE_PARAM,
> +	} no_fbc_reason;
>  };
>  
> +
>  enum intel_pch {
>  	PCH_NONE = 0,	/* No PCH present */
>  	PCH_IBX,	/* Ibexpeak PCH */
> @@ -1064,12 +1082,7 @@ typedef struct drm_i915_private {
>  
>  	int num_plane;
>  
> -	unsigned long cfb_size;
> -	unsigned int cfb_fb;
> -	enum plane cfb_plane;
> -	int cfb_y;
> -	struct intel_fbc_work *fbc_work;
> -
> +	struct i915_fbc fbc;
>  	struct intel_opregion opregion;
>  	struct intel_vbt_data vbt;
>  
> @@ -1147,11 +1160,6 @@ typedef struct drm_i915_private {
>  	/* Haswell power well */
>  	struct i915_power_well power_well;
>  
> -	enum no_fbc_reason no_fbc_reason;
> -
> -	struct drm_mm_node *compressed_fb;
> -	struct drm_mm_node *compressed_llb;
> -
>  	struct i915_gpu_error gpu_error;
>  
>  	struct drm_i915_gem_object *vlv_pctx;
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index f713294..8e02344 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -120,7 +120,7 @@ static int i915_setup_compression(struct drm_device *dev, int size)
>  		if (!compressed_llb)
>  			goto err_fb;
>  
> -		dev_priv->compressed_llb = compressed_llb;
> +		dev_priv->fbc.compressed_llb = compressed_llb;
>  
>  		I915_WRITE(FBC_CFB_BASE,
>  			   dev_priv->mm.stolen_base + compressed_fb->start);
> @@ -128,8 +128,8 @@ static int i915_setup_compression(struct drm_device *dev, int size)
>  			   dev_priv->mm.stolen_base + compressed_llb->start);
>  	}
>  
> -	dev_priv->compressed_fb = compressed_fb;
> -	dev_priv->cfb_size = size;
> +	dev_priv->fbc.compressed_fb = compressed_fb;
> +	dev_priv->fbc.size = size;
>  
>  	DRM_DEBUG_KMS("reserved %d bytes of contiguous stolen space for FBC\n",
>  		      size);
> @@ -150,7 +150,7 @@ int i915_gem_stolen_setup_compression(struct drm_device *dev, int size)
>  	if (dev_priv->mm.stolen_base == 0)
>  		return -ENODEV;
>  
> -	if (size < dev_priv->cfb_size)
> +	if (size < dev_priv->fbc.size)
>  		return 0;
>  
>  	/* Release any current block */
> @@ -163,16 +163,16 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	if (dev_priv->cfb_size == 0)
> +	if (dev_priv->fbc.size == 0)
>  		return;
>  
> -	if (dev_priv->compressed_fb)
> -		drm_mm_put_block(dev_priv->compressed_fb);
> +	if (dev_priv->fbc.compressed_fb)
> +		drm_mm_put_block(dev_priv->fbc.compressed_fb);
>  
> -	if (dev_priv->compressed_llb)
> -		drm_mm_put_block(dev_priv->compressed_llb);
> +	if (dev_priv->fbc.compressed_llb)
> +		drm_mm_put_block(dev_priv->fbc.compressed_llb);
>  
> -	dev_priv->cfb_size = 0;
> +	dev_priv->fbc.size = 0;
>  }
>  
>  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 8d075b1f..f056eca 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3391,7 +3391,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>  	intel_crtc_wait_for_pending_flips(crtc);
>  	drm_vblank_off(dev, pipe);
>  
> -	if (dev_priv->cfb_plane == plane)
> +	if (dev_priv->fbc.plane == plane)
>  		intel_disable_fbc(dev);
>  
>  	intel_crtc_update_cursor(crtc, false);
> @@ -3464,7 +3464,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>  	drm_vblank_off(dev, pipe);
>  
>  	/* FBC must be disabled before disabling the plane on HSW. */
> -	if (dev_priv->cfb_plane == plane)
> +	if (dev_priv->fbc.plane == plane)
>  		intel_disable_fbc(dev);
>  
>  	hsw_disable_ips(intel_crtc);
> @@ -3705,7 +3705,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>  	intel_crtc_wait_for_pending_flips(crtc);
>  	drm_vblank_off(dev, pipe);
>  
> -	if (dev_priv->cfb_plane == plane)
> +	if (dev_priv->fbc.plane == plane)
>  		intel_disable_fbc(dev);
>  
>  	intel_crtc_dpms_overlay(intel_crtc, false);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ffe9d35..af68861 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -548,13 +548,6 @@ struct intel_unpin_work {
>  	bool enable_stall_check;
>  };
>  
> -struct intel_fbc_work {
> -	struct delayed_work work;
> -	struct drm_crtc *crtc;
> -	struct drm_framebuffer *fb;
> -	int interval;
> -};
> -
>  int intel_pch_rawclk(struct drm_device *dev);
>  
>  int intel_connector_update_modes(struct drm_connector *connector,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b27bda0..d32734d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -86,7 +86,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];
>  
> @@ -325,7 +325,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.fbc_work) {
>  		/* Double check that we haven't switched fb without cancelling
>  		 * the prior work.
>  		 */
> @@ -333,12 +333,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_id = work->crtc->fb->base.id;
> +			dev_priv->fbc.y = work->crtc->y;
>  		}
>  
> -		dev_priv->fbc_work = NULL;
> +		dev_priv->fbc.fbc_work = NULL;
>  	}
>  	mutex_unlock(&dev->struct_mutex);
>  
> @@ -347,25 +347,25 @@ 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.fbc_work == NULL)
>  		return;
>  
>  	DRM_DEBUG_KMS("cancelling pending FBC enable\n");
>  
>  	/* Synchronisation is provided by struct_mutex and checking of
> -	 * dev_priv->fbc_work, so we can perform the cancellation
> +	 * dev_priv->fbc.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.fbc_work->work))
>  		/* tasklet was killed before being run, clean up */
> -		kfree(dev_priv->fbc_work);
> +		kfree(dev_priv->fbc.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.fbc_work = NULL;
>  }
>  
>  void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
> @@ -390,7 +390,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.fbc_work = work;
>  
>  	DRM_DEBUG_KMS("scheduling delayed FBC enable\n");
>  
> @@ -418,7 +418,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;
>  }
>  
>  /**
> @@ -471,7 +471,8 @@ void intel_update_fbc(struct drm_device *dev)
>  		    !to_intel_crtc(tmp_crtc)->primary_disabled) {
>  			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;
> @@ -480,7 +481,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;
>  	}
>  
> @@ -498,14 +499,14 @@ 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 ((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;
>  	}
>  
> @@ -519,13 +520,13 @@ void intel_update_fbc(struct drm_device *dev)
>  	if ((crtc->mode.hdisplay > max_hdisplay) ||
>  	    (crtc->mode.vdisplay > max_vdisplay)) {
>  		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) || IS_HASWELL(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;
>  	}
>  
> @@ -535,7 +536,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;
>  	}
>  
> @@ -545,7 +546,7 @@ void intel_update_fbc(struct drm_device *dev)
>  
>  	if (i915_gem_stolen_setup_compression(dev, intel_fb->obj->base.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;
>  	}
>  
> @@ -554,9 +555,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_id == fb->base.id &&
> +	    dev_priv->fbc.y == crtc->y)
>  		return;
>  
>  	if (intel_fbc_enabled(dev)) {
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch