Alex Deucher wrote:
On Wed, 05 Jan 2005 15:20:41 +0000, Keith Whitwell
<[EMAIL PROTECTED]> wrote:

Alex Deucher wrote:

On Wed, 05 Jan 2005 14:35:30 +0000, Keith Whitwell
<[EMAIL PROTECTED]> wrote:


Alex Deucher wrote:


Ok, so last night I think I figured out why pageflipping was borked
with mergedfb.  Unfortunately the fix requires changes to the drm and
sarea.  I'm wondering what the best way to do this is and maintain
compatability.  When crtc1 is at offset 0 pageflipping should work
with a simple patch to radeon_driver.c::RadeonDoAdjustframe().  the
backbuffer offset was being added to the base address after the
crtc2_base info was updated in the sarea.  That's an easy fix.   For
situations where crtc2 is at offset 0 and crtc1 is at a higher offset,
changes to the drm and sarea are required.  I think the easiest
solution would be to add a crtc1_base to the sarea and update that in
RadeonDoAdjustframe() just like crtc2_base.  The problem is that in
the drm in radeon_state.c::radeon_cp_dispatch_flip() the offset of
crtc1 is calculated from the sarea frame info which could be crtc1 or
crtc2 depending on their orientation.  It should really be calcluated
in RadeonDoAdjustframe() just like crtc2 base.  So how to I do it in a
compatible way?

Thanks,


Alex,

It might be easer to understand the issues if you post a non-compatible
version of the fix, so that there's something concrete to talk about.

Keith



I'm not at my pc at the moment, but I threw this together quickly to
give you all an idea of what I'm talking about:
http://www.botchco.com/alex/mfb-fix.patch

--- radeon_driver.c.orig      2005-01-05 10:03:39.660423035 -0500
+++ radeon_driver.c   2005-01-05 10:07:11.867000308 -0500
@@ -7536,13 +7536,16 @@

     pSAREAPriv = DRIGetSAREAPrivate(pScrn->pScreen);

-     if (clone || info->IsSecondary) {
-         pSAREAPriv->crtc2_base = Base;
-     }
-
     if (pSAREAPriv->pfCurrentPage == 1) {
         Base += info->backOffset;
     }
+
+        if (clone || info->IsSecondary) {
+            pSAREAPriv->crtc2_base = Base;
+        } else {
+            pSAREAPriv->crtc1_base = Base;
+        }
+
    }
#endif

--- radeon_drm.h.orig 2005-01-05 10:03:51.709730361 -0500
+++ radeon_drm.h      2005-01-05 10:09:08.057034243 -0500
@@ -359,6 +359,7 @@
     int pfState;            /* number of 3d windows (0,1,2ormore) */
     int pfCurrentPage;      /* which buffer is being displayed? */
     int crtc2_base;         /* CRTC2 frame offset */
+        int crtc1_base;
} drm_radeon_sarea_t;

/* WARNING: If you change any of these defines, make sure to change the
--- radeon_sarea.h.orig       2005-01-05 10:04:05.904558219 -0500
+++ radeon_sarea.h    2005-01-05 10:08:01.756851008 -0500
@@ -226,6 +226,7 @@
    int pfAllowPageFlip;     /* set by the 2d driver, read by the client */
    int pfCurrentPage;               /* set by kernel, read by others */
    int crtc2_base;          /* for pageflipping with CloneMode */
+    int crtc1_base;
} RADEONSAREAPriv, *RADEONSAREAPrivPtr;

#endif
--- radeon_state.c.orig       2005-01-05 10:04:17.660930998 -0500
+++ radeon_state.c    2005-01-05 10:10:31.661350159 -0500
@@ -1330,9 +1330,7 @@
     BEGIN_RING(6);

     RADEON_WAIT_UNTIL_3D_IDLE();
-     OUT_RING_REG(RADEON_CRTC_OFFSET,
-                  ((sarea->frame.y * dev_priv->front_pitch +
-                    sarea->frame.x * (dev_priv->color_fmt - 2)) & ~7)
+     OUT_RING_REG(RADEON_CRTC_OFFSET, dev_priv->sarea_priv->crtc1_base
                  + offset);
     OUT_RING_REG(RADEON_CRTC2_OFFSET, dev_priv->sarea_priv->crtc2_base
                  + offset);



I don't think there's actually any problem with this patch - old
clientside drivers won't use the new sarea field, but they didn't handle
this right anyway it isn't as if you are breaking an already-working setup.



what if you use an old DDX with a new drm? The old DDX wouldn't set
sarea_priv->crtc1_base right; it wouldn't even know about it. In
radeon_cp_dispatch_flip() something bogus would get written to
RADEON_CRTC_OFFSET.


Ah right, I see it now.

It's a little tricky - I guess you'll need some way of informing the kernel which way it is supposed to calculate crtc1_base, either by a new call to the SET_PARAMETER ioctl, or via the ABI version setting interface. I don't have a good understanding of the latter, the former sounds like an easy option, just ensure that there is a root-only check on that parameter.

Keith


------------------------------------------------------- The SF.Net email is sponsored by: Beat the post-holiday blues Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek. It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt -- _______________________________________________ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to