[Nouveau] Removal of Non-KMS support

Maarten Maathuis madman2003 at gmail.com
Wed Jan 6 14:20:15 PST 2010


It should be Acked-by (notice the lack of capital b):

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

And curro_ == Francisco Jerez, i was thinking in irc mode.

On Wed, Jan 6, 2010 at 11:16 PM, Maarten Maathuis <madman2003 at gmail.com> wrote:
> 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