[Intel-gfx] [PATCH] adjust FIFO watermarks instead of DSPARB
Ma, Ling
ling.ma at intel.com
Wed Apr 1 12:42:52 CEST 2009
hi Jesse
Today I tested the patch, it work fine on my G35 platform connected by VGA.
I used the largest mode line 1680x1050.
The attachment file includes Xorg log file
Thanks
Ma Ling
-----Original Message-----
From: intel-gfx-bounces at lists.freedesktop.org [mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf Of Jesse Barnes
Sent: Tuesday, March 31, 2009 11:16 PM
To: intel-gfx at lists.freedesktop.org
Subject: [Intel-gfx] [PATCH] adjust FIFO watermarks instead of DSPARB
This patch should make our driver handle multi-pipe configuration more
correctly than our current code. Currently, we adjust DSPARB when
changing modes, allocating FIFO space for the pipes based on the
programmed mode. We did this because we didn't have enough info to
properly program the FIFO watermark regs.
Now that we have that info, this patch uses the provided calculation to
adjust the watermark settings based on programmed mode. It should be
more reliable than the old method, and approximately matches what
other drivers do, so is also better tested.
Any comments? The only unknown here is post-965 chips. On those, FIFO
sizes are adjusted at runtime automatically by hardware, so we don't
have a reliable way of doing the calculation (I think we can just leave
it alone though; we haven't had any reports of pipe underrun issues on
post-965). And of course we need a KMS equivalent... Will do after I
get feedback on this patch.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
diff --git a/src/i810_reg.h b/src/i810_reg.h
index bc462fa..f961fcc 100644
--- a/src/i810_reg.h
+++ b/src/i810_reg.h
@@ -550,6 +550,7 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
#define MM_FIFO_WATERMARK 0x0001F000
#define LM_BURST_LENGTH 0x00000700
#define LM_FIFO_WATERMARK 0x0000001F
+#define FWATER_BLC_SELF 0x20e0
/* Fence/Tiling ranges [0..7]
diff --git a/src/i830_display.c b/src/i830_display.c
index dd1310f..e9cabf9 100644
--- a/src/i830_display.c
+++ b/src/i830_display.c
@@ -1484,66 +1484,165 @@ i830_panel_fitter_pipe(I830Ptr pI830)
}
/**
- * Sets up the DSPARB register to split the display fifo appropriately between
- * the display planes.
+ * i830_update_watermarks - update FIFO watermark values based on current modes
*
- * Adjusting this register requires that the planes be off.
+ * Calculate watermark values for the various WM regs based on current mode
+ * and plane configuration.
+ *
+ * There are several cases to deal with here:
+ * - normal (i.e. non-self-refresh)
+ * - self-refresh (SR) mode
+ * - lines are large relative to FIFO size (buffer can hold up to 2)
+ * - lines are small relative to FIFO size (buffer can hold more than 2
+ * lines), so need to account for TLB latency
+ *
+ * The normal calculation is:
+ * watermark = dotclock * bytes per pixel * latency
+ * where latency is platform & configuration dependent (we assume pessimal
+ * values here).
+ *
+ * The SR calculation is:
+ * watermark = (trunc(latency/line time)+1) * surface width *
+ * bytes per pixel
+ * where
+ * line time = htotal / dotclock
+ * and latency is assumed to be high, as above.
+ *
+ * The final value programmed to the register should always be rounded up,
+ * and include an extra 2 entries to account for clock crossings.
+ *
+ * We don't use the sprite, so we can ignore that. And on Crestline we have
+ * to set the non-SR watermarks to 8.
*/
static void
-i830_update_dsparb(ScrnInfoPtr pScrn)
+i830_update_watermarks(ScrnInfoPtr pScrn)
{
xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(pScrn);
I830Ptr pI830 = I830PTR(pScrn);
- int total_hdisplay = 0, planea_hdisplay = 0, planeb_hdisplay = 0;
- int fifo_entries = 0, planea_entries = 0, planeb_entries = 0, i;
-
- if ((INREG(DSPACNTR) & DISPLAY_PLANE_ENABLE) &&
- (INREG(DSPBCNTR) & DISPLAY_PLANE_ENABLE))
- xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
- "tried to update DSPARB with both planes enabled!\n");
-
- /*
- * FIFO entries will be split based on programmed modes
- */
- if (IS_I965GM(pI830))
+ int total_hdisplay = 0, planea_hdisplay = 0, planeb_hdisplay = 0, sr_hdisplay = 0;
+ int planea_entries = 0, planeb_entries = 0, sr_entries = 0, i;
+ float planea_dotclock = 0, planeb_dotclock = 0, sr_dotclock = 0;
+ int fifo_entries = 0, line_size = 0, enabled = 0;
+ float latency = .000003; /* fairly pessimistic value */
+ uint32_t dsparb = INREG(DSPARB);
+
+ if (IS_I965GM(pI830) || IS_I945GM(pI830)) {
fifo_entries = 127;
- else if (IS_I9XX(pI830))
+ line_size = 64;
+ } else if (IS_I9XX(pI830)) {
fifo_entries = 95;
- else if (IS_MOBILE(pI830)) {
+ line_size = 64;
+ } else if (IS_MOBILE(pI830)) {
fifo_entries = 255;
+ line_size = 32;
} else {
- /* The 845/865 only have a AEND field. Though the field size would
+ /* The 845/865 only have a AEND field. Though the field size would
* allow 128 entries, the 865 rendered the cursor wrong then.
* The BIOS set it up for 96.
*/
- fifo_entries = 95;
+ line_size = 32;
+ fifo_entries = 95;
}
for (i = 0; i < xf86_config->num_crtc; i++) {
xf86CrtcPtr crtc = xf86_config->crtc[i];
I830CrtcPrivatePtr intel_crtc = crtc->driver_private;
if (crtc->enabled) {
+ enabled++;
total_hdisplay += crtc->mode.HDisplay;
- if (intel_crtc->plane == 0)
+ if (intel_crtc->plane == 0) {
planea_hdisplay = crtc->mode.HDisplay;
- else
+ planea_dotclock = crtc->mode.Clock * 1000;
+ } else {
planeb_hdisplay = crtc->mode.HDisplay;
+ planeb_dotclock = crtc->mode.Clock * 1000;
+ }
+ sr_hdisplay = crtc->mode.HDisplay;
+ sr_dotclock = crtc->mode.Clock * 1000;
}
}
- planea_entries = fifo_entries * planea_hdisplay / total_hdisplay;
- planeb_entries = fifo_entries * planeb_hdisplay / total_hdisplay;
-
- if (IS_I9XX(pI830))
- OUTREG(DSPARB,
- ((planea_entries + planeb_entries) << DSPARB_CSTART_SHIFT) |
- (planea_entries << DSPARB_BSTART_SHIFT));
- else if (IS_MOBILE(pI830))
- OUTREG(DSPARB,
- ((planea_entries + planeb_entries) << DSPARB_BEND_SHIFT) |
- (planea_entries << DSPARB_AEND_SHIFT));
- else
- OUTREG(DSPARB, planea_entries << DSPARB_AEND_SHIFT);
+ planea_entries = rint(planea_dotclock * pI830->cpp * latency) / line_size;
+ planeb_entries = rint(planeb_dotclock * pI830->cpp * latency) / line_size;
+
+ xf86DrvMsg(pScrn->scrnIndex, X_INFO, "FIFO entries - A: %d, B: %d\n",
+ planea_entries, planeb_entries);
+
+ if (enabled == 1) {
+ float line_time = sr_hdisplay / sr_dotclock;
+
+ sr_entries = ((latency / line_time) + 1) * pI830->cpp * sr_hdisplay;
+ sr_entries = rint((double)sr_entries / (double)line_size);
+ }
+
+ if (IS_I965G(pI830)) {
+ xf86DrvMsg(pScrn->scrnIndex, X_INFO,
+ "Setting FIFO watermarks - A: 8, B: 8, C: 8, SR 8\n");
+
+ /* 965 has limitations... */
+ OUTREG(DSPFW1, (8 << 16) | (8 << 8) | (8 << 0));
+ OUTREG(DSPFW2, (8 << 8) | (8 << 0));
+ } else if (IS_I9XX(pI830) || IS_MOBILE(pI830)) {
+ uint32_t fwater_lo = INREG(FWATER_BLC) & MM_FIFO_WATERMARK;
+ uint32_t fwater_hi = INREG(FWATER_BLC2) & LM_FIFO_WATERMARK;
+ unsigned int bsize, asize, cwm, bwm = 1, awm = 1, srwm = 1;
+
+ if (IS_I9XX(pI830)) {
+ asize = dsparb & 0x7f;
+ bsize = (dsparb >> DSPARB_CSTART_SHIFT) & 0x7f;
+ } else {
+ asize = dsparb & 0x1ff;
+ bsize = (dsparb >> DSPARB_BEND_SHIFT) & 0x1ff;
+ }
+ xf86DrvMsg(pScrn->scrnIndex, X_INFO, "FIFO size - A: %d, B: %d\n",
+ asize, bsize);
+
+ /* Two extra entries for padding */
+ awm = asize - (planea_entries + 2);
+ bwm = bsize - (planeb_entries + 2);
+
+ /* Sanity check against potentially bad FIFO allocations */
+ if (planea_entries <= 0) {
+ xf86DrvMsg(pScrn->scrnIndex, X_WARNING,
+ "plane A needs more FIFO entries\n");
+ awm = 1;
+ }
+ if (planeb_entries <= 0) {
+ xf86DrvMsg(pScrn->scrnIndex, X_WARNING,
+ "plane B needs more FIFO entries\n");
+ bwm = 1;
+ }
+
+ /*
+ * Overlay gets an aggressive default since video jitter is bad.
+ */
+ cwm = 2;
+ if (sr_entries < fifo_entries)
+ srwm = fifo_entries - sr_entries;
+
+ xf86DrvMsg(pScrn->scrnIndex, X_INFO,
+ "Setting FIFO watermarks - A: %d, B: %d, C: %d, SR %d\n",
+ awm, bwm, cwm, srwm);
+
+ fwater_lo = fwater_lo | ((bwm & 0x1f) << 16) | (awm & 0x1f);
+ fwater_hi = fwater_hi | (cwm & 0x1f);
+
+ OUTREG(FWATER_BLC, fwater_lo);
+ OUTREG(FWATER_BLC2, fwater_hi);
+ if (IS_I9XX(pI830))
+ OUTREG(FWATER_BLC_SELF, srwm);
+ } else {
+ uint32_t fwater_lo = INREG(FWATER_BLC) & MM_FIFO_WATERMARK;
+ unsigned int asize, awm;
+
+ asize = dsparb & 0x7f;
+
+ awm = asize - planea_entries;
+
+ fwater_lo = fwater_lo | awm;
+
+ OUTREG(FWATER_BLC, fwater_lo);
+ }
}
/**
@@ -1889,7 +1988,7 @@ i830_crtc_mode_set(xf86CrtcPtr crtc, DisplayModePtr mode,
usleep(150);
if (!DSPARB_HWCONTROL(pI830))
- i830_update_dsparb(pScrn);
+ i830_update_watermarks(pScrn);
OUTREG(htot_reg, (adjusted_mode->CrtcHDisplay - 1) |
((adjusted_mode->CrtcHTotal - 1) << 16));
_______________________________________________
Intel-gfx mailing list
Intel-gfx at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Xorg.0.log
Type: application/octet-stream
Size: 71977 bytes
Desc: Xorg.0.log
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20090401/25149ec3/attachment.obj>
More information about the Intel-gfx
mailing list