Re: ttm_mem_global

[Jerome Glisse <glisse@freedesktop.org>]

On Wed, 2009-07-29 at 11:39 +0200, Thomas Hellström wrote:
> Jerome Glisse wrote:
> > On Tue, 2009-07-28 at 20:55 +0200, Thomas Hellström wrote:
> >   
> >> Jerome Glisse skrev:
> >>     
> >>> On Wed, 2009-07-22 at 10:37 +0200, Thomas Hellström wrote:
> >>>   
> >>>       
> >>>> TTM has a device struct per device and an optional global struct that is 
> >>>> common for all devices and intended to be per subsystem.
> >>>>
> >>>> The only subsystem currently having a global structure is the memory 
> >>>> accounting subsystem:
> >>>> struct ttm_mem_global
> >>>>     
> >>>>         
> >>> Thomas i don't think the way we init ttm_mem_global today make
> >>> it follow the 1 struct ttm_mem_global for everyone. I think it
> >>> should be initialized and refcounted by device struct.
> >>>
> >>> So on first device creation a ttm_mem_global is created and
> >>> then anytime a new device is created the refcount of ttm_mem_global
> >>> is increased. 
> >>>       
> >> Jerome,
> >> This is exactly what the current code intends to do.
> >>
> >> Are you seeing something different?
> >>     
> >
> > I definitly don't see that :) In radeon we do create a structure
> > which hold the ttm_mem_global struct so it's not shared at all
> > it got inited & destroyed along the driver. This is why i think
> > it's better to remove the driver initialization and let bo_device
> > init path take care of initializing one and only one object which
> > can be shared by multiple driverttm_mem_global_inits.
> >
> >   
> Which radeon struct is holding the ttm_mem_global struct?
> 
> The radeon code looks very similar to the openchrome code in which the 
> struct ttm_mem_global is allocated at ttm_global.c, line 74 and freed at 
> ttm_global.c, line 108 when its refcount has reached zero.
> 
> So the device holds a struct ttm_global_reference that *only points* to 
> the global item, and which is destroyed on device takedown. If there are 
> more than one device pointing to the mem_global object, it won't get 
> destroyed.
> 
> So the code should be working perfectly fine unless there is a bug.
> 
> > So what i propose is remove mem_glob parameter from :
> > ttm_bo_device_init, add a call to ttm_mem_global_init in
> > ttm_bo_device_init 
> 
> Nope, The ttm_mem_global object is used by other  ttm subsystems 
> (fencing, user-space objects),
> so that can't be done.
> 
> > and add some static refcount in ttm_memory.c
> > if refcount = 0 then ttm_mem_global_init create a ttm_mem_global
> > struct and initialize things, if refcount > 0 then it gives
> > back the already initialized ttm_mem_global.
> >
> >   
> 
> This is exactly what ttm_global was created to do, and what it hopefully 
> does. If you create two radeon devices the ttm_mem_global object should 
> be the same, even though the global references pointing to it are of 
> course different. Have you actually tried this?
> 
> /Thomas
> 

Ok code wasn't clear for me until i read ttm_global.c

Cheers,
Jerome