Re: [PATCH 2/3] drm: fb_helper: implement ioctl FBIO_WAITFORVSYNC

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jul 13, 2016 at 12:16:05PM +0200, Daniel Vetter wrote:
> On Wed, Jul 13, 2016 at 10:11:47AM +0200, Stefan Christ wrote:
> > Implement legacy framebuffer ioctl FBIO_WAITFORVSYNC in the generic
> > framebuffer emulation driver. Legacy framebuffer users like non kms/drm
> > based OpenGL(ES)/EGL implementations may require the ioctl to
> > synchronize drawing or buffer flip for double buffering. It is tested on
> > the i.MX6.
> > 
> > Code is based on
> >     https://github.com/Xilinx/linux-xlnx/blob/master/drivers/gpu/drm/xilinx/xilinx_drm_fb.c#L196
> > 
> > Signed-off-by: Stefan Christ <s.christ@phytec.de>
> > ---
> >  drivers/gpu/drm/drm_fb_cma_helper.c |  1 +
> >  drivers/gpu/drm/drm_fb_helper.c     | 43 +++++++++++++++++++++++++++++++++++++
> >  include/drm/drm_fb_helper.h         |  2 ++
> >  3 files changed, 46 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
> > index be66042..95657da 100644
> > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > @@ -313,6 +313,7 @@ static struct fb_ops drm_fbdev_cma_ops = {
> >  	.fb_blank	= drm_fb_helper_blank,
> >  	.fb_pan_display	= drm_fb_helper_pan_display,
> >  	.fb_setcmap	= drm_fb_helper_setcmap,
> > +	.fb_ioctl       = drm_fb_helper_ioctl,
> >  };
> >  
> >  static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info,
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index 7c2eb75..4d1f9b9 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -1167,6 +1167,49 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
> >  EXPORT_SYMBOL(drm_fb_helper_setcmap);
> >  
> >  /**
> > + * drm_fb_helper_ioctl - legacy ioctl implementation
> > + * @info:
> > + * @cmd: ioctl like FBIO_WAITFORVSYNC
> > + * @arg: ioctl argument
> 
> A bit more verbose kerneldoc would be great. Also, if you add this I think
> we should roll it out for all drivers, for consistency of the supported
> fbdev features when using emulation.

To roll new vfuncs out consistently I think it'd be great to create a
DRM_FB_HELPER_DEFAULT_OPS #define which sets all the fb_ops. Drivers can
then overwrite just what they need, e.g.

static struct fb_ops drm_fbdev_cma_ops = {
	.owner		= THIS_MODULE,
	DRM_FB_HELPER_DEFAULT_OPS,
	.fb_mmap 	= drm_fb_cma_mmap,
};

Maybe even include the .owner = THIS_MODULE line in the macro, but that
might be too much magic and mislead people into reusing it when it's not
the same module.

Of course this would be an additional (subsystem-wide) prep patch before
this one here.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel