On Sun, 2004-12-12 at 14:52 +0100, Stephane Marchesin wrote:
>
> Here is my second try at the surface ioctl.
> Does everything look correct ?
Looks like you've made good progress.
> Index: shared/radeon_drm.h
> ===================================================================
> RCS file: /cvs/dri/drm/shared/radeon_drm.h,v
> retrieving revision 1.25
> diff -u -r1.25 radeon_drm.h
> --- shared/radeon_drm.h 8 Dec 2004 16:43:00 -0000 1.25
> +++ shared/radeon_drm.h 12 Dec 2004 13:39:16 -0000
> @@ -628,6 +634,20 @@
> int64_t value;
> } drm_radeon_setparam_t;
>
> +/* 1.14: Clients can allocate/free a surface
> + */
> +
> +typedef struct drm_radeon_surface_alloc {
> + int lower;
> + int upper;
> + int flags;
> +} drm_radeon_surface_alloc_t;
> +
> +typedef struct drm_radeon_surface_free {
> + int lower;
> +} drm_radeon_surface_free_t;
The members of these structs should be explicitly sized (and aligned?)
to avoid any 32/64 bit problems.
> Index: shared/radeon_state.c
> ===================================================================
> RCS file: /cvs/dri/drm/shared/radeon_state.c,v
> retrieving revision 1.40
> diff -u -r1.40 radeon_state.c
> --- shared/radeon_state.c 8 Dec 2004 16:43:00 -0000 1.40
> +++ shared/radeon_state.c 12 Dec 2004 13:39:18 -0000
> @@ -1706,11 +1706,193 @@
> ADVANCE_RING();
> }
>
> +static void radeon_apply_surface_regs(int surf_index, drm_radeon_private_t
> *dev_priv)
> +{
> + RING_LOCALS;
> +
> + BEGIN_RING( 6 );
> + OUT_RING( CP_PACKET0( RADEON_SURFACE0_INFO+16*surf_index, 0 ) );
> + OUT_RING( dev_priv->surfaces[surf_index].flags );
> + OUT_RING( CP_PACKET0( RADEON_SURFACE0_LOWER_BOUND+16*surf_index, 0 ) );
> + OUT_RING( dev_priv->surfaces[surf_index].lower );
> + OUT_RING( CP_PACKET0( RADEON_SURFACE0_UPPER_BOUND+16*surf_index, 0 ) );
> + OUT_RING( dev_priv->surfaces[surf_index].upper );
> + ADVANCE_RING();
> +}
It doesn't make sense to use the CP for these register writes because
AFAIK the surfaces only apply to CPU access, so you want them to take
effect immediately, not whenever the CP gets around to processing them.
These registers aren't FIFO'd, so it should be safe to touch them at any
time.
> + /* make sure there is no overlap with existing surfaces */
> + for(i=0;i<RADEON_MAX_SURFACES;i++) {
> + if ((dev_priv->surfaces[i].refcount!=0) &&
> + (( (new->lower>=dev_priv->surfaces[i].lower)&&
> (new->lower<dev_priv->surfaces[i].upper) ) ||
> + ( (new->upper>dev_priv->surfaces[i].lower)&&
> (new->upper<=dev_priv->surfaces[i].upper) ) ||
> + ( (new->lower<dev_priv->surfaces[i].lower)&&
> (new->upper>dev_priv->surfaces[i].upper) )) )
> + return -1;
> + }
This also fails if the exact same surface (range) exists already, right?
Should it? I guess allowing that would complicate the refcounting...
Also, you might allow overlapping as long as the flags are compatible?
I'm not sure those cases are important in practice though.
BTW, you should use a standard error code like EINVAL instead of an
explicit value.
--
Earthling Michel DÃnzer | Debian (powerpc), X and DRI developer
Libre software enthusiast | http://svcs.affero.net/rm.php?r=daenzer
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://productguide.itmanagersjournal.com/
--
_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
