Am 14.11.2016 um 00:03 schrieb Stefan Christ: > Hi, > > this is the second version of the 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 */ > };
Looks good to me in general, but I've got one question. Why didn't you put the owner field into the macro as well? I have strong doubts that anybody would set anything else than THIS_MODULE here. Regards, Christian. > > The first patch that adds the helper define was already accepted into the > kernel release candidate v4.9-rc1. > > For the new stuff: > > Implementing fb_debug_enter and fb_debug_leave callbacks is not without > pitfalls. It was necessary that driver also implements the > mode_set_base_atomic > callback in struct drm_crtc_helper_funcs. Otherwise it may segfaults. The > first > patch in this series changes that and introduces a check for it. Now > functions > drm_fb_helper_debug_(enter|leave) can be used without implementing callback > mode_set_base_atomic. At least the drm drivers armada, ast, qxl, udl and > virtio > are affected by this issue. It seems that the code path was never executed for > these drivers and nobody noticed. > > Refactoring patches for ast, cirrus and mgag200 are dropped. I leave it to > Daniel if additional drivers should not be converted. > > This series is based on tag v4.9-rc1 and applies cleanly onto drm-next, too. > > Kind regards, > Stefan Christ > > > Changes in v2: > - add patch 'fix segfaults in drm_fb_helper_debug_*'. > So functions drm_fb_helper_debug_(enter|leave) can be used > unconditionally. > - add patch 'add fb_debug_* to DRM_FB_HELPER_DEFAULT_OPS' that > uses drm_fb_helper_debug_(enter|leave) to implement > fb_debug_(enter|leave). It depends on the previous patch > to avoid regressions. > - Drop patches for drm drivers 'ast', 'cirrus' and 'mgag200' > since they reimplemented most of the fb_ops callbacks anyway. > - Also remove fb_debug_(enter|leave) callbacks in fb_ops all > refactoring patches. > > > Stefan Christ (18): > drm/fb-helper: fix segfaults in drm_fb_helper_debug_* > drm/fb-helper: add fb_debug_* to DRM_FB_HELPER_DEFAULT_OPS > drm/amdgpu: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops > drm/armada: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops > drm/bochs: 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/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 | 8 +------- > drivers/gpu/drm/armada/armada_fbdev.c | 8 +------- > drivers/gpu/drm/bochs/bochs_fbdev.c | 6 +----- > drivers/gpu/drm/drm_fb_cma_helper.c | 6 +----- > drivers/gpu/drm/drm_fb_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 | 5 +---- > drivers/gpu/drm/msm/msm_fbdev.c | 7 +------ > drivers/gpu/drm/nouveau/nouveau_fbcon.c | 16 ++-------------- > drivers/gpu/drm/omapdrm/omap_fbdev.c | 5 +---- > drivers/gpu/drm/qxl/qxl_fb.c | 8 +------- > drivers/gpu/drm/radeon/radeon_fb.c | 8 +------- > drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 6 +----- > drivers/gpu/drm/tegra/fb.c | 6 +----- > drivers/gpu/drm/udl/udl_fb.c | 8 +------- > drivers/gpu/drm/virtio/virtgpu_fb.c | 8 +------- > include/drm/drm_fb_helper.h | 4 +++- > 18 files changed, 28 insertions(+), 105 deletions(-) >
