Bug#613593: xserver-xorg-video-radeon: [regression] X fails to start

[Michel Dänzer]

On Die, 2011-03-01 at 22:22 +0100, Stephen Kitt wrote: 
> On Tue, 01 Mar 2011 11:50:23 +0100, Michel Dänzer <daenzer at debian.org> wrote:
> > On Die, 2011-03-01 at 11:33 +0100, Stephen Kitt wrote: 
> > > Would you like a patch to ignore (with a warning) these options if they're
> > > set?
> > 
> > I'm afraid it's not that simple, see
> > https://bugs.freedesktop.org/show_bug.cgi?id=30647#c3 . That said, if
> > you can come up with a solution, it'll certainly be considered though.
> 
> How about the attached patches (against current unstable)? On the server side,
> they add an EXA_REQUIRES_UPLOAD_DOWNLOAD flag, which ignores options
> deactivating UTS and DFS with a warning in the logs; on the driver side, they
> state that r600 requires UTS and DFS. I imagine radeon_exa_funcs.c and
> evergreen_exa.c would need a similar patch, [...]

Indeed, they would.

I can't help feeling this is overkill. Why did you set these options in
the first place?

A simpler solution might be to put a warning in the log file if any of
these options are used, reminding of the manpage bit about them only
being intended for debugging purposes, and adding that they might cause
any kind of problems.


Anyway, some technical comments on the patches below. If you still feel
they are necessary, please post them individually (e.g. as a series with
git send-email) to the xorg-devel at lists.freedesktop.org list for review.


> --- xorg-server-1.9.4.orig/hw/xfree86/exa/examodule.c
> +++ xorg-server-1.9.4/hw/xfree86/exa/examodule.c
> @@ -163,15 +163,23 @@
>      }
>  
>      if (xf86ReturnOptValBool(pScreenPriv->options, EXAOPT_NO_UTS, FALSE)) {
> -       xf86DrvMsg(pScreen->myNum, X_CONFIG,
> -                  "EXA: Disabling UploadToScreen\n");
> -       pExaScr->info->UploadToScreen = NULL;
> +       if (pExaScr->info->flags & EXA_REQUIRES_UPLOAD_DOWNLOAD) {
> +           xf86DrvMsg(pScreen->myNum, X_CONFIG, "EXA: Driver requires UploadToScreen, ignoring request to disable it\n");

These should probably use something other than X_CONFIG, e.g. X_WARNING.


> --- xserver-xorg-video-ati-6.14.0.orig/src/r600_exa.c
> +++ xserver-xorg-video-ati-6.14.0/src/r600_exa.c
> @@ -2221,6 +2221,7 @@
>      }
>  
>      info->accel_state->exa->flags = EXA_OFFSCREEN_PIXMAPS;
> +    info->accel_state->exa->flags |= EXA_REQUIRES_UPLOAD_DOWNLOAD;
>  #ifdef EXA_SUPPORTS_PREPARE_AUX
>      info->accel_state->exa->flags |= EXA_SUPPORTS_PREPARE_AUX;
>  #endif

This needs to be guarded by #ifdef EXA_REQUIRES_UPLOAD_DOWNLOAD, or it
will break the build against X servers not defining that flag.


-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer