[Nouveau] Removal of Non-KMS support

Ben Skeggs skeggsb at gmail.com
Wed Jan 6 14:20:21 PST 2010


On Wed, 2010-01-06 at 21:59 +0100, Francisco Jerez 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.
Right, there's probably a lot of other things laying around that aren't
strictly necessary anymore too.  I guess we can just remove them as we
find them.

> 
> > [...]
> > 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.
Oops, I did indeed miss a couple, thanks :)

> 
> > -	pNv->BlendingPossible = ((pNv->Chipset & 0xffff) > CHIPSET_NV04);
> > [...]




More information about the Nouveau mailing list