Re: [PATCH 00/20] Introduce DRM_FB_HELPER_DEFAULT_OPS for struct fb_ops

[Gustavo Padovan <gustavo@padovan.org>]

Hi Stefan,

2016-09-29 Stefan Christ <contact@stefanchrist.eu>:

> Hi,
> 
> this series is refactoring work suggested by Daniel Vetter in the email:
> 
>    https://lists.freedesktop.org/archives/dri-devel/2016-July/113237.html
> 
> The define DRM_FB_HELPER_DEFAULT_OPS provides the drm_fb_helper default
> implementations for functions in struct fb_ops. A drm driver can use it like:
> 
>     static struct fb_ops drm_fbdev_cma_ops = {
>         .owner          = THIS_MODULE,
>         DRM_FB_HELPER_DEFAULT_OPS,
>         /* driver specific implementations */
>     };
> 
> The patches do not make any functional change to the kernel binary except
> driver 'drm/gma500'. The patch for gma500 enables two new functions (fb_setcmap
> and fb_pan_display) in fb_ops.  If this is not appropriate, the driver may
> reassign the struct members to null.
> 
> There is no refactoring patch for driver 'vmwgfx'. It reimplements nearly all
> fb_ops with driver specific functions anyways. 
> 
> This series is based on tag v4.8-rc8. If rebased onto 'drm-next' there are two
> small conflicts.
> 
> Kind regards,
> 	Stefan Christ
> 
> Stefan Christ (20):
>   drm/fb-helper: add DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/amdgpu: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/armada: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/ast: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/bochs: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/cirrus: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/fb_cma_helper: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/exynos: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/mgag200: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/nouveau: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/qxl: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/rockchip: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/radeon: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/tegra: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/udl: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/msm: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/virtio: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/omapdrm: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/i915: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
>   drm/gma500: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c        |  6 +-----
>  drivers/gpu/drm/armada/armada_fbdev.c         |  6 +-----
>  drivers/gpu/drm/ast/ast_fb.c                  |  6 +-----
>  drivers/gpu/drm/bochs/bochs_fbdev.c           |  6 +-----
>  drivers/gpu/drm/cirrus/cirrus_fbdev.c         |  6 +-----
>  drivers/gpu/drm/drm_fb_cma_helper.c           |  6 +-----
>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c     |  6 +-----
>  drivers/gpu/drm/gma500/framebuffer.c          | 12 +++---------
>  drivers/gpu/drm/i915/intel_fbdev.c            |  3 +--
>  drivers/gpu/drm/mgag200/mgag200_fb.c          |  6 +-----
>  drivers/gpu/drm/msm/msm_fbdev.c               |  7 +------
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c       | 12 ++----------
>  drivers/gpu/drm/omapdrm/omap_fbdev.c          |  5 +----
>  drivers/gpu/drm/qxl/qxl_fb.c                  |  6 +-----
>  drivers/gpu/drm/radeon/radeon_fb.c            |  6 +-----
>  drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c |  6 +-----
>  drivers/gpu/drm/tegra/fb.c                    |  6 +-----
>  drivers/gpu/drm/udl/udl_fb.c                  |  6 +-----
>  drivers/gpu/drm/virtio/virtgpu_fb.c           |  6 +-----
>  include/drm/drm_fb_helper.h                   | 13 +++++++++++++
>  20 files changed, 35 insertions(+), 101 deletions(-)

This looks good to me.

Reviewed-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

I wonder if we shouldn't start doing the same to drm_crtc_funcs and
drm_plane_funcs after we added the atomic helpers.

Gustavo
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel