[Intel-gfx] [PATCH] drm/i915: Align initial plane backing objects correctly

[Chris Wilson]

On Tue, Feb 10, 2015 at 11:10:51PM +0100, Daniel Vetter wrote:
> Some bios really like to joke and start the planes at an offset ...
> hooray!
> 
> Align start and end to fix this.
> 
> v2: Fixup calculation of size, spotted by Chris Wilson.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86883
> Cc: stable at vger.kernel.org
> Cc: Johannes W <jargon at molb.org>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Reported-by: Johannes W <jargon at molb.org>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> 
> --
> 
> Johannes, can you please test this patch instead of the one from
> Chris?
> 
> Thanks, Daniel
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c |  5 +----
>  drivers/gpu/drm/i915/intel_display.c   | 18 ++++++++++++------
>  2 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 59401f3b902c..4382696087c9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -485,10 +485,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
>  			stolen_offset, gtt_offset, size);
>  
>  	/* KISS and expect everything to be page-aligned */
> -	BUG_ON(stolen_offset & 4095);
> -	BUG_ON(size & 4095);
> -
> -	if (WARN_ON(size == 0))
> +	if (WARN_ON(size == 0 || stolen_offset & 4095 || size & 4095))
>  		return NULL;

I'd vote for keeping these as two separate WARNs, or using a format
string to print the bogus values.
  
>  	stolen = kzalloc(sizeof(*stolen), GFP_KERNEL);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2655b63d65e9..308a620f2594 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2372,13 +2372,19 @@ intel_alloc_plane_obj(struct intel_crtc *crtc,
>  	struct drm_i915_gem_object *obj = NULL;
>  	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
>  	struct drm_framebuffer *fb = &plane_config->fb->base;
> -	u32 base = plane_config->base;
> +	u32 base_aligned = round_down(plane_config->base, PAGE_SIZE);
> +	u32 size_aligned = round_up(plane_config->base + plane_config->size,
> +				    PAGE_SIZE);
> +
> +	size_aligned -= base_aligned;
>  
>  	if (plane_config->size == 0)
>  		return false;
>  
> -	obj = i915_gem_object_create_stolen_for_preallocated(dev, base, base,
> -							     plane_config->size);

Didn't we used to have a comment here wondering if the 1:1 mapping was
sane. That feels like it might be a factor here. Given that that our
plane_obj does not match the one used by the BIOS, do we have some
mechanism for forcing the modeset (presuming that fastboot is otherwise
successful)?

> +	obj = i915_gem_object_create_stolen_for_preallocated(dev,
> +							     base_aligned,
> +							     base_aligned,
> +							     size_aligned);
>  	if (!obj)
>  		return false;

> @@ -7694,7 +7700,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
>  	aligned_height = intel_fb_align_height(dev, fb->height,
>  					       plane_config->tiling);
>  
> -	plane_config->size = ALIGN(fb->pitches[0] * aligned_height, PAGE_SIZE);
> +	plane_config->size = fb->pitches[0] * aligned_height, PAGE_SIZE;

Stray comma operator.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre