On Sun, 8 Aug 2010 10:20:25 +0100 Chris Wilson <[email protected]> wrote:
> i965 uses the Display Registers to compute the offset from the display > base so the new base does not need adjusting when flipping. The older > chipsets use a fence to access the display and so do perceive the > surface as linear and have a single base register which is reprogrammed > using the flip. Mostly correct, we should get this in quickly to fix https://bugs.freedesktop.org/show_bug.cgi?id=28518 (which is my fault; I tested on 945 and assumed the fix applied to all future generations. Of course I should have realized the hw guys like to keep us on our toes so they keep changing register and command layouts on us). Comments below. > - if (intel_crtc->plane) > - flip_mask = MI_WAIT_FOR_PLANE_B_FLIP; > - else > - flip_mask = MI_WAIT_FOR_PLANE_A_FLIP; > - > if (IS_GEN3(dev) || IS_GEN2(dev)) { > + u32 flip_mask; > + > + if (intel_crtc->plane) > + flip_mask = MI_WAIT_FOR_PLANE_B_FLIP; > + else > + flip_mask = MI_WAIT_FOR_PLANE_A_FLIP; > + > BEGIN_LP_RING(2); > OUT_RING(MI_WAIT_FOR_EVENT | flip_mask); > OUT_RING(0); > @@ -5092,28 +5092,36 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, > } Nice cleanup. > > /* Offset into the new buffer for cases of shared fbs between CRTCs */ > - offset = obj_priv->gtt_offset; > - offset += (crtc->y * fb->pitch) + (crtc->x * (fb->bits_per_pixel) / 8); > + offset = crtc->y * fb->pitch + crtc->x * fb->bits_per_pixel/8; Yep. > > BEGIN_LP_RING(4); > if (IS_I965G(dev)) { > + int pipe = intel_crtc->pipe; > + u32 pf, pipesrc; > + > OUT_RING(MI_DISPLAY_FLIP | > MI_DISPLAY_FLIP_PLANE(intel_crtc->plane)); > - OUT_RING(fb->pitch); > - OUT_RING(offset | obj_priv->tiling_mode); > - pipesrc = I915_READ(pipesrc_reg); > - OUT_RING(pipesrc & 0x0fff0fff); > + OUT_RING(fb->pitch | obj_priv->tiling_mode); > + /* i965+ uses the linear or tiled offsets from the > + * Display Registers (which do not change across a page-flip) > + * so we need only reprogram the base address. > + */ > + OUT_RING(obj_priv->gtt_offset); > + > + pf = I915_READ(pipe == 0 ? PFA_CTL_1 : PFB_CTL_1) & PF_ENABLE; > + pipesrc = I915_READ(pipe == 0 ? PIPEASRC : PIPEBSRC) & > 0x0fff0fff; > + OUT_RING(pf | pipesrc); Should be OUT_RING(fb->pitch); OUT_RING(obj_priv->gtt_offset | obj_priv->tiling_mode); (as discussed on IRC the fields are documented incorrectly in some places and the tiling param is required). Also I'm worried about the panel fitting bits; I know ILK has more flexible panel fitting, but I'd like to see this tested on 965 and GM45 with a non-native mode to be sure these bits also work there. > } else if (IS_GEN3(dev)) { > OUT_RING(MI_DISPLAY_FLIP_I915 | > MI_DISPLAY_FLIP_PLANE(intel_crtc->plane)); > OUT_RING(fb->pitch); > - OUT_RING(offset); > + OUT_RING(obj_priv->gtt_offset + offset); > OUT_RING(MI_NOOP); > } else { > OUT_RING(MI_DISPLAY_FLIP | > MI_DISPLAY_FLIP_PLANE(intel_crtc->plane)); > OUT_RING(fb->pitch); > - OUT_RING(offset); > + OUT_RING(obj_priv->gtt_offset + offset); > OUT_RING(MI_NOOP); > } > ADVANCE_LP_RING(); These bits look good. Thanks, -- Jesse Barnes, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
