Re: New ioctl for surface allocation/deallocation
Dave Airlie wrote: Any comments ? I'ts untested, but that's my first ioctl, so I wanted feedback (both on the idea and the implementation) before I build anything upon it. I'm thinking this could be applicable to a number of devices, so maybe a generic ioctl might be a better idea with some card specific hooks.. Or is there something radeon specific about it I missed... This ties in directly to the idea of a generalized memory manager for the DRI drivers, something that I should be able to get some time to look into fairly shortly. It's not clear what is being allocated in the patch except for a unique number - what is the number used for after that? There's already the equivalent level of trust exported to the clients in the existing interfaces to allow them to manage a shared bitfield themselves in the SAREA shared area using the drm lock for serialization. The aspect missing from both your implementation and the SAREA-based bitflag is that there is no way to free bits in your bitflag which are held by contexts in processes which die without clearing their bits, eg. by segfault. Keith --- 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
Re: New ioctl for surface allocation/deallocation
Michel Dnzer wrote:
+ // allocate the surface
+ for(i=0;i8;i++)
+ if (!(dev_priv-surfaces(1i)))
+ break;
+
+ if (i=8)
+ return DRM_ERR(ENOMEM);
+ else
+ dev_priv-surfaces=(1i);
+
+ if ( DRM_COPY_TO_USER( alloc.surface, i,
+ sizeof(int) ) ) {
+ DRM_ERROR( copy_to_user\n );
+ return DRM_ERR(EFAULT);
IMHO it should also manage the ranges (prevent overlapping, ...) and
parameters of the surfaces.
Ok, that was one of my doubts.
So if we go that route, it would manage the ranges, prevent overlapping,
and also try to spare the surfaces by merging adjacent ones with
similar properties.
+ DRM_COPY_FROM_USER_IOCTL( memfree, (drm_radeon_mem_free_t __user *)data,
+ sizeof(memfree) );
+
+ dev_priv-surfaces= (~(1memfree.surface));
It should definitely ensure that only the owner can free a surface
though. It would also need to free a client's surfaces if it dies, etc.
How do you know the owner ? I'm not sure the pid would be enough.
Stephane
---
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
Re: New ioctl for surface allocation/deallocation
Dave Airlie wrote: Any comments ? I'ts untested, but that's my first ioctl, so I wanted feedback (both on the idea and the implementation) before I build anything upon it. I'm thinking this could be applicable to a number of devices, so maybe a generic ioctl might be a better idea with some card specific hooks.. Or is there something radeon specific about it I missed... If we decide to manage the surface ranges too, there is something radeon-specific. Also what other things could be managed by such an ioctl ? Stephane --- 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
Re: New ioctl for surface allocation/deallocation
Keith Whitwell wrote: Dave Airlie wrote: Any comments ? I'ts untested, but that's my first ioctl, so I wanted feedback (both on the idea and the implementation) before I build anything upon it. I'm thinking this could be applicable to a number of devices, so maybe a generic ioctl might be a better idea with some card specific hooks.. Or is there something radeon specific about it I missed... This ties in directly to the idea of a generalized memory manager for the DRI drivers, something that I should be able to get some time to look into fairly shortly. It's not clear what is being allocated in the patch except for a unique number - what is the number used for after that? It's supposed to be the surface number : it gives you a surface between RADEON_SURFACE0* and RADEON_SURFACE7* and then allows you playing with LOWER/UPPER. There's already the equivalent level of trust exported to the clients in the existing interfaces to allow them to manage a shared bitfield themselves in the SAREA shared area using the drm lock for serialization. The aspect missing from both your implementation and the SAREA-based bitflag is that there is no way to free bits in your bitflag which are held by contexts in processes which die without clearing their bits, eg. by segfault. Hmm, I thought about this but wasn't sure if that was a problem. How can this be solved ? Stephane --- 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
Re: New ioctl for surface allocation/deallocation
Stephane Marchesin wrote: Keith Whitwell wrote: Dave Airlie wrote: Any comments ? I'ts untested, but that's my first ioctl, so I wanted feedback (both on the idea and the implementation) before I build anything upon it. I'm thinking this could be applicable to a number of devices, so maybe a generic ioctl might be a better idea with some card specific hooks.. Or is there something radeon specific about it I missed... This ties in directly to the idea of a generalized memory manager for the DRI drivers, something that I should be able to get some time to look into fairly shortly. It's not clear what is being allocated in the patch except for a unique number - what is the number used for after that? It's supposed to be the surface number : it gives you a surface between RADEON_SURFACE0* and RADEON_SURFACE7* and then allows you playing with LOWER/UPPER. There's already the equivalent level of trust exported to the clients in the existing interfaces to allow them to manage a shared bitfield themselves in the SAREA shared area using the drm lock for serialization. The aspect missing from both your implementation and the SAREA-based bitflag is that there is no way to free bits in your bitflag which are held by contexts in processes which die without clearing their bits, eg. by segfault. Hmm, I thought about this but wasn't sure if that was a problem. How can this be solved ? Associate the allocated resources with the filp of the connection which was granted them (note: NOT the pid), and when that connection closes, deallocate the resource. Keith --- 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
Re: New ioctl for surface allocation/deallocation
--- Stephane Marchesin [EMAIL PROTECTED] wrote:
Michel Dänzer wrote:
+ // allocate the surface
+ for(i=0;i8;i++)
+ if (!(dev_priv-surfaces(1i)))
+ break;
+
+ if (i=8)
+ return DRM_ERR(ENOMEM);
+ else
+ dev_priv-surfaces=(1i);
+
+ if ( DRM_COPY_TO_USER( alloc.surface, i,
+ sizeof(int) ) ) {
+ DRM_ERROR( copy_to_user\n );
+ return DRM_ERR(EFAULT);
IMHO it should also manage the ranges (prevent overlapping, ...) and
parameters of the surfaces.
Ok, that was one of my doubts.
So if we go that route, it would manage the ranges, prevent overlapping,
and also try to spare the surfaces by merging adjacent ones with
similar properties.
+ DRM_COPY_FROM_USER_IOCTL( memfree, (drm_radeon_mem_free_t __user
*)data,
+ sizeof(memfree) );
+
+ dev_priv-surfaces= (~(1memfree.surface));
It should definitely ensure that only the owner can free a surface
though. It would also need to free a client's surfaces if it dies, etc.
How do you know the owner ? I'm not sure the pid would be enough.
I'm sure there is a better way, also keep inmind GLX-remote direct
rendering.
For now using the PID to get the UID and EUID would allow any process by
that user to dealloc the reasource(for threaded programs).
Stephane
---
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
__
Do you Yahoo!?
All your favorites on one personal page Try My Yahoo!
http://my.yahoo.com
---
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
Re: New ioctl for surface allocation/deallocation
Stephane Marchesin wrote: Michel Dnzer wrote: +DRM_COPY_FROM_USER_IOCTL( memfree, (drm_radeon_mem_free_t __user *)data, + sizeof(memfree) ); + +dev_priv-surfaces= (~(1memfree.surface)); It should definitely ensure that only the owner can free a surface though. It would also need to free a client's surfaces if it dies, etc. How do you know the owner ? I'm not sure the pid would be enough. The same way everything else in the DRM does: using the caller's file pointer (filp). --- 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
New ioctl for surface allocation/deallocation
Hi,
The small attached patch adds a new drm ioctl to do surface
allocation/deallocation on the radeon.
The reason to add a new ioctl is the following : since there are only 8
surfaces someone has to arbitrate between allocations by the ddx (color
buffers) and from the dri (depth buffers). Doing it this way is more
future proof than statically allocating them (for example, if we once do
private z buffers or pbuffers these will be created/destroyed on the fly).
Any comments ? I'ts untested, but that's my first ioctl, so I wanted
feedback (both on the idea and the implementation) before I build
anything upon it.
Stephane
PS : I jumped to drm version 1.14 directly because hyperz already has 1.13
? linux/build
? linux/drm_pciids.h
Index: shared/radeon.h
===
RCS file: /cvs/dri/drm/shared/radeon.h,v
retrieving revision 1.33
diff -u -r1.33 radeon.h
--- shared/radeon.h 23 Oct 2004 06:25:56 - 1.33
+++ shared/radeon.h 8 Dec 2004 01:43:53 -
@@ -45,7 +45,7 @@
#define DRIVER_DATE20020828
#define DRIVER_MAJOR 1
-#define DRIVER_MINOR 12
+#define DRIVER_MINOR 14
#define DRIVER_PATCHLEVEL 0
/* Interface history:
@@ -82,6 +82,7 @@
* and GL_EXT_blend_[func|equation]_separate on r200
* 1.12- Add R300 CP microcode support - this just loads the CP on r300
* (No 3D support yet - just microcode loading).
+ * 1.14- Add R100/R200 surface allocation/free support
*/
#define DRIVER_IOCTLS \
[DRM_IOCTL_NR(DRM_IOCTL_DMA)] = { radeon_cp_buffers, 1, 0 }, \
@@ -110,5 +111,7 @@
[DRM_IOCTL_NR(DRM_IOCTL_RADEON_IRQ_EMIT)] = { radeon_irq_emit,1, 0 }, \
[DRM_IOCTL_NR(DRM_IOCTL_RADEON_IRQ_WAIT)] = { radeon_irq_wait,1, 0 }, \
[DRM_IOCTL_NR(DRM_IOCTL_RADEON_SETPARAM)] = { radeon_cp_setparam, 1, 0 }, \
+ [DRM_IOCTL_NR(DRM_IOCTL_RADEON_SURF_ALLOC)] = { radeon_surface_alloc,1, 0 }, \
+ [DRM_IOCTL_NR(DRM_IOCTL_RADEON_SURF_FREE)] = { radeon_surface_free ,1, 0 }, \
#endif
Index: shared/radeon_cp.c
===
RCS file: /cvs/dri/drm/shared/radeon_cp.c,v
retrieving revision 1.45
diff -u -r1.45 radeon_cp.c
--- shared/radeon_cp.c 23 Oct 2004 06:25:56 - 1.45
+++ shared/radeon_cp.c 8 Dec 2004 01:43:54 -
@@ -1519,6 +1519,8 @@
radeon_cp_init_ring_buffer( dev, dev_priv );
dev_priv-last_buf = 0;
+
+ dev_priv-surfaces = 0;
radeon_do_engine_reset( dev );
Index: shared/radeon_drm.h
===
RCS file: /cvs/dri/drm/shared/radeon_drm.h,v
retrieving revision 1.24
diff -u -r1.24 radeon_drm.h
--- shared/radeon_drm.h 23 Oct 2004 06:25:56 - 1.24
+++ shared/radeon_drm.h 8 Dec 2004 01:43:54 -
@@ -399,6 +399,8 @@
#define DRM_RADEON_IRQ_WAIT 0x17
#define DRM_RADEON_CP_RESUME 0x18
#define DRM_RADEON_SETPARAM 0x19
+#define DRM_RADEON_SURF_ALLOC 0x20
+#define DRM_RADEON_SURF_FREE 0x21
#define DRM_IOCTL_RADEON_CP_INITDRM_IOW( DRM_COMMAND_BASE +
DRM_RADEON_CP_INIT, drm_radeon_init_t)
#define DRM_IOCTL_RADEON_CP_START DRM_IO( DRM_COMMAND_BASE +
DRM_RADEON_CP_START)
@@ -425,6 +427,8 @@
#define DRM_IOCTL_RADEON_IRQ_WAIT DRM_IOW( DRM_COMMAND_BASE +
DRM_RADEON_IRQ_WAIT, drm_radeon_irq_wait_t)
#define DRM_IOCTL_RADEON_CP_RESUME DRM_IO( DRM_COMMAND_BASE +
DRM_RADEON_CP_RESUME)
#define DRM_IOCTL_RADEON_SETPARAM DRM_IOW( DRM_COMMAND_BASE +
DRM_RADEON_SETPARAM, drm_radeon_setparam_t)
+#define DRM_IOCTL_RADEON_SURF_ALLOC DRM_IOWR(DRM_COMMAND_BASE +
DRM_RADEON_SURF_ALLOC, drm_radeon_surface_alloc_t)
+#define DRM_IOCTL_RADEON_SURF_FREE DRM_IOW( DRM_COMMAND_BASE +
DRM_RADEON_SURF_FREE, drm_radeon_surface_free_t)
typedef struct drm_radeon_init {
enum {
@@ -624,6 +628,18 @@
int64_t value;
} drm_radeon_setparam_t;
+/* 1.14: Clients can allocate/free a surface
+ */
+
+typedef struct drm_radeon_surface_alloc {
+ int surface;
+} drm_radeon_surface_alloc_t;
+
+typedef struct drm_radeon_surface_free {
+ int surface;
+} drm_radeon_surface_free_t;
+
+
#define RADEON_SETPARAM_FB_LOCATION1 /* determined framebuffer location */
Index: shared/radeon_drv.h
===
RCS file: /cvs/dri/drm/shared/radeon_drv.h,v
retrieving revision 1.37
diff -u -r1.37 radeon_drv.h
--- shared/radeon_drv.h 9 Nov 2004 00:54:19 - 1.37
+++ shared/radeon_drv.h 8 Dec 2004 01:43:55 -
@@ -185,6 +185,8 @@
struct mem_block *gart_heap;
struct mem_block *fb_heap;
+ u32 surfaces;
+
/* SW interrupt */
wait_queue_head_t swi_queue;
atomic_t swi_emitted;
@@ -239,6 +241,8 @@
extern int radeon_mem_init_heap( DRM_IOCTL_ARGS );
extern void radeon_mem_takedown( struct mem_block **heap );
extern void
Re: New ioctl for surface allocation/deallocation
On Wed, 2004-12-08 at 02:54 +0100, Stephane Marchesin wrote:
The small attached patch adds a new drm ioctl to do surface
allocation/deallocation on the radeon.
[...]
Any comments ? I'ts untested, but that's my first ioctl, so I wanted
feedback (both on the idea and the implementation) before I build
anything upon it.
I think it's a good start but still missing some details.
+ // allocate the surface
+ for(i=0;i8;i++)
+ if (!(dev_priv-surfaces(1i)))
+ break;
+
+ if (i=8)
+ return DRM_ERR(ENOMEM);
+ else
+ dev_priv-surfaces=(1i);
+
+ if ( DRM_COPY_TO_USER( alloc.surface, i,
+sizeof(int) ) ) {
+ DRM_ERROR( copy_to_user\n );
+ return DRM_ERR(EFAULT);
IMHO it should also manage the ranges (prevent overlapping, ...) and
parameters of the surfaces.
+ DRM_COPY_FROM_USER_IOCTL( memfree, (drm_radeon_mem_free_t __user *)data,
+ sizeof(memfree) );
+
+ dev_priv-surfaces= (~(1memfree.surface));
It should definitely ensure that only the owner can free a surface
though. It would also need to free a client's surfaces if it dies, etc.
--
Earthling Michel Dnzer | 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
Re: New ioctl for surface allocation/deallocation
Any comments ? I'ts untested, but that's my first ioctl, so I wanted feedback (both on the idea and the implementation) before I build anything upon it. I'm thinking this could be applicable to a number of devices, so maybe a generic ioctl might be a better idea with some card specific hooks.. Or is there something radeon specific about it I missed... Dave. -- David Airlie, Software Engineer http://www.skynet.ie/~airlied / airlied at skynet.ie pam_smb / Linux DECstation / Linux VAX / ILUG person --- 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
