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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to