svn commit: r268624 - head/sys/dev/vt/hw/efifb

[Nathan Whitehorn]

On 07/15/14 08:43, Alan Cox wrote:
> On 07/15/2014 09:01, John Baldwin wrote:
>> On Monday, July 14, 2014 1:53:45 pm Konstantin Belousov wrote:
>>> On Mon, Jul 14, 2014 at 05:42:22PM +0000, Nathan Whitehorn wrote:
>>>> Author: nwhitehorn
>>>> Date: Mon Jul 14 17:42:22 2014
>>>> New Revision: 268624
>>>> URL: http://svnweb.freebsd.org/changeset/base/268624
>>>>
>>>> Log:
>>>>    On my Lenovo laptop, the firmware maps the EFI framebuffer with MTRRs
>> set
>>>>    to uncacheable. This leads to execrable console performance. Once PMAP
>> is
>>>>    up, remap the framebuffer as write-combining. This reduces boot time on
>> my
>>>>    laptop by 60% when booting with EFI.
>>>>    
>>>>    MFC after:	2 weeks
>>>>
>>>> Modified:
>>>>    head/sys/dev/vt/hw/efifb/efifb.c
>>>>
>>>> Modified: head/sys/dev/vt/hw/efifb/efifb.c
>>>>
>> ==============================================================================
>>>> --- head/sys/dev/vt/hw/efifb/efifb.c	Mon Jul 14 17:16:09 2014	
>> (r268623)
>>>> +++ head/sys/dev/vt/hw/efifb/efifb.c	Mon Jul 14 17:42:22 2014	
>> (r268624)
>>>> @@ -53,6 +53,7 @@ __FBSDID("$FreeBSD$");
>>>>   
>>>>   static vd_init_t vt_efifb_init;
>>>>   static vd_probe_t vt_efifb_probe;
>>>> +static void vt_efifb_remap(void *efifb_data);
>>>>   
>>>>   static struct vt_driver vt_efifb_driver = {
>>>>   	.vd_name = "efifb",
>>>> @@ -68,6 +69,8 @@ static struct vt_driver vt_efifb_driver
>>>>   static struct fb_info local_info;
>>>>   VT_DRIVER_DECLARE(vt_efifb, vt_efifb_driver);
>>>>   
>>>> +SYSINIT(efifb_remap, SI_SUB_KMEM, SI_ORDER_ANY, vt_efifb_remap,
>> &local_info);
>>>> +
>>>>   static int
>>>>   vt_efifb_probe(struct vt_device *vd)
>>>>   {
>>>> @@ -133,9 +136,9 @@ vt_efifb_init(struct vt_device *vd)
>>>>   	info->fb_size = info->fb_height * info->fb_stride;
>>>>   	info->fb_pbase = efifb->fb_addr;
>>>>   	/*
>>>> -	 * We could use pmap_mapdev here except that the kernel pmap
>>>> -	 * hasn't been created yet and hence any attempt to lock it will
>>>> -	 * fail.
>>>> +	 * Use the direct map as a crutch until pmap is available. Once pmap
>>>> +	 * is online, the framebuffer will be remapped by vt_efifb_remap()
>>>> +	 * using pmap_mapdev_attr().
>>>>   	 */
>>>>   	info->fb_vbase = PHYS_TO_DMAP(efifb->fb_addr);
>>>>   
>>>> @@ -163,3 +166,22 @@ vt_efifb_init(struct vt_device *vd)
>>>>   
>>>>   	return (CN_INTERNAL);
>>>>   }
>>>> +
>>>> +static void
>>>> +vt_efifb_remap(void *xinfo)
>>>> +{
>>>> +	struct fb_info *info = xinfo;
>>>> +
>>>> +	if (info->fb_pbase == 0)
>>>> +		return;
>>>> +
>>>> +	/*
>>>> +	 * Remap as write-combining. This massively improves performance and
>>>> +	 * happens very early in kernel initialization, when everything is
>>>> +	 * still single-threaded and interrupts are off, so replacing the
>>>> +	 * mapping address is safe.
>>>> +	 */
>>>> +	info->fb_vbase = (intptr_t)pmap_mapdev_attr(info->fb_pbase,
>>>> +	    info->fb_size, VM_MEMATTR_WRITE_COMBINING);
>>>> +}
>>>> +
>>> Could you use pmap_change_attr() ? This would save some KVA.
>> I think that is a no-op in this case.  pmap_mapdev_attr() on amd64 is already
>> going to re-use the existing DMAP mapping after doing pmap_change_attr() on
>> it.
>>
> Yes, it automatically uses the direct map:
>
> void *
> pmap_mapdev_attr(vm_paddr_t pa, vm_size_t size, int mode)
> {
>          vm_offset_t va, offset;
>          vm_size_t tmpsize;
>
>          /*
>           * If the specified range of physical addresses fits within the
> direct
>           * map window, use the direct map.
>           */
>          if (pa < dmaplimit && pa + size < dmaplimit) {
>                  va = PHYS_TO_DMAP(pa);
>                  if (!pmap_change_attr(va, size, mode))
>                          return ((void *)va);
>
>

Well, I'm glad I didn't fix it earlier :)
-Nathan