[PATCH] drm/fb-helper: Mark screen buffers in system memory with FB_VIRTFB

[Daniel Vetter]

On Thu, Jan 27, 2022 at 12:58:30PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 27.01.22 um 12:42 schrieb Daniel Vetter:
> > On Thu, Jan 27, 2022 at 11:26:21AM +0100, Thomas Zimmermann wrote:
> > > Mark screen buffers in system memory with FB_VIRTFB. Otherwise, the
> > > buffers are mmap'ed as I/O memory (i.e., VM_IO). For shadow buffers,
> > > also set the FB_READS_FAST hint.
> > 
> > Maybe clarify that this only holds for the defio case, and since we have
> > our own shadow copy for that anyway it shouldn't matter. I'm also not sure
> > how much the memcpy gains us compared to just redrawing ...
> > 
> > What's the motivation here, or just something you spotted?
> 
> Correctness mostly. fbdev's fbdefio tests for (the absence of) this flag and
> sets VM_IO accordingly.
> 
> It's actually for userspace. Maybe userspace tests these flags as well and
> can optimize memcpy pattern for different types of caching. But I wouldn't
> expect it TBH.

Hm I thought so too, but the #define is in the internal header, not the
uapi header. And I don't see any ioctl code in fbmem.c that would shove
fb_info->flags to userspace. That's why I wondered why you care about
this? Or did I miss something somewhere?
-Daniel

> 
> Best regards
> Thomas
> 
> > -Daniel
> > 
> > > 
> > > Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> > > ---
> > >   drivers/gpu/drm/drm_fb_helper.c | 9 ++++++---
> > >   1 file changed, 6 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > > index ed43b987d306..f15127a32f7a 100644
> > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > @@ -2346,6 +2346,7 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
> > >   	fbi->fbops = &drm_fbdev_fb_ops;
> > >   	fbi->screen_size = sizes->surface_height * fb->pitches[0];
> > >   	fbi->fix.smem_len = fbi->screen_size;
> > > +	fbi->flags = FBINFO_DEFAULT;
> > >   	drm_fb_helper_fill_info(fbi, fb_helper, sizes);
> > > @@ -2353,19 +2354,21 @@ static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
> > >   		fbi->screen_buffer = vzalloc(fbi->screen_size);
> > >   		if (!fbi->screen_buffer)
> > >   			return -ENOMEM;
> > > +		fbi->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST;
> > >   		fbi->fbdefio = &drm_fbdev_defio;
> > > -
> > >   		fb_deferred_io_init(fbi);
> > >   	} else {
> > >   		/* buffer is mapped for HW framebuffer */
> > >   		ret = drm_client_buffer_vmap(fb_helper->buffer, &map);
> > >   		if (ret)
> > >   			return ret;
> > > -		if (map.is_iomem)
> > > +		if (map.is_iomem) {
> > >   			fbi->screen_base = map.vaddr_iomem;
> > > -		else
> > > +		} else {
> > >   			fbi->screen_buffer = map.vaddr;
> > > +			fbi->flags |= FBINFO_VIRTFB;
> > > +		}
> > >   		/*
> > >   		 * Shamelessly leak the physical address to user-space. As
> > > -- 
> > > 2.34.1
> > > 
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev




-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch