[PATCH] a lockup in the mga driver

[Dave Airlie]

On Fri, Mar 14, 2014 at 8:25 AM, Mikulas Patocka <mpatocka at redhat.com> wrote:
> Hi David
>
> Are you maintaining the Xorg mga driver? (you anounced the last update
> 1.6.3) Or is someone else maintaining it?

Nobody maintains it really, the usermode driver is mostly ignored at this point,

I try to look after the kernel mgag200 for the server cards, I can
commit patches to this
if they fix things, but otherwise its mostly dead, volunteers welcome.


Dave.

>
> I've had a Xserver lockup in the mga driver, examining it with gdb showed
> this obviously broken loop:
>         count = INREG(MGAREG_VCOUNT) + 2;
>         while(INREG(MGAREG_VCOUNT) < count);
>
> It reads the line counter and waits until the counter advances by two. The
> cause of the lockup is this - if the kernel reschedules the Xorg process
> and lets it run in such a moment when INREG(MGAREG_VCOUNT) returns the
> maximum (or maximum minus 1) line count, the loop never exits.
>
> I created this patch for the bug, it tries to imitate the old behavior,
> but it also exits the loop when the line counter wraps to zero, preventing
> the lockup.
>
> BTW. did you notice my previous patch for pan_ctl in the mga driver?
>
> Mikulas
>
> ---
>  src/mga_driver.c |   12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> Index: xf86-video-mga-1.6.3/src/mga_driver.c
> ===================================================================
> --- xf86-video-mga-1.6.3.orig/src/mga_driver.c  2013-12-05 02:15:53.000000000 +0100
> +++ xf86-video-mga-1.6.3/src/mga_driver.c       2014-03-13 22:56:10.852221315 +0100
> @@ -3618,7 +3618,7 @@ void
>  MGAAdjustFrame(ADJUST_FRAME_ARGS_DECL)
>  {
>      SCRN_INFO_PTR(arg);
> -    int Base, tmp, count;
> +    int Base, tmp, count, last_vcount;
>
>      MGAFBLayout *pLayout;
>      MGAPtr pMga;
> @@ -3648,8 +3648,14 @@ MGAAdjustFrame(ADJUST_FRAME_ARGS_DECL)
>          while (INREG8(0x1FDA) & 0x08);
>          while (!(INREG8(0x1FDA) & 0x08));
>          /* wait until we're past the start (fixseg.c in the DDK) */
> -        count = INREG(MGAREG_VCOUNT) + 2;
> -        while(INREG(MGAREG_VCOUNT) < count);
> +        last_vcount = INREG(MGAREG_VCOUNT);
> +        count = last_vcount + 2;
> +        while (1) {
> +           int vcount = INREG(MGAREG_VCOUNT);
> +           if (vcount >= count) break;
> +           if (vcount < last_vcount) break;
> +           last_vcount = count;
> +        }
>
>          OUTREG16(MGAREG_CRTC_INDEX, (Base & 0x00FF00) | 0x0C);
>          OUTREG16(MGAREG_CRTC_INDEX, ((Base & 0x0000FF) << 8) | 0x0D);
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel