On Tue, 2009-08-11 at 10:48 +0200, Thomas Hellström wrote:
> Michel Dänzer wrote:
> > On Wed, 2009-08-05 at 12:06 +0200, Thomas Hellström wrote: 
> >   
> >> Michel Dänzer wrote:
> >>     
> >>> On Wed, 2009-08-05 at 11:18 +0200, Thomas Hellström wrote:
> >>>   
> >>>       
> >>>> Aargh. Wait. I remember now.
> >>>>
> >>>> The fbcon bo is exported through the fbdev address space at offset 0.
> >>>> The vm_node is for the drm device address space only. So it is perfectly 
> >>>> legal and actually correct for it not to have a vm_node, unless it's 
> >>>> going to be accessible from the drm device. Does it need to for KMS?
> >>>>     
> >>>>         
> >>> I don't think so.
> >>>
> >>>   
> >>>       
> >>>> I'm a bit unsure whether it's OK to export a bo through two different 
> >>>> address spaces. In particular, unmap_mapping_range() on the drm device 
> >>>> will not kill the fbdev user-space mappings.
> >>>>     
> >>>>         
> >>> Hmm, so that would mean that if an fbdev mapping is created while the BO
> >>> is in VRAM, it would still access VRAM after the BO has been evicted? Is
> >>> there a solution for this?
> >>>
> >>>   
> >>>       
> >> Yes, You need to call unmap_mapping_range() on the fbdev address space. 
> >> See how that is done in ttm_bo_unmap_virtual() for the drm address 
> >> space. Actually, I think you need to set up a bo_driver hook in 
> >> ttm_bo_unmap_virtual() to do this every time the bo is moved or swapped 
> >> out.
> >>     
> >
> > The attached patches should hopefully address all the feedback I've
> > received here and on IRC. Let me know what you think.
> >
> >
> >   
> Michel,
> The TTM parts look good to me.

The only thing I'm not quite sure about is the ttm_bo_vm_fault changes;
I think they make the assumption that the BO backs the beginning of the
address space if bo->vm_node is NULL? If that is a problem, how could it
be solved?

> At some point we should fix up the ttm_bo_fbdev_io performance by 
> avoiding vmap() / ioremap() and instead use atomic maps but that's  for 
> the future.

Yeah, I don't think this is important, most certainly not for now.


> I'm not 100% sure, but the Radeon part still looks a little problematic.
> First, I think it's illegal to call radeonfb_remap() from the fbdev 
> accelerated hooks, since these may be called from within atomic context 
> and there's no way to detect this since in_atomic() only works if 
> CONFIG_PREEMPT is set. IIRC Jakob hit this with psbfb.
> 
> Also when you test for mem_reg_is_pci() to determine that it's ok to 
> kmap, I'd instead test for TTM_BO_FLAG_NO_EVICT to make sure the object 
> is pinned, regardless of its whereabouts.

Note that the primary purpose of the test isn't to check that it's okay
to kmap but that the resulting screen_base will be suitable for cfb_*
(i.e. essentially that it uses ioremap). Otherwise it just leaves
screen_base invalid.

> So the problem is really: How do we avoid the 
> invalidate_kernel_address() racing with an fbcon access?
> To be completely sure we would want a spinlock protecting the address 
> pointer, but we don't have one, and the error is fatal if this happens.

Hmm. Maybe the console semaphore could be used for this? If we can't
acquire it, maybe we could just fail whatever we would need it for...


-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to