[Nouveau] Removal of Non-KMS support

Maarten Maathuis madman2003 at gmail.com
Wed Jan 6 14:16:49 PST 2010


I had a quick look, and i have nothing to add. I must say a signed off
is inappropriate here, since we are neither the original copyright
holder or any of the subsequent "gateways". Depending on the level of
checking either an acked-by or reviewed-by is the right thing to do.
Since i did not check the patch line by line, and only agree with the
intention of the patch:

Acked-By: Maarten Maathuis <madman2003 at gmail.com>

Assuming curro_ concerns are dealt with.

On Wed, Jan 6, 2010 at 9:59 PM, Francisco Jerez <currojerez at riseup.net> wrote:
> Ben Skeggs <skeggsb at gmail.com> writes:
>
>> I did a very quick pass at removing all the non-KMS support from the
>> DDX.  It's tested on G80 but nowhere else currently, I thought some
>> discussion would be a good idea rather than just ripping it out :)
>>
>> The non-KMS paths are messy, and lets face it, rotting badly.  IMO the
>> KMS code is stable enough now that we can continue without the UMS
>> crutch, and indeed, the KMS code supports a lot more chipsets
>> (particularly on GF8 and up) than the UMS code ever will.
>>
>> I've left the Xv overlay code intact, but ifdef'd out.  I want to bring
>> support back with KMS enabled (thinking of older chipsets where the
>> blitter sucks), so it made sense to leave the old code there for now.
>>
>> So, who has some Signed-off-by's :)
>
> I'm very happy that we're finally getting rid of this :), besides a few
> comments, you got my blessing:
> Signed-off-by: Francisco Jerez <currojerez at riseup.net>
>
>>
>> Ben.
>>
>>
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/nouveau
>
> Some snips from the patch in question:
>
>> [...]
>> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
>> index e37e7c1..3d2df8d 100644
>> --- a/src/drmmode_display.c
>> +++ b/src/drmmode_display.c
>> [...]
>> +#define SOURCE_MASK_INTERLEAVE 32
>> +#define TRANSPARENT_PIXEL   0
>> +
>> +/*
>> + * Convert a source/mask bitmap cursor to an ARGB cursor, clipping or
>> + * padding as necessary. source/mask are assumed to be alternated each
>> + * SOURCE_MASK_INTERLEAVE bits.
>> + */
>> +static void
>> +nv_cursor_convert_cursor(uint32_t *src, void *dst, int src_stride,
>> +                      int dst_stride, int bpp, uint32_t fg, uint32_t bg)
>> +{
>> +     int width = min(src_stride, dst_stride);
>> +     uint32_t b, m, pxval;
>> +     int i, j, k;
>> +
>> +     for (i = 0; i < width; i++) {
>> +             for (j = 0; j < width / SOURCE_MASK_INTERLEAVE; j++) {
>> +                     int src_off = i*src_stride/SOURCE_MASK_INTERLEAVE + j;
>> +                     int dst_off = i*dst_stride + j*SOURCE_MASK_INTERLEAVE;
>> +
>> +                     b = src[2*src_off];
>> +                     m = src[2*src_off + 1];
>> +
>> +                     for (k = 0; k < SOURCE_MASK_INTERLEAVE; k++) {
>> +                             pxval = TRANSPARENT_PIXEL;
>> +#if X_BYTE_ORDER == X_BIG_ENDIAN
>> +                             if (m & 0x80000000)
>> +                                     pxval = (b & 0x80000000) ? fg : bg;
>> +                             b <<= 1;
>> +                             m <<= 1;
>> +#else
>> +                             if (m & 1)
>> +                                     pxval = (b & 1) ? fg : bg;
>> +                             b >>= 1;
>> +                             m >>= 1;
>> +#endif
>> +                             if (bpp == 32)
>> +                                     ((uint32_t *)dst)[dst_off + k] = pxval;
>> +                             else
>> +                                     ((uint16_t *)dst)[dst_off + k] = pxval;
>> +                     }
>> +             }
>> +     }
>> +}
>> +
>
> I'm quite sure that, without UMS, this function doesn't make sense
> anymore, you could leave this fun for the X server (we can use the
> load_cursor_argb hook even on the cards we don't advertise ARGB
> support). As a bonus that would probably solve Craig's rotated cursor
> issue.
>
>> [...]
>> diff --git a/src/nv_setup.c b/src/nv_setup.c
>> deleted file mode 100644
>> index f0478ca..0000000
>> --- a/src/nv_setup.c
>> +++ /dev/null
>> [...]
>> -     pNv->alphaCursor = (pNv->NVArch >= 0x11);
>> -
>> -     pNv->twoHeads = (pNv->Architecture >= NV_ARCH_10) &&
>> -                     (implementation != CHIPSET_NV10) &&
>> -                     (implementation != CHIPSET_NV15) &&
>> -                     (implementation != CHIPSET_NFORCE) &&
>> -                     (implementation != CHIPSET_NV20);
>> -
>> -     pNv->gf4_disp_arch = pNv->twoHeads && implementation != CHIPSET_NV11;
>> -
>> -     /* nv30 and nv35 have two stage PLLs, but use only one register; they are dealt with separately */
>> -     pNv->two_reg_pll = (implementation == CHIPSET_NV31) ||
>> -                        (implementation == CHIPSET_NV36) ||
>> -                        (pNv->Architecture >= NV_ARCH_40);
>> -
>> -     pNv->WaitVSyncPossible = (pNv->Architecture >= NV_ARCH_10) &&
>> -                              (implementation != CHIPSET_NV10);
>> -
>
> The accel code still has some bogus checks for WaitVSyncPossible, and
> bad things will happen if it's left uninitialized. Anyway, it would be
> nice to kill all this crap from NVRec.
>
>> -     pNv->BlendingPossible = ((pNv->Chipset & 0xffff) > CHIPSET_NV04);
>> [...]
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau
>
>


More information about the Nouveau mailing list