On Thu, Nov 14, 2013 at 06:17:03PM +0100, Thomas Richter wrote: > Dear Daniel, dear intel-experts, > > please find a patch for the dreadful pipe-A underruns on the i830 > attached that works at least in the linear framebuffer mode. As far > as the tiling mode is concerned, I'm still scratching my head about > it. It as at this time unclear how precisely this works, but I'll > keep looking. > At least this is something.
A few bikesheds below. Once the polish is done I'll merge it. -Daniel > > So long, > Thomas > From ba72c1e506bb162f8ac595af94cc6ed850439883 Mon Sep 17 00:00:00 2001 > From: Thomas Richter <[email protected]> > Date: Thu, 14 Nov 2013 18:16:14 +0100 > Subject: [PATCH 11/11] Added a workaround for pipe-A underruns on i830 with > panning. > A bit commit message summarizing what has been done thus far would be good. Also please keep here a patch reflog, iirc we're at v3 or so. E.g. v3: Extract workaround into i830_update_plane_offset function. > > Signed-off-by: Thomas Richter <[email protected]> > --- > drivers/gpu/drm/i915/intel_display.c | 61 > ++++++++++++++++++++++++++++++++++ > 1 file changed, 61 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 971f6b9..21895b8 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2002,6 +2002,64 @@ unsigned long intel_gen4_compute_page_offset(int *x, > int *y, > } > } > > +/* > +** Update the linear frame pointer for i830 based devices. Due to some > +** unknown hardware feature, updating the frame pointer may cause > +** the unified FIFO of these chips run dry, then causing a flicker > +** and a potential shutdown of the video overlay, or worse. > +** thor, 14.11.2013 > +*/ Multi-line comments should be like this: /* * text ... */ with the text flown to align to 80 chars. vim will do this for you. > +static void i830_update_plane_offset(struct drm_crtc *crtc, > + struct drm_framebuffer *fb, > + unsigned long linear_offset) > +{ > + struct drm_i915_gem_object *obj; > + struct intel_framebuffer *intel_fb; > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + unsigned long planeadr, oldadr; > + int plane = intel_crtc->plane; > + u32 reg = DSPCNTR(plane); > + > + intel_fb = to_intel_framebuffer(fb); > + obj = intel_fb->obj; > + > + planeadr = i915_gem_obj_ggtt_offset(obj) + linear_offset; > + DRM_DEBUG_KMS("Plane address is 0x%lx", planeadr); I'd ditch this debug output due to dmesg spam. I guess it was useful for debugging, but should go away for upstream. > + /* I830 panning flicker work around. > + ** Only for non-LVDS output, only for i830. > + ** Tiling mode is still not supported. > + */ > + if (obj->tiling_mode != I915_TILING_NONE) { > + if ((planeadr & 0x40)) { We tend to use decimal numbers for limits like these. > + DRM_DEBUG_KMS("Detected potential flicker"); > + DRM_DEBUG_KMS("No workaround for tiling mode"); > + DRM_DEBUG_KMS("Use a linear frame buffer"); > + } I'd just ditch this for now, no point spamming dmesg with that much noise if we don't have a fix. > + } else { > + switch (fb->pixel_format) { > + case DRM_FORMAT_XRGB1555: > + case DRM_FORMAT_ARGB1555: > + case DRM_FORMAT_RGB565: > + case DRM_FORMAT_XRGB8888: > + case DRM_FORMAT_ARGB8888: > + case DRM_FORMAT_XBGR8888: > + case DRM_FORMAT_ABGR8888: The switch here can be killed - higher levels already check for valid pixel formats. > + oldadr = I915_READ(DSPADDR(plane)); > + if (((oldadr ^ planeadr) & 0x40) && > + (planeadr & 0xc0) == 0xc0) { > + I915_WRITE(DSPADDR(plane), > + planeadr & (~(0x80))); > + POSTING_READ(reg); > + intel_wait_for_vblank(dev, intel_crtc->pipe); > + } > + break; > + } > + } > + I915_WRITE(DSPADDR(plane), planeadr); > +} > + > static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer > *fb, > int x, int y) > { > @@ -2095,6 +2153,9 @@ static int i9xx_update_plane(struct drm_crtc *crtc, > struct drm_framebuffer *fb, > i915_gem_obj_ggtt_offset(obj) + > intel_crtc->dspaddr_offset); > I915_WRITE(DSPTILEOFF(plane), (y << 16) | x); > I915_WRITE(DSPLINOFF(plane), linear_offset); > + } else if (INTEL_INFO(dev)->gen == 2 && IS_I830(dev) && > + !intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) { IS_I830(dev) is good enough - the gen2 check is redundant and i830M doesn't have an LVDS encoder (lvds is only possible with the DVO encoder). > + i830_update_plane_offset(crtc, fb, linear_offset); > } else > I915_WRITE(DSPADDR(plane), i915_gem_obj_ggtt_offset(obj) + > linear_offset); > POSTING_READ(reg); > -- > 1.7.10.4 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
