[PATCH v2 00/18] Introduce DRM_FB_HELPER_DEFAULT_OPS for struct fb_ops

[PATCH v2 00/18] Introduce DRM_FB_HELPER_DEFAULT_OPS for struct fb_ops

[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 */
    };

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(-)

-- 
2.7.3

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

[PATCH v2 01/18] drm/fb-helper: fix segfaults in drm_fb_helper_debug_*

[Stefan Christ]

A drm driver that is implementing

      fb_debug_enter and fb_debug_leave

in struct fb_ops with drm fb helper functions

     drm_fb_helper_debug_enter and drm_fb_helper_debug_leave

must also implement the callback 'mode_set_base_atomic' in struct
drm_crtc_helper_funcs. See Documentation/DocBook/kgdb.tmpl.  The current
implementation will segfault when 'mode_set_base_atomic' is a NULL
pointer.

Before this patch at least the drm drivers armada, ast, qxl, udl and
virtio do not have a 'mode_set_base_atomic' implementation but using
drm_fb_helper_debug_(enter|leave). So these drivers may segfault when
callbacks fb_debug_(enter|leave) are called.

Avoid the issue by just checking for NULL pointers. So all drivers can
unconditionally implement fb_debug_(enter|leave) with the drm_fb_helper
functions. If callback 'mode_set_base_atomic' is not implemented, the
code in drm_fb_helper_debug_(enter|leave) does effectively nothing.

Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
 drivers/gpu/drm/drm_fb_helper.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 03414bd..823fc9e 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -251,6 +251,9 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
 				continue;
 
 			funcs =	mode_set->crtc->helper_private;
+			if (funcs->mode_set_base_atomic == NULL)
+				continue;
+
 			drm_fb_helper_save_lut_atomic(mode_set->crtc, helper);
 			funcs->mode_set_base_atomic(mode_set->crtc,
 						    mode_set->fb,
@@ -304,6 +307,9 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
 			continue;
 		}
 
+		if (funcs->mode_set_base_atomic == NULL)
+			continue;
+
 		drm_fb_helper_restore_lut_atomic(mode_set->crtc);
 		funcs->mode_set_base_atomic(mode_set->crtc, fb, crtc->x,
 					    crtc->y, LEAVE_ATOMIC_MODE_SET);
-- 
2.7.3

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

[PATCH v2 02/18] drm/fb-helper: add fb_debug_* to DRM_FB_HELPER_DEFAULT_OPS

[Stefan Christ]

Add additional members fb_debug_enter and fb_debug_leave to helper
define. They are shared by all fb_ops implementations.

Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
 include/drm/drm_fb_helper.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index ed8edfe..975deed 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -228,7 +228,9 @@ struct drm_fb_helper {
 	.fb_set_par	= drm_fb_helper_set_par, \
 	.fb_setcmap	= drm_fb_helper_setcmap, \
 	.fb_blank	= drm_fb_helper_blank, \
-	.fb_pan_display	= drm_fb_helper_pan_display
+	.fb_pan_display	= drm_fb_helper_pan_display, \
+	.fb_debug_enter = drm_fb_helper_debug_enter, \
+	.fb_debug_leave = drm_fb_helper_debug_leave
 
 #ifdef CONFIG_DRM_FBDEV_EMULATION
 void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
-- 
2.7.3

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

[PATCH v2 03/18] drm/amdgpu: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Christ]

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
v2: Also remove fb_debug_(enter|leave) assignments
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index 9fb8aa4..8c23cf3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -75,18 +75,12 @@ amdgpufb_release(struct fb_info *info, int user)
 
 static struct fb_ops amdgpufb_ops = {
 	.owner = THIS_MODULE,
+	DRM_FB_HELPER_DEFAULT_OPS,
 	.fb_open = amdgpufb_open,
 	.fb_release = amdgpufb_release,
-	.fb_check_var = drm_fb_helper_check_var,
-	.fb_set_par = drm_fb_helper_set_par,
 	.fb_fillrect = drm_fb_helper_cfb_fillrect,
 	.fb_copyarea = drm_fb_helper_cfb_copyarea,
 	.fb_imageblit = drm_fb_helper_cfb_imageblit,
-	.fb_pan_display = drm_fb_helper_pan_display,
-	.fb_blank = drm_fb_helper_blank,
-	.fb_setcmap = drm_fb_helper_setcmap,
-	.fb_debug_enter = drm_fb_helper_debug_enter,
-	.fb_debug_leave = drm_fb_helper_debug_leave,
 };
 
 
-- 
2.7.3

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

[PATCH v2 04/18] drm/armada: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Christ]

Cc: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
v2: Also remove fb_debug_(enter|leave) assignments
---
 drivers/gpu/drm/armada/armada_fbdev.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_fbdev.c b/drivers/gpu/drm/armada/armada_fbdev.c
index ca73ad8..c5dc06a 100644
--- a/drivers/gpu/drm/armada/armada_fbdev.c
+++ b/drivers/gpu/drm/armada/armada_fbdev.c
@@ -19,16 +19,10 @@
 
 static /*const*/ struct fb_ops armada_fb_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	= drm_fb_helper_cfb_fillrect,
 	.fb_copyarea	= drm_fb_helper_cfb_copyarea,
 	.fb_imageblit	= drm_fb_helper_cfb_imageblit,
-	.fb_pan_display	= drm_fb_helper_pan_display,
-	.fb_blank	= drm_fb_helper_blank,
-	.fb_setcmap	= drm_fb_helper_setcmap,
-	.fb_debug_enter	= drm_fb_helper_debug_enter,
-	.fb_debug_leave	= drm_fb_helper_debug_leave,
 };
 
 static int armada_fb_create(struct drm_fb_helper *fbh,
-- 
2.7.3

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

[PATCH v2 05/18] drm/bochs: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Christ]

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
 drivers/gpu/drm/bochs/bochs_fbdev.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c b/drivers/gpu/drm/bochs/bochs_fbdev.c
index e1ec498..da790a1 100644
--- a/drivers/gpu/drm/bochs/bochs_fbdev.c
+++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
@@ -22,14 +22,10 @@ static int bochsfb_mmap(struct fb_info *info,
 
 static struct fb_ops bochsfb_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 = drm_fb_helper_sys_fillrect,
 	.fb_copyarea = drm_fb_helper_sys_copyarea,
 	.fb_imageblit = drm_fb_helper_sys_imageblit,
-	.fb_pan_display = drm_fb_helper_pan_display,
-	.fb_blank = drm_fb_helper_blank,
-	.fb_setcmap = drm_fb_helper_setcmap,
 	.fb_mmap = bochsfb_mmap,
 };
 
-- 
2.7.3

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

[PATCH v2 06/18] drm/fb_cma_helper: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Christ]

Cc: David Airlie <airlied@linux.ie>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
 drivers/gpu/drm/drm_fb_cma_helper.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 1fd6eac..71551fd 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -311,14 +311,10 @@ static int drm_fb_cma_mmap(struct fb_info *info, struct vm_area_struct *vma)
 
 static struct fb_ops drm_fbdev_cma_ops = {
 	.owner		= THIS_MODULE,
+	DRM_FB_HELPER_DEFAULT_OPS,
 	.fb_fillrect	= drm_fb_helper_sys_fillrect,
 	.fb_copyarea	= drm_fb_helper_sys_copyarea,
 	.fb_imageblit	= drm_fb_helper_sys_imageblit,
-	.fb_check_var	= drm_fb_helper_check_var,
-	.fb_set_par	= drm_fb_helper_set_par,
-	.fb_blank	= drm_fb_helper_blank,
-	.fb_pan_display	= drm_fb_helper_pan_display,
-	.fb_setcmap	= drm_fb_helper_setcmap,
 	.fb_mmap	= drm_fb_cma_mmap,
 };
 
-- 
2.7.3

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

[PATCH v2 07/18] drm/exynos: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Christ]

Cc: Inki Dae <inki.dae@samsung.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
v2: Add Acked-by from Inki Dae
---
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 4cfb39d..9f35deb 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -63,15 +63,11 @@ static int exynos_drm_fb_mmap(struct fb_info *info,
 
 static struct fb_ops exynos_drm_fb_ops = {
 	.owner		= THIS_MODULE,
+	DRM_FB_HELPER_DEFAULT_OPS,
 	.fb_mmap        = exynos_drm_fb_mmap,
 	.fb_fillrect	= drm_fb_helper_cfb_fillrect,
 	.fb_copyarea	= drm_fb_helper_cfb_copyarea,
 	.fb_imageblit	= drm_fb_helper_cfb_imageblit,
-	.fb_check_var	= drm_fb_helper_check_var,
-	.fb_set_par	= drm_fb_helper_set_par,
-	.fb_blank	= drm_fb_helper_blank,
-	.fb_pan_display	= drm_fb_helper_pan_display,
-	.fb_setcmap	= drm_fb_helper_setcmap,
 };
 
 static int exynos_drm_fbdev_update(struct drm_fb_helper *helper,
-- 
2.7.3

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

[PATCH v2 08/18] drm/nouveau: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Christ]

Cc: Ben Skeggs <bskeggs@redhat.com>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
v2: Also remove fb_debug_(enter|leave) assignments
---
 drivers/gpu/drm/nouveau/nouveau_fbcon.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 9f56927..5db55b0 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -199,35 +199,23 @@ nouveau_fbcon_release(struct fb_info *info, int user)
 
 static struct fb_ops nouveau_fbcon_ops = {
 	.owner = THIS_MODULE,
+	DRM_FB_HELPER_DEFAULT_OPS,
 	.fb_open = nouveau_fbcon_open,
 	.fb_release = nouveau_fbcon_release,
-	.fb_check_var = drm_fb_helper_check_var,
-	.fb_set_par = drm_fb_helper_set_par,
 	.fb_fillrect = nouveau_fbcon_fillrect,
 	.fb_copyarea = nouveau_fbcon_copyarea,
 	.fb_imageblit = nouveau_fbcon_imageblit,
 	.fb_sync = nouveau_fbcon_sync,
-	.fb_pan_display = drm_fb_helper_pan_display,
-	.fb_blank = drm_fb_helper_blank,
-	.fb_setcmap = drm_fb_helper_setcmap,
-	.fb_debug_enter = drm_fb_helper_debug_enter,
-	.fb_debug_leave = drm_fb_helper_debug_leave,
 };
 
 static struct fb_ops nouveau_fbcon_sw_ops = {
 	.owner = THIS_MODULE,
+	DRM_FB_HELPER_DEFAULT_OPS,
 	.fb_open = nouveau_fbcon_open,
 	.fb_release = nouveau_fbcon_release,
-	.fb_check_var = drm_fb_helper_check_var,
-	.fb_set_par = drm_fb_helper_set_par,
 	.fb_fillrect = drm_fb_helper_cfb_fillrect,
 	.fb_copyarea = drm_fb_helper_cfb_copyarea,
 	.fb_imageblit = drm_fb_helper_cfb_imageblit,
-	.fb_pan_display = drm_fb_helper_pan_display,
-	.fb_blank = drm_fb_helper_blank,
-	.fb_setcmap = drm_fb_helper_setcmap,
-	.fb_debug_enter = drm_fb_helper_debug_enter,
-	.fb_debug_leave = drm_fb_helper_debug_leave,
 };
 
 void
-- 
2.7.3

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

[PATCH v2 09/18] drm/qxl: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Christ]

This patch removes a TODO comment in the code. I do not know whether it
is still relevant.

Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
v2: Also remove fb_debug_(enter|leave) assignments
v2: Add note about TODO comment in commit message
---
 drivers/gpu/drm/qxl/qxl_fb.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c
index 2cd879a..7e305d8 100644
--- a/drivers/gpu/drm/qxl/qxl_fb.c
+++ b/drivers/gpu/drm/qxl/qxl_fb.c
@@ -81,16 +81,10 @@ static struct fb_deferred_io qxl_defio = {
 
 static struct fb_ops qxlfb_ops = {
 	.owner = THIS_MODULE,
-	.fb_check_var = drm_fb_helper_check_var,
-	.fb_set_par = drm_fb_helper_set_par, /* TODO: copy vmwgfx */
+	DRM_FB_HELPER_DEFAULT_OPS,
 	.fb_fillrect = drm_fb_helper_sys_fillrect,
 	.fb_copyarea = drm_fb_helper_sys_copyarea,
 	.fb_imageblit = drm_fb_helper_sys_imageblit,
-	.fb_pan_display = drm_fb_helper_pan_display,
-	.fb_blank = drm_fb_helper_blank,
-	.fb_setcmap = drm_fb_helper_setcmap,
-	.fb_debug_enter = drm_fb_helper_debug_enter,
-	.fb_debug_leave = drm_fb_helper_debug_leave,
 };
 
 static void qxlfb_destroy_pinned_object(struct drm_gem_object *gobj)
-- 
2.7.3

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

[PATCH v2 10/18] drm/rockchip: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Christ]

Cc: Mark Yao <mark.yao@rock-chips.com>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c
index a16c69f..8f639c8 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c
@@ -37,15 +37,11 @@ static int rockchip_fbdev_mmap(struct fb_info *info,
 
 static struct fb_ops rockchip_drm_fbdev_ops = {
 	.owner		= THIS_MODULE,
+	DRM_FB_HELPER_DEFAULT_OPS,
 	.fb_mmap	= rockchip_fbdev_mmap,
 	.fb_fillrect	= drm_fb_helper_cfb_fillrect,
 	.fb_copyarea	= drm_fb_helper_cfb_copyarea,
 	.fb_imageblit	= drm_fb_helper_cfb_imageblit,
-	.fb_check_var	= drm_fb_helper_check_var,
-	.fb_set_par	= drm_fb_helper_set_par,
-	.fb_blank	= drm_fb_helper_blank,
-	.fb_pan_display	= drm_fb_helper_pan_display,
-	.fb_setcmap	= drm_fb_helper_setcmap,
 };
 
 static int rockchip_drm_fbdev_create(struct drm_fb_helper *helper,
-- 
2.7.3

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

[PATCH v2 11/18] drm/radeon: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Christ]

Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
v2: Also remove fb_debug_(enter|leave) assignments
---
 drivers/gpu/drm/radeon/radeon_fb.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index 0daad44..6514422 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -74,18 +74,12 @@ radeonfb_release(struct fb_info *info, int user)
 
 static struct fb_ops radeonfb_ops = {
 	.owner = THIS_MODULE,
+	DRM_FB_HELPER_DEFAULT_OPS,
 	.fb_open = radeonfb_open,
 	.fb_release = radeonfb_release,
-	.fb_check_var = drm_fb_helper_check_var,
-	.fb_set_par = drm_fb_helper_set_par,
 	.fb_fillrect = drm_fb_helper_cfb_fillrect,
 	.fb_copyarea = drm_fb_helper_cfb_copyarea,
 	.fb_imageblit = drm_fb_helper_cfb_imageblit,
-	.fb_pan_display = drm_fb_helper_pan_display,
-	.fb_blank = drm_fb_helper_blank,
-	.fb_setcmap = drm_fb_helper_setcmap,
-	.fb_debug_enter = drm_fb_helper_debug_enter,
-	.fb_debug_leave = drm_fb_helper_debug_leave,
 };
 
 
-- 
2.7.3

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

[PATCH v2 12/18] drm/tegra: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Christ]

Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
 drivers/gpu/drm/tegra/fb.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index e6d71fa..e4a5ab0 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -186,14 +186,10 @@ struct drm_framebuffer *tegra_fb_create(struct drm_device *drm,
 #ifdef CONFIG_DRM_FBDEV_EMULATION
 static struct fb_ops tegra_fb_ops = {
 	.owner = THIS_MODULE,
+	DRM_FB_HELPER_DEFAULT_OPS,
 	.fb_fillrect = drm_fb_helper_sys_fillrect,
 	.fb_copyarea = drm_fb_helper_sys_copyarea,
 	.fb_imageblit = drm_fb_helper_sys_imageblit,
-	.fb_check_var = drm_fb_helper_check_var,
-	.fb_set_par = drm_fb_helper_set_par,
-	.fb_blank = drm_fb_helper_blank,
-	.fb_pan_display = drm_fb_helper_pan_display,
-	.fb_setcmap = drm_fb_helper_setcmap,
 };
 
 static int tegra_fbdev_probe(struct drm_fb_helper *helper,
-- 
2.7.3

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

[PATCH v2 13/18] drm/udl: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Christ]

Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
v2: Also remove fb_debug_(enter|leave) assignments
---
 drivers/gpu/drm/udl/udl_fb.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index 611b6b9..167f42c 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -254,16 +254,10 @@ static int udl_fb_release(struct fb_info *info, int user)
 
 static struct fb_ops udlfb_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 = drm_fb_helper_sys_fillrect,
 	.fb_copyarea = drm_fb_helper_sys_copyarea,
 	.fb_imageblit = drm_fb_helper_sys_imageblit,
-	.fb_pan_display = drm_fb_helper_pan_display,
-	.fb_blank = drm_fb_helper_blank,
-	.fb_setcmap = drm_fb_helper_setcmap,
-	.fb_debug_enter = drm_fb_helper_debug_enter,
-	.fb_debug_leave = drm_fb_helper_debug_leave,
 	.fb_mmap = udl_fb_mmap,
 	.fb_open = udl_fb_open,
 	.fb_release = udl_fb_release,
-- 
2.7.3

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

[PATCH v2 14/18] drm/msm: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Christ]

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
 drivers/gpu/drm/msm/msm_fbdev.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index ffd4a33..d29f5e8 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -39,6 +39,7 @@ struct msm_fbdev {
 
 static struct fb_ops msm_fb_ops = {
 	.owner = THIS_MODULE,
+	DRM_FB_HELPER_DEFAULT_OPS,
 
 	/* Note: to properly handle manual update displays, we wrap the
 	 * basic fbdev ops which write to the framebuffer
@@ -49,12 +50,6 @@ static struct fb_ops msm_fb_ops = {
 	.fb_copyarea = drm_fb_helper_sys_copyarea,
 	.fb_imageblit = drm_fb_helper_sys_imageblit,
 	.fb_mmap = msm_fbdev_mmap,
-
-	.fb_check_var = drm_fb_helper_check_var,
-	.fb_set_par = drm_fb_helper_set_par,
-	.fb_pan_display = drm_fb_helper_pan_display,
-	.fb_blank = drm_fb_helper_blank,
-	.fb_setcmap = drm_fb_helper_setcmap,
 };
 
 static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma)
-- 
2.7.3

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

[PATCH v2 15/18] drm/virtio: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Christ]

This patch removes a TODO comment in the code. I do not know whether it
is still relevant.

Cc: David Airlie <airlied@linux.ie>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
v2: Also remove fb_debug_(enter|leave) assignments
---
 drivers/gpu/drm/virtio/virtgpu_fb.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_fb.c b/drivers/gpu/drm/virtio/virtgpu_fb.c
index 2242a80..dd21f95 100644
--- a/drivers/gpu/drm/virtio/virtgpu_fb.c
+++ b/drivers/gpu/drm/virtio/virtgpu_fb.c
@@ -200,16 +200,10 @@ static void virtio_gpu_3d_imageblit(struct fb_info *info,
 
 static struct fb_ops virtio_gpufb_ops = {
 	.owner = THIS_MODULE,
-	.fb_check_var = drm_fb_helper_check_var,
-	.fb_set_par = drm_fb_helper_set_par, /* TODO: copy vmwgfx */
+	DRM_FB_HELPER_DEFAULT_OPS,
 	.fb_fillrect = virtio_gpu_3d_fillrect,
 	.fb_copyarea = virtio_gpu_3d_copyarea,
 	.fb_imageblit = virtio_gpu_3d_imageblit,
-	.fb_pan_display = drm_fb_helper_pan_display,
-	.fb_blank = drm_fb_helper_blank,
-	.fb_setcmap = drm_fb_helper_setcmap,
-	.fb_debug_enter = drm_fb_helper_debug_enter,
-	.fb_debug_leave = drm_fb_helper_debug_leave,
 };
 
 static int virtio_gpu_vmap_fb(struct virtio_gpu_device *vgdev,
-- 
2.7.3

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

[PATCH v2 16/18] drm/omapdrm: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Christ]

Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
 drivers/gpu/drm/omapdrm/omap_fbdev.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
index adb10fb..8d8ac17 100644
--- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
+++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
@@ -82,6 +82,7 @@ static int omap_fbdev_pan_display(struct fb_var_screeninfo *var,
 
 static struct fb_ops omap_fb_ops = {
 	.owner = THIS_MODULE,
+	DRM_FB_HELPER_DEFAULT_OPS,
 
 	/* Note: to properly handle manual update displays, we wrap the
 	 * basic fbdev ops which write to the framebuffer
@@ -92,11 +93,7 @@ static struct fb_ops omap_fb_ops = {
 	.fb_copyarea = drm_fb_helper_sys_copyarea,
 	.fb_imageblit = drm_fb_helper_sys_imageblit,
 
-	.fb_check_var = drm_fb_helper_check_var,
-	.fb_set_par = drm_fb_helper_set_par,
 	.fb_pan_display = omap_fbdev_pan_display,
-	.fb_blank = drm_fb_helper_blank,
-	.fb_setcmap = drm_fb_helper_setcmap,
 };
 
 static int omap_fbdev_create(struct drm_fb_helper *helper,
-- 
2.7.3

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

[PATCH v2 17/18] drm/i915: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Christ]

Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
v2: Also remove fb_debug_(enter|leave) assignments
---
 drivers/gpu/drm/i915/intel_fbdev.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index b7098f9..e87221b 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -102,16 +102,13 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var,
 
 static struct fb_ops intelfb_ops = {
 	.owner = THIS_MODULE,
-	.fb_check_var = drm_fb_helper_check_var,
+	DRM_FB_HELPER_DEFAULT_OPS,
 	.fb_set_par = intel_fbdev_set_par,
 	.fb_fillrect = drm_fb_helper_cfb_fillrect,
 	.fb_copyarea = drm_fb_helper_cfb_copyarea,
 	.fb_imageblit = drm_fb_helper_cfb_imageblit,
 	.fb_pan_display = intel_fbdev_pan_display,
 	.fb_blank = intel_fbdev_blank,
-	.fb_setcmap = drm_fb_helper_setcmap,
-	.fb_debug_enter = drm_fb_helper_debug_enter,
-	.fb_debug_leave = drm_fb_helper_debug_leave,
 };
 
 static int intelfb_alloc(struct drm_fb_helper *helper,
-- 
2.7.3

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

[PATCH v2 18/18] drm/gma500: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops

[Stefan Christ]

This refactoring leads to real functional changes in the driver.

Now the struct psbfb_ops implements two additional members:

       .fb_setcmap     = drm_fb_helper_setcmap,
       .fb_pan_display = drm_fb_helper_pan_display,

and the struct psbfb_roll_ops implements one additional member:

       .fb_setcmap     = drm_fb_helper_setcmap,

and the struct psbfb_unaccel_ops implements two additional members:

       .fb_setcmap     = drm_fb_helper_setcmap,
       .fb_pan_display = drm_fb_helper_pan_display,

These changes are not tested.

Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
 drivers/gpu/drm/gma500/framebuffer.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
index 3a44e70..1bd156e 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -185,9 +185,7 @@ static int psbfb_mmap(struct fb_info *info, struct vm_area_struct *vma)
 
 static struct fb_ops psbfb_ops = {
 	.owner = THIS_MODULE,
-	.fb_check_var = drm_fb_helper_check_var,
-	.fb_set_par = drm_fb_helper_set_par,
-	.fb_blank = drm_fb_helper_blank,
+	DRM_FB_HELPER_DEFAULT_OPS,
 	.fb_setcolreg = psbfb_setcolreg,
 	.fb_fillrect = drm_fb_helper_cfb_fillrect,
 	.fb_copyarea = psbfb_copyarea,
@@ -198,9 +196,7 @@ static struct fb_ops psbfb_ops = {
 
 static struct fb_ops psbfb_roll_ops = {
 	.owner = THIS_MODULE,
-	.fb_check_var = drm_fb_helper_check_var,
-	.fb_set_par = drm_fb_helper_set_par,
-	.fb_blank = drm_fb_helper_blank,
+	DRM_FB_HELPER_DEFAULT_OPS,
 	.fb_setcolreg = psbfb_setcolreg,
 	.fb_fillrect = drm_fb_helper_cfb_fillrect,
 	.fb_copyarea = drm_fb_helper_cfb_copyarea,
@@ -211,9 +207,7 @@ static struct fb_ops psbfb_roll_ops = {
 
 static struct fb_ops psbfb_unaccel_ops = {
 	.owner = THIS_MODULE,
-	.fb_check_var = drm_fb_helper_check_var,
-	.fb_set_par = drm_fb_helper_set_par,
-	.fb_blank = drm_fb_helper_blank,
+	DRM_FB_HELPER_DEFAULT_OPS,
 	.fb_setcolreg = psbfb_setcolreg,
 	.fb_fillrect = drm_fb_helper_cfb_fillrect,
 	.fb_copyarea = drm_fb_helper_cfb_copyarea,
-- 
2.7.3

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

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

[Daniel Vetter]

On Mon, Nov 14, 2016 at 12:03:10AM +0100, Stefan Christ wrote:
> 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 */
>     };
> 
> 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.

Applied the entire series to drm-misc, thanks.
-Daniel

> 
> 
> 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(-)
> 
> -- 
> 2.7.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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

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

[Christian König]

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(-)
>

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

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

[Stefan Christ]

Hi Christian,

On Mon, Nov 14, 2016 at 10:43:10AM +0100, Christian König wrote:
> 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.
> 

In the initial suggestion by Daniel Vetter he also mentioned this
possibility. See email and response:

    https://lists.freedesktop.org/archives/dri-devel/2016-July/114136.html

But I decided against it, since it hides even more magic behind the
helper define.

Mit freundlichen Grüßen / Kind regards,
	Stefan Christ

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