xf86-video-savage: (2/2) Do not use XAAGetCopyROP outside of XAA-specific code

[Alex Deucher]

On 4/13/09, Alex Villací­s Lasso <a_villacis at palosanto.com> wrote:
> The DRI code attempts to use XAAGetCopyROP without checking whether XAA or
> EXA is in effect. This results in the server crashing with an
> undefined-symbol error when enabling EXA, then starting glxgears under
> GNOME/Metacity and attempting to drag the glxgears window.
>
>  The EXA code happens to have a functional duplicate of the required code
> under the name SavageGetCopyROP(). This patch moves the code to
> savage_accel.c where it is now shared between EXA and the DRI code.
>
>  It is disturbing that this has not been caught before...
>
>  Just out of curiosity, how many subscribers of this list have access to a
> Savage chipset, besides myself? Is the Savage EXA code being exercised in
> the wild?
>
>  I followed the steps at
> http://www.freedesktop.org/wiki/AccountRequests in order to
> get commit access for xf86-video-savage, and opened
> https://bugs.freedesktop.org/show_bug.cgi?id=20727 for the
> required information. No reply has yet been received. Is there anything else
> missing?

I didn't realize you had requested an account.  I've signed off on it.
 Sorry about the delay.

Alex


>
>  --
>  perl -e '$x=2.3;printf("%.0f + %.0f = %.0f\n",$x,$x,$x+$x);'
>
>
> From 4028dd0e0a351bdca9af2032717e97f5c7added6 Mon Sep 17
> 00:00:00 2001
>  From: =?utf-8?q?Alex=20Villac=C3=ADs=20Lasso?=
> <a_villacis at palosanto.com>
>  Date: Sun, 12 Apr 2009 16:07:06 -0500
>  Subject: [PATCH] Do not use XAAGetCopyROP outside of XAA-specific code
>
>  The DRI code attempts to use XAAGetCopyROP without checking whether XAA or
> EXA is in effect. This results in the server crashing with an
> undefined-symbol error when enabling EXA, then starting glxgears under
> GNOME/Metacity and attempting to drag the glxgears window.
>
>  The EXA code happens to have a functional duplicate of the required code
> under the name SavageGetCopyROP(). This patch moves the code to
> savage_accel.c where it is now shared between EXA and the DRI code.
>
>  It is disturbing that this has not been caught before.
>  ---
>   src/savage_accel.c |   26 ++++++++++++++++++++++++++
>   src/savage_dri.c   |    5 ++++-
>   src/savage_exa.c   |   27 ++-------------------------
>   3 files changed, 32 insertions(+), 26 deletions(-)
>
>  diff --git a/src/savage_accel.c b/src/savage_accel.c
>  index 450b402..73c2d90 100644
>  --- a/src/savage_accel.c
>  +++ b/src/savage_accel.c
>  @@ -1537,6 +1537,32 @@ SavageInitAccel(ScreenPtr pScreen)
>         return SavageXAAInit(pScreen);
>   }
>
>  +int SavageGetCopyROP(int rop) {
>  +
>  +    int ALUCopyROP[16] =
>  +    {
>  +       0x00, /*ROP_0 GXclear */
>  +       0x88, /*ROP_DSa GXand */
>  +       0x44, /*ROP_SDna GXandReverse */
>  +       0xCC, /*ROP_S GXcopy */
>  +       0x22, /*ROP_DSna GXandInverted */
>  +       0xAA, /*ROP_D GXnoop */
>  +       0x66, /*ROP_DSx GXxor */
>  +       0xEE, /*ROP_DSo GXor */
>  +       0x11, /*ROP_DSon GXnor */
>  +       0x99, /*ROP_DSxn GXequiv */
>  +       0x55, /*ROP_Dn GXinvert*/
>  +       0xDD, /*ROP_SDno GXorReverse */
>  +       0x33, /*ROP_Sn GXcopyInverted */
>  +       0xBB, /*ROP_DSno GXorInverted */
>  +       0x77, /*ROP_DSan GXnand */
>  +       0xFF, /*ROP_1 GXset */
>  +    };
>  +
>  +    return (ALUCopyROP[rop]);
>  +
>  +}
>  +
>   /* Routines for debugging. */
>
>
>  diff --git a/src/savage_dri.c b/src/savage_dri.c
>  index 216c915..190e8ee 100644
>  --- a/src/savage_dri.c
>  +++ b/src/savage_dri.c
>  @@ -1578,6 +1578,9 @@ SAVAGEDRIMoveBuffers(WindowPtr pParent, DDXPointRec
> ptOldOrg,
>         psav->AccelInfoRec->NeedToSync = TRUE;
>   }
>
>  +/* Definition in savage_accel.c */
>  +int SavageGetCopyROP(int rop);
>  +
>   static void
>   SAVAGEDRISetupForScreenToScreenCopy(
>      ScrnInfoPtr pScrn,
>  @@ -1591,7 +1594,7 @@ SAVAGEDRISetupForScreenToScreenCopy(
>      int cmd =0;
>
>      cmd = BCI_CMD_RECT | BCI_CMD_DEST_PBD | BCI_CMD_SRC_PBD_COLOR;
>  -    BCI_CMD_SET_ROP( cmd, XAAGetCopyROP(rop) );
>  +    BCI_CMD_SET_ROP( cmd, SavageGetCopyROP(rop) );
>      if (transparency_color != -1)
>          cmd |= BCI_CMD_SEND_COLOR | BCI_CMD_SRC_TRANSPARENT;
>
>  diff --git a/src/savage_exa.c b/src/savage_exa.c
>  index 08524f0..3fc8f30 100644
>  --- a/src/savage_exa.c
>  +++ b/src/savage_exa.c
>  @@ -69,31 +69,8 @@ SavageDownloadFromScreen(PixmapPtr
> pSrc, int x, int y, int w, int h, char *dst,
>   #define        GXset   0xFF
>   #endif
>
>  -static int SavageGetCopyROP(int rop) {
>  -
>  -    int ALUCopyROP[16] =
>  -    {
>  -       0x00, /*ROP_0 GXclear */
>  -       0x88, /*ROP_DSa GXand */
>  -       0x44, /*ROP_SDna GXandReverse */
>  -       0xCC, /*ROP_S GXcopy */
>  -       0x22, /*ROP_DSna GXandInverted */
>  -       0xAA, /*ROP_D GXnoop */
>  -       0x66, /*ROP_DSx GXxor */
>  -       0xEE, /*ROP_DSo GXor */
>  -       0x11, /*ROP_DSon GXnor */
>  -       0x99, /*ROP_DSxn GXequiv */
>  -       0x55, /*ROP_Dn GXinvert*/
>  -       0xDD, /*ROP_SDno GXorReverse */
>  -       0x33, /*ROP_Sn GXcopyInverted */
>  -       0xBB, /*ROP_DSno GXorInverted */
>  -       0x77, /*ROP_DSan GXnand */
>  -       0xFF, /*ROP_1 GXset */
>  -    };
>  -
>  -    return (ALUCopyROP[rop]);
>  -
>  -}
>  +/* Definition moved to savage_accel.c */
>  +int SavageGetCopyROP(int rop);
>
>   static int SavageGetSolidROP(int rop) {
>
>  --
>  1.6.0.6
>
>
>
>