| Message ID | 1303291342-27668-7-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 20 Apr 2011 10:22:22 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > For bonus amusement value, we perform the first load detect before even > establishing our fbdev. It seems like we need to be able to perform load detection in this case to create a suitable frame buffer at boot time. I also don't see a check to ensure that the fbdev frame buffer is large enough to cover the load detect mode; of course, it probably is given that the load detect mode is usually quite small, but... Seems like we need to allocate a temporary object for load detect mode setting in some cases.
On Wed, 20 Apr 2011 11:43:38 -0700, Keith Packard <keithp@keithp.com> wrote: > On Wed, 20 Apr 2011 10:22:22 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > For bonus amusement value, we perform the first load detect before even > > establishing our fbdev. > > It seems like we need to be able to perform load detection in this > case to create a suitable frame buffer at boot time. Aye, a classic chicken and egg problem. I wanted to ignore it as everything would have been resized upon the first probe by userspace (hotplug polling being disabled for load-detect paths), but only after the probe completes so there is the issue that the fb may be too small and so we don't prevent all potential PGTBL_ER. And VGA is laso likely to be a boot device. I'll spin up a patch for a temporary buffer and see what you think. -Chris
On Wed, 20 Apr 2011 19:55:10 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> I'll spin up a patch for a temporary buffer and see what you think.
Ok. I'd love to be able to use that code path all of the time, but I
don't like the thought of the cost of a temporary allocation every time
you do load detection.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9b1a3e1..e68dd08 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5549,6 +5549,8 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder, struct drm_encoder *encoder = &intel_encoder->base; struct drm_crtc *crtc = NULL; struct drm_device *dev = encoder->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_framebuffer *old_fb; int i = -1; /* @@ -5613,8 +5615,22 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder, if (!mode) mode = &load_detect_mode; - if (!drm_crtc_helper_set_mode(crtc, mode, 0, 0, crtc->fb)) { + /* Ensure we bind a framebuffer to supply the plane. + * As we may be called during intel_framebuffer_init, + * we need to be careful that we have actually initialised + * the fbcon before using it. + */ + if (dev_priv->fbdev == NULL || dev_priv->fbdev->ifb.obj == NULL) { + DRM_DEBUG("no fb to bind for load-detect pipe\n"); + return false; + } + + old_fb = crtc->fb; + crtc->fb = &dev_priv->fbdev->ifb.base; + + if (!drm_crtc_helper_set_mode(crtc, mode, 0, 0, old_fb)) { DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n"); + crtc->fb = old_fb; return false; }
We need to ensure that we feed valid memory into the display plane attached to the pipe when switching the pipe on. Otherwise, the display engine may read through an invalid PTE and so throw an PGTBL_ER exception. For bonus amusement value, we perform the first load detect before even establishing our fbdev. Reported-by: Knut Petersen <Knut_Petersen@t-online.de> References: https://bugs.freedesktop.org/show_bug.cgi?id=36246 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_display.c | 18 +++++++++++++++++- 1 files changed, 17 insertions(+), 1 deletions(-)