[Intel-gfx] [PATCH 6/6] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon v11

[Chris Wilson]

On Tue, Feb 11, 2014 at 03:29:01PM -0800, Jesse Barnes wrote:
> +/*
> + * Build an intel_fbdev struct using a BIOS allocated framebuffer, if possible.
> + * The core display code will have read out the current plane configuration,
> + * so we use that to figure out if there's an object for us to use as the
> + * fb, and if so, we re-use it for the fbdev configuration.
> + *
> + * Note we only support a single fb shared across pipes for boot (mostly for
> + * fbcon), so we just find the biggest and use that.
> + */
> +static bool intel_fbdev_init_bios(struct drm_device *dev,
> +				 struct intel_fbdev *ifbdev)
> +{
> +	struct intel_framebuffer *fb = NULL;
> +	struct drm_crtc *crtc;
> +	struct intel_crtc *intel_crtc;
> +	struct intel_plane_config *plane_config = NULL;
> +	unsigned int last_size = 0, max_size = 0;
> +
> +	if (!i915.fastboot)
> +		return false;
> +
> +	/* Find how big the fb would need to be for the largest pipe */
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {

Use struct intel_crtc as your iterator:
  list_for_each_entry(crtc, &dev->mode_config.crtc_list, head.base)
The few cases where you want crtc->base.fb merit the overall reduction
in noise.

> +		unsigned int tmp, stride;
> +
> +		intel_crtc = to_intel_crtc(crtc);
> +
> +		if (!intel_crtc->active || !intel_crtc->plane_config.fb) {
> +			DRM_DEBUG_KMS("pipe %c not active or no fb, skipping\n",
> +				      pipe_name(intel_crtc->pipe));
> +			continue;
> +		}
> +
> +		stride = intel_crtc->plane_config.fb->base.pitches[0];
> +		tmp = intel_crtc->config.adjusted_mode.crtc_vdisplay * stride;

This calculation is still flawed.

The fb you select later may have a massive stride, but this fb may be
narrow, i.e. crtc->vdisplay * preferred_fb->stride > preferred_fb->size
We don't subsequently check that the CRTC fits into the new fb.

> +		max_size = max(max_size, tmp);
> +	}


> +	/* Final pass to check if any active pipes don't have fbs */
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +		intel_crtc = to_intel_crtc(crtc);
> +
> +		if (!intel_crtc->active)
> +			continue;
> +
> +		WARN(!crtc->fb,
> +		     "re-used BIOS config but lost an fb on crtc %d\n",
> +		     crtc->base.id);

Does sanitize_crtc do the right thing? Or are we still displaying
garbage and triggering GPU hangs?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre