[PATCH 04/20] drm/ast: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Lengfeld]

Hi Daniel,

On Tue, Oct 25, 2016 at 04:57:37PM +0200, Daniel Vetter wrote:
> On Thu, Sep 29, 2016 at 10:48:40PM +0200, Stefan Christ wrote:
> > Cc: Dave Airlie <airlied at redhat.com>
> > Signed-off-by: Stefan Christ <contact at stefanchrist.eu>
> > ---
> >  drivers/gpu/drm/ast/ast_fb.c | 6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
> > index c017a93..b604fdd 100644
> > --- a/drivers/gpu/drm/ast/ast_fb.c
> > +++ b/drivers/gpu/drm/ast/ast_fb.c
> > @@ -150,14 +150,10 @@ static void ast_imageblit(struct fb_info *info,
> >  
> >  static struct fb_ops astfb_ops = {
> >  	.owner = THIS_MODULE,
> > -	.fb_check_var = drm_fb_helper_check_var,
> > -	.fb_set_par = drm_fb_helper_set_par,
> > +	DRM_FB_HELPER_DEFAULT_OPS,
> >  	.fb_fillrect = ast_fillrect,
> >  	.fb_copyarea = ast_copyarea,
> >  	.fb_imageblit = ast_imageblit,
> 
> Ah, here's the likely reason for not sharing these, ast/cirrus have their
> special dirtying stuff for fbdev emulation. And because the fbdev mmap
> must have a stable pointer it can't use the ttm bo mmap magic of
> automatically picking the right place for the mmap.
> 
> I'd say just leave these 2 drivers out as special cases.
> -Daniel

Hmm. There are even more drivers using special implementations like the
mgag200 using function mga_fillrect(), which is a wrapper around
drm_fb_helper_sys_fillrect().

Even if the drivers ast/cirrus/.. are left out, there are still two
different fb_fillrect, fb_copyarea and fb_imageblit implementations:
   1/  drm_fb_helper_sys_*() and 
   2/  drm_fb_helper_cfb_*()
used by different drivers. I don't know which one should be preferred.

Including fb_debug_enter and fb_debug_leave in DRM_FB_HELPER_DEFAULT_OPS
is not a problem since there is only a single implementation yet.

Should I resend this series (without the first patch), but with
additional memebers fb_debug_enter and fb_debug_leave?

Kind regards,
	Stefan Christ

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