Re: [PATCH 1/5] drm/i915: make pitch_for_width take a tiled arg v2

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Nov 25, 2013 at 03:51:15PM -0800, Jesse Barnes wrote:
> And move it up in the file for earlier usage.
> 
> v2: add pre-gen4 support as well (Chris)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 53 ++++++++++++++++++++++++++----------
>  1 file changed, 38 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e85d838..321d751 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5452,6 +5452,28 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
>  	pipe_config->port_clock = clock.dot / 5;
>  }
>  
> +static u32
> +intel_framebuffer_pitch_for_width(struct drm_i915_private *dev_priv, int width,
> +				  int bpp, bool tiled)
> +{
> +	u32 pitch = DIV_ROUND_UP(width * bpp, 8);
> +	int align;
> +
> +	if (tiled) {
> +		if (INTEL_INFO(dev_priv->dev)->gen < 4) {
> +			/* Pre-965 needs power of two tile width */
> +			for (align = 512; align < pitch; align <<= 1)
> +				;
> +		} else {
> +			align = 512;
> +		}

Gen2 tiles are 128 bytes wide, not 512 bytes.

So maybe something like this:
 if (IS_GEN2()) {
   return roundup_power_of_two(max(pitch, 128));
 else if (IS_GEN3())
   return roundup_power_of_two(max(pitch, 512));
 else
   return ALIGN(pitch, 512);

> +	} else {
> +		align = 64;

Also I just noticed in the docs that gen2/3 only seem to require 32byte
alignment for linear buffers. But relaxing that would require a change
to intel_framebuffer_init() as well, so it looks like material for
another patch. Also would need to be tested on real hardware.

> +	}
> +
> +	return ALIGN(pitch, align);
> +}
> +
>  static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>  				 struct intel_crtc_config *pipe_config)
>  {
> @@ -7652,16 +7674,12 @@ err:
>  }
>  
>  static u32
> -intel_framebuffer_pitch_for_width(int width, int bpp)
> -{
> -	u32 pitch = DIV_ROUND_UP(width * bpp, 8);
> -	return ALIGN(pitch, 64);
> -}
> -
> -static u32
> -intel_framebuffer_size_for_mode(struct drm_display_mode *mode, int bpp)
> +intel_framebuffer_size_for_mode(struct drm_i915_private *dev_priv,
> +				struct drm_display_mode *mode, int bpp)
>  {
> -	u32 pitch = intel_framebuffer_pitch_for_width(mode->hdisplay, bpp);
> +	u32 pitch = intel_framebuffer_pitch_for_width(dev_priv,
> +						      mode->hdisplay, bpp,
> +						      false);
>  	return ALIGN(pitch * mode->vdisplay, PAGE_SIZE);

This should also align the fb height to tile height, otherwise
intel_framebuffer_init() might reject it.

>  }
>  
> @@ -7670,18 +7688,21 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
>  				  struct drm_display_mode *mode,
>  				  int depth, int bpp)
>  {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_i915_gem_object *obj;
>  	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> +	int size;
>  
> -	obj = i915_gem_alloc_object(dev,
> -				    intel_framebuffer_size_for_mode(mode, bpp));
> +	size =  intel_framebuffer_size_for_mode(dev_priv, mode, bpp);

> +	obj = i915_gem_alloc_object(dev, size);
>  	if (obj == NULL)
>  		return ERR_PTR(-ENOMEM);
>  
>  	mode_cmd.width = mode->hdisplay;
>  	mode_cmd.height = mode->vdisplay;
> -	mode_cmd.pitches[0] = intel_framebuffer_pitch_for_width(mode_cmd.width,
> -								bpp);
> +	mode_cmd.pitches[0] = intel_framebuffer_pitch_for_width(dev_priv,
> +								mode_cmd.width,
> +								bpp, false);
>  	mode_cmd.pixel_format = drm_mode_legacy_fb_format(bpp, depth);
>  
>  	return intel_framebuffer_create(dev, &mode_cmd, obj);
> @@ -7704,8 +7725,10 @@ mode_fits_in_fbdev(struct drm_device *dev,
>  		return NULL;
>  
>  	fb = &dev_priv->fbdev->ifb.base;
> -	if (fb->pitches[0] < intel_framebuffer_pitch_for_width(mode->hdisplay,
> -							       fb->bits_per_pixel))
> +	if (fb->pitches[0] < intel_framebuffer_pitch_for_width(dev_priv,
> +							       mode->hdisplay,
> +							       fb->bits_per_pixel,
> +							       false))
>  		return NULL;
>  
>  	if (obj->base.size < mode->vdisplay * fb->pitches[0])
> -- 
> 1.8.4.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC