All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] drm/vram: Provide helpers for prepare_fb() and cleanup_fb()
@ 2019-10-22 10:25 Thomas Zimmermann
  2019-10-22 10:25 ` [PATCH 1/5] drm/simple-kms-helper: Pass plane to " Thomas Zimmermann
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Thomas Zimmermann @ 2019-10-22 10:25 UTC (permalink / raw)
  To: kraxel, noralf, sam, daniel, airlied, joel, andrew,
	z.liuxinliang, zourongrong, kong.kongxinwei, puck.chen,
	linus.walleij, marex, stefan, shawnguo, s.hauer, kernel,
	festevam, linux-imx, eric, david, hdegoede,
	oleksandr_andrushchenko, liviu.dudau, ck.hu
  Cc: Thomas Zimmermann, dri-devel

The implementation of the plane's call-back functions prepare_fb() and
cleanup_fb() for GEM VRAM helpers are sharable among drivers.

The first patch adapts the the interface of simple KSM helpers such that
bochs can benefit from the shared code. We add the helper functions in
patch #2. Patches #3 to #5 covert several drivers to the new helpers.

Patch #4 also fixes two bugs that have been present in hibmc since it was
first added. The primary plane's atomic_update() is not responsible for
pinning BOs. And it never unpinned unused BOs. VRAM is being exausted
over time.

The new helpers have been tested to work. The drivers affected by the
interface change have been compiled at least.

Thomas Zimmermann (5):
  drm/simple-kms-helper: Pass plane to prepare_fb() and cleanup_fb()
  drm/vram-helpers: Add helpers for struct drm_plane_helper_funcs
  drm/bochs: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers
  drm/hisilicon/hibmc: Use GEM VRAM's prepare_fb() and cleanup_fb()
    helpers
  drm/vboxvideo: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers

 drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c      |  2 +-
 drivers/gpu/drm/bochs/bochs_kms.c             | 26 +-----
 drivers/gpu/drm/drm_gem_framebuffer_helper.c  | 22 -----
 drivers/gpu/drm/drm_gem_vram_helper.c         | 81 +++++++++++++++++++
 drivers/gpu/drm/drm_simple_kms_helper.c       |  4 +-
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c    | 14 +---
 drivers/gpu/drm/mcde/mcde_display.c           |  2 +-
 drivers/gpu/drm/mxsfb/mxsfb_drv.c             |  2 +-
 drivers/gpu/drm/pl111/pl111_display.c         |  2 +-
 drivers/gpu/drm/tiny/hx8357d.c                |  2 +-
 drivers/gpu/drm/tiny/ili9225.c                |  2 +-
 drivers/gpu/drm/tiny/ili9341.c                |  2 +-
 drivers/gpu/drm/tiny/mi0283qt.c               |  2 +-
 drivers/gpu/drm/tiny/repaper.c                |  2 +-
 drivers/gpu/drm/tiny/st7586.c                 |  2 +-
 drivers/gpu/drm/tiny/st7735r.c                |  2 +-
 drivers/gpu/drm/tve200/tve200_display.c       |  2 +-
 drivers/gpu/drm/vboxvideo/vbox_mode.c         | 61 +-------------
 drivers/gpu/drm/xen/xen_drm_front_kms.c       |  2 +-
 include/drm/drm_gem_framebuffer_helper.h      |  4 +-
 include/drm/drm_gem_vram_helper.h             | 12 +++
 include/drm/drm_plane.h                       |  2 +-
 include/drm/drm_simple_kms_helper.h           |  6 +-
 23 files changed, 123 insertions(+), 135 deletions(-)

--
2.23.0

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/5] drm/simple-kms-helper: Pass plane to prepare_fb() and cleanup_fb()
  2019-10-22 10:25 [PATCH 0/5] drm/vram: Provide helpers for prepare_fb() and cleanup_fb() Thomas Zimmermann
@ 2019-10-22 10:25 ` Thomas Zimmermann
  2019-10-22 14:14   ` Daniel Vetter
  2019-10-22 10:25 ` [PATCH 2/5] drm/vram-helpers: Add helpers for struct drm_plane_helper_funcs Thomas Zimmermann
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Thomas Zimmermann @ 2019-10-22 10:25 UTC (permalink / raw)
  To: kraxel, noralf, sam, daniel, airlied, joel, andrew,
	z.liuxinliang, zourongrong, kong.kongxinwei, puck.chen,
	linus.walleij, marex, stefan, shawnguo, s.hauer, kernel,
	festevam, linux-imx, eric, david, hdegoede,
	oleksandr_andrushchenko, liviu.dudau, ck.hu
  Cc: Thomas Zimmermann, dri-devel

Passing the plane structure to prepare_fb() and cleanup_fb() of
struct drm_simple_display_pipe_funcs unifies the interface with
struct drm_plane_helper_funcs. Implementations of these functions
can now be shared between simple-pipeline and 'full-pipeline' drivers.

Before, the functions received the simple display pipeline's structure
as their first argument. Apparently no implementation needed that argument
and anyone interested can still get it from the plane via container_of().

As a side effect, drm_gem_fb_simple_display_pipe_prepare_fb() has been
removed.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c     |  2 +-
 drivers/gpu/drm/bochs/bochs_kms.c            |  4 ++--
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 22 --------------------
 drivers/gpu/drm/drm_simple_kms_helper.c      |  4 ++--
 drivers/gpu/drm/mcde/mcde_display.c          |  2 +-
 drivers/gpu/drm/mxsfb/mxsfb_drv.c            |  2 +-
 drivers/gpu/drm/pl111/pl111_display.c        |  2 +-
 drivers/gpu/drm/tiny/hx8357d.c               |  2 +-
 drivers/gpu/drm/tiny/ili9225.c               |  2 +-
 drivers/gpu/drm/tiny/ili9341.c               |  2 +-
 drivers/gpu/drm/tiny/mi0283qt.c              |  2 +-
 drivers/gpu/drm/tiny/repaper.c               |  2 +-
 drivers/gpu/drm/tiny/st7586.c                |  2 +-
 drivers/gpu/drm/tiny/st7735r.c               |  2 +-
 drivers/gpu/drm/tve200/tve200_display.c      |  2 +-
 drivers/gpu/drm/xen/xen_drm_front_kms.c      |  2 +-
 include/drm/drm_gem_framebuffer_helper.h     |  4 +---
 include/drm/drm_plane.h                      |  2 +-
 include/drm/drm_simple_kms_helper.h          |  6 +++---
 19 files changed, 22 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
index 2184b8be6fd4..0fe72f46f397 100644
--- a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
@@ -219,7 +219,7 @@ static const struct drm_simple_display_pipe_funcs aspeed_gfx_funcs = {
 	.enable		= aspeed_gfx_pipe_enable,
 	.disable	= aspeed_gfx_pipe_disable,
 	.update		= aspeed_gfx_pipe_update,
-	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
+	.prepare_fb	= drm_gem_fb_prepare_fb,
 	.enable_vblank	= aspeed_gfx_enable_vblank,
 	.disable_vblank	= aspeed_gfx_disable_vblank,
 };
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
index 02a9c1ed165b..0891640491eb 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -69,7 +69,7 @@ static void bochs_pipe_update(struct drm_simple_display_pipe *pipe,
 	}
 }
 
-static int bochs_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
+static int bochs_pipe_prepare_fb(struct drm_plane *plane,
 				 struct drm_plane_state *new_state)
 {
 	struct drm_gem_vram_object *gbo;
@@ -80,7 +80,7 @@ static int bochs_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
 	return drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
 }
 
-static void bochs_pipe_cleanup_fb(struct drm_simple_display_pipe *pipe,
+static void bochs_pipe_cleanup_fb(struct drm_plane *plane,
 				  struct drm_plane_state *old_state)
 {
 	struct drm_gem_vram_object *gbo;
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index b9bcd310ca2d..c7ea288bef0a 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -300,25 +300,3 @@ int drm_gem_fb_prepare_fb(struct drm_plane *plane,
 	return 0;
 }
 EXPORT_SYMBOL_GPL(drm_gem_fb_prepare_fb);
-
-/**
- * drm_gem_fb_simple_display_pipe_prepare_fb - prepare_fb helper for
- *     &drm_simple_display_pipe
- * @pipe: Simple display pipe
- * @plane_state: Plane state
- *
- * This function uses drm_gem_fb_prepare_fb() to extract the exclusive fence
- * from &drm_gem_object.resv and attaches it to plane state for the atomic
- * helper to wait on. This is necessary to correctly implement implicit
- * synchronization for any buffers shared as a struct &dma_buf. Drivers can use
- * this as their &drm_simple_display_pipe_funcs.prepare_fb callback.
- *
- * See drm_atomic_set_fence_for_plane() for a discussion of implicit and
- * explicit fencing in atomic modeset updates.
- */
-int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
-					      struct drm_plane_state *plane_state)
-{
-	return drm_gem_fb_prepare_fb(&pipe->plane, plane_state);
-}
-EXPORT_SYMBOL(drm_gem_fb_simple_display_pipe_prepare_fb);
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index 046055719245..e51f336e67f0 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -173,7 +173,7 @@ static int drm_simple_kms_plane_prepare_fb(struct drm_plane *plane,
 	if (!pipe->funcs || !pipe->funcs->prepare_fb)
 		return 0;
 
-	return pipe->funcs->prepare_fb(pipe, state);
+	return pipe->funcs->prepare_fb(&pipe->plane, state);
 }
 
 static void drm_simple_kms_plane_cleanup_fb(struct drm_plane *plane,
@@ -185,7 +185,7 @@ static void drm_simple_kms_plane_cleanup_fb(struct drm_plane *plane,
 	if (!pipe->funcs || !pipe->funcs->cleanup_fb)
 		return;
 
-	pipe->funcs->cleanup_fb(pipe, state);
+	pipe->funcs->cleanup_fb(&pipe->plane, state);
 }
 
 static bool drm_simple_kms_format_mod_supported(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/mcde/mcde_display.c b/drivers/gpu/drm/mcde/mcde_display.c
index 751454ae3cd1..31efbdcced5d 100644
--- a/drivers/gpu/drm/mcde/mcde_display.c
+++ b/drivers/gpu/drm/mcde/mcde_display.c
@@ -1097,7 +1097,7 @@ static struct drm_simple_display_pipe_funcs mcde_display_funcs = {
 	.enable = mcde_display_enable,
 	.disable = mcde_display_disable,
 	.update = mcde_display_update,
-	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
+	.prepare_fb = drm_gem_fb_prepare_fb,
 };
 
 int mcde_display_init(struct drm_device *drm)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 497cf443a9af..547388b32c64 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -186,7 +186,7 @@ static struct drm_simple_display_pipe_funcs mxsfb_funcs = {
 	.enable		= mxsfb_pipe_enable,
 	.disable	= mxsfb_pipe_disable,
 	.update		= mxsfb_pipe_update,
-	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
+	.prepare_fb	= drm_gem_fb_prepare_fb,
 	.enable_vblank	= mxsfb_pipe_enable_vblank,
 	.disable_vblank	= mxsfb_pipe_disable_vblank,
 };
diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index 024771a4083e..e564c838a081 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -441,7 +441,7 @@ static struct drm_simple_display_pipe_funcs pl111_display_funcs = {
 	.enable = pl111_display_enable,
 	.disable = pl111_display_disable,
 	.update = pl111_display_update,
-	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
+	.prepare_fb = drm_gem_fb_prepare_fb,
 };
 
 static int pl111_clk_div_choose_div(struct clk_hw *hw, unsigned long rate,
diff --git a/drivers/gpu/drm/tiny/hx8357d.c b/drivers/gpu/drm/tiny/hx8357d.c
index 9af8ff84974f..235223190c04 100644
--- a/drivers/gpu/drm/tiny/hx8357d.c
+++ b/drivers/gpu/drm/tiny/hx8357d.c
@@ -183,7 +183,7 @@ static const struct drm_simple_display_pipe_funcs hx8357d_pipe_funcs = {
 	.enable = yx240qv29_enable,
 	.disable = mipi_dbi_pipe_disable,
 	.update = mipi_dbi_pipe_update,
-	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
+	.prepare_fb = drm_gem_fb_prepare_fb,
 };
 
 static const struct drm_display_mode yx350hv15_mode = {
diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index c66acc566c2b..39b0d425359a 100644
--- a/drivers/gpu/drm/tiny/ili9225.c
+++ b/drivers/gpu/drm/tiny/ili9225.c
@@ -342,7 +342,7 @@ static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = {
 	.enable		= ili9225_pipe_enable,
 	.disable	= ili9225_pipe_disable,
 	.update		= ili9225_pipe_update,
-	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
+	.prepare_fb	= drm_gem_fb_prepare_fb,
 };
 
 static const struct drm_display_mode ili9225_mode = {
diff --git a/drivers/gpu/drm/tiny/ili9341.c b/drivers/gpu/drm/tiny/ili9341.c
index 33b51dc7faa8..d107b6172bbb 100644
--- a/drivers/gpu/drm/tiny/ili9341.c
+++ b/drivers/gpu/drm/tiny/ili9341.c
@@ -139,7 +139,7 @@ static const struct drm_simple_display_pipe_funcs ili9341_pipe_funcs = {
 	.enable = yx240qv29_enable,
 	.disable = mipi_dbi_pipe_disable,
 	.update = mipi_dbi_pipe_update,
-	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
+	.prepare_fb = drm_gem_fb_prepare_fb,
 };
 
 static const struct drm_display_mode yx240qv29_mode = {
diff --git a/drivers/gpu/drm/tiny/mi0283qt.c b/drivers/gpu/drm/tiny/mi0283qt.c
index e2cfd9a17143..811143a17c77 100644
--- a/drivers/gpu/drm/tiny/mi0283qt.c
+++ b/drivers/gpu/drm/tiny/mi0283qt.c
@@ -143,7 +143,7 @@ static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = {
 	.enable = mi0283qt_enable,
 	.disable = mipi_dbi_pipe_disable,
 	.update = mipi_dbi_pipe_update,
-	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
+	.prepare_fb = drm_gem_fb_prepare_fb,
 };
 
 static const struct drm_display_mode mi0283qt_mode = {
diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
index 76d179200775..9228d813d3d2 100644
--- a/drivers/gpu/drm/tiny/repaper.c
+++ b/drivers/gpu/drm/tiny/repaper.c
@@ -874,7 +874,7 @@ static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = {
 	.enable = repaper_pipe_enable,
 	.disable = repaper_pipe_disable,
 	.update = repaper_pipe_update,
-	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
+	.prepare_fb = drm_gem_fb_prepare_fb,
 };
 
 static int repaper_connector_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
index 3cc21a1b30c8..7b837c46bbfc 100644
--- a/drivers/gpu/drm/tiny/st7586.c
+++ b/drivers/gpu/drm/tiny/st7586.c
@@ -281,7 +281,7 @@ static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = {
 	.enable		= st7586_pipe_enable,
 	.disable	= st7586_pipe_disable,
 	.update		= st7586_pipe_update,
-	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
+	.prepare_fb	= drm_gem_fb_prepare_fb,
 };
 
 static const struct drm_display_mode st7586_mode = {
diff --git a/drivers/gpu/drm/tiny/st7735r.c b/drivers/gpu/drm/tiny/st7735r.c
index 3f4487c71684..4801d56a6820 100644
--- a/drivers/gpu/drm/tiny/st7735r.c
+++ b/drivers/gpu/drm/tiny/st7735r.c
@@ -113,7 +113,7 @@ static const struct drm_simple_display_pipe_funcs jd_t18003_t01_pipe_funcs = {
 	.enable		= jd_t18003_t01_pipe_enable,
 	.disable	= mipi_dbi_pipe_disable,
 	.update		= mipi_dbi_pipe_update,
-	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
+	.prepare_fb	= drm_gem_fb_prepare_fb,
 };
 
 static const struct drm_display_mode jd_t18003_t01_mode = {
diff --git a/drivers/gpu/drm/tve200/tve200_display.c b/drivers/gpu/drm/tve200/tve200_display.c
index d733bbc4ac0e..6173c61166bb 100644
--- a/drivers/gpu/drm/tve200/tve200_display.c
+++ b/drivers/gpu/drm/tve200/tve200_display.c
@@ -297,7 +297,7 @@ static const struct drm_simple_display_pipe_funcs tve200_display_funcs = {
 	.enable = tve200_display_enable,
 	.disable = tve200_display_disable,
 	.update = tve200_display_update,
-	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
+	.prepare_fb = drm_gem_fb_prepare_fb,
 	.enable_vblank = tve200_display_enable_vblank,
 	.disable_vblank = tve200_display_disable_vblank,
 };
diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c
index 21ad1c359b61..5689bc91633a 100644
--- a/drivers/gpu/drm/xen/xen_drm_front_kms.c
+++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c
@@ -289,7 +289,7 @@ static const struct drm_simple_display_pipe_funcs display_funcs = {
 	.mode_valid = display_mode_valid,
 	.enable = display_enable,
 	.disable = display_disable,
-	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
+	.prepare_fb = drm_gem_fb_prepare_fb,
 	.update = display_update,
 };
 
diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
index d9f13fd25b0a..1b5a2ff43b4a 100644
--- a/include/drm/drm_gem_framebuffer_helper.h
+++ b/include/drm/drm_gem_framebuffer_helper.h
@@ -10,7 +10,6 @@ struct drm_gem_object;
 struct drm_mode_fb_cmd2;
 struct drm_plane;
 struct drm_plane_state;
-struct drm_simple_display_pipe;
 
 struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
 					  unsigned int plane);
@@ -31,6 +30,5 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
 
 int drm_gem_fb_prepare_fb(struct drm_plane *plane,
 			  struct drm_plane_state *state);
-int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
-					      struct drm_plane_state *plane_state);
+
 #endif
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 3f396d94afe4..4065661eb497 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -75,7 +75,7 @@ struct drm_plane_state {
 	 *
 	 * Drivers should store any implicit fence in this from their
 	 * &drm_plane_helper_funcs.prepare_fb callback. See drm_gem_fb_prepare_fb()
-	 * and drm_gem_fb_simple_display_pipe_prepare_fb() for suitable helpers.
+	 * for a suitable helper.
 	 */
 	struct dma_fence *fence;
 
diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
index 4d89cd0a60db..9d6ae8c9769f 100644
--- a/include/drm/drm_simple_kms_helper.h
+++ b/include/drm/drm_simple_kms_helper.h
@@ -114,9 +114,9 @@ struct drm_simple_display_pipe_funcs {
 	 * more details.
 	 *
 	 * Drivers which always have their buffers pinned should use
-	 * drm_gem_fb_simple_display_pipe_prepare_fb() for this hook.
+	 * drm_gem_fb_prepare_fb() for this hook.
 	 */
-	int (*prepare_fb)(struct drm_simple_display_pipe *pipe,
+	int (*prepare_fb)(struct drm_plane *plane,
 			  struct drm_plane_state *plane_state);
 
 	/**
@@ -126,7 +126,7 @@ struct drm_simple_display_pipe_funcs {
 	 * the documentation for the &drm_plane_helper_funcs.cleanup_fb hook for
 	 * more details.
 	 */
-	void (*cleanup_fb)(struct drm_simple_display_pipe *pipe,
+	void (*cleanup_fb)(struct drm_plane *plane,
 			   struct drm_plane_state *plane_state);
 
 	/**
-- 
2.23.0

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/5] drm/vram-helpers: Add helpers for struct drm_plane_helper_funcs
  2019-10-22 10:25 [PATCH 0/5] drm/vram: Provide helpers for prepare_fb() and cleanup_fb() Thomas Zimmermann
  2019-10-22 10:25 ` [PATCH 1/5] drm/simple-kms-helper: Pass plane to " Thomas Zimmermann
@ 2019-10-22 10:25 ` Thomas Zimmermann
  2019-10-22 10:25 ` [PATCH 3/5] drm/bochs: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers Thomas Zimmermann
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Thomas Zimmermann @ 2019-10-22 10:25 UTC (permalink / raw)
  To: kraxel, noralf, sam, daniel, airlied, joel, andrew,
	z.liuxinliang, zourongrong, kong.kongxinwei, puck.chen,
	linus.walleij, marex, stefan, shawnguo, s.hauer, kernel,
	festevam, linux-imx, eric, david, hdegoede,
	oleksandr_andrushchenko, liviu.dudau, ck.hu
  Cc: Thomas Zimmermann, dri-devel

The new helpers pin and unpin a framebuffer's GEM VRAM objects during
plane updates. This should be sufficient for most drivers prepare_fb and
cleanup_fb implementations.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 81 +++++++++++++++++++++++++++
 include/drm/drm_gem_vram_helper.h     | 12 ++++
 2 files changed, 93 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index dc7942981f4a..3ddb90475178 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -3,9 +3,11 @@
 #include <drm/drm_debugfs.h>
 #include <drm/drm_device.h>
 #include <drm/drm_file.h>
+#include <drm/drm_framebuffer.h>
 #include <drm/drm_gem_ttm_helper.h>
 #include <drm/drm_gem_vram_helper.h>
 #include <drm/drm_mode.h>
+#include <drm/drm_plane.h>
 #include <drm/drm_prime.h>
 #include <drm/ttm/ttm_page_alloc.h>
 
@@ -653,6 +655,85 @@ int drm_gem_vram_driver_dumb_mmap_offset(struct drm_file *file,
 }
 EXPORT_SYMBOL(drm_gem_vram_driver_dumb_mmap_offset);
 
+/*
+ * Helpers for struct drm_plane_helper_funcs
+ */
+
+/**
+ * drm_gem_vram_plane_helper_funcs_prepare_fb() - \
+ *	Implements &struct drm_plane_helper_funcs.prepare_fb
+ * @plane:	a DRM plane.
+ * @new_state:	the plane's new state
+ *
+ * During plane updates, this function pins the GEM VRAM
+ * objects of the plane's new framebuffer to VRAM. Call
+ * drm_gem_vram_plane_helper_funcs_cleanup_fb() to unpin them.
+ *
+ * Returns:
+ *	0 on success, or
+ *	a negative errno code otherwise.
+ */
+int
+drm_gem_vram_plane_helper_funcs_prepare_fb(struct drm_plane *plane,
+					   struct drm_plane_state *new_state)
+{
+	size_t i;
+	struct drm_gem_vram_object *gbo;
+	int ret;
+
+	if (!new_state->fb)
+		return 0;
+
+	for (i = 0; i < ARRAY_SIZE(new_state->fb->obj); ++i) {
+		if (!new_state->fb->obj[i])
+			continue;
+		gbo = drm_gem_vram_of_gem(new_state->fb->obj[i]);
+		ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
+		if (ret)
+			goto err_drm_gem_vram_unpin;
+	}
+
+	return 0;
+
+err_drm_gem_vram_unpin:
+	while (i) {
+		--i;
+		gbo = drm_gem_vram_of_gem(new_state->fb->obj[i]);
+		drm_gem_vram_unpin(gbo);
+	}
+	return ret;
+}
+EXPORT_SYMBOL(drm_gem_vram_plane_helper_funcs_prepare_fb);
+
+/**
+ * drm_gem_vram_plane_helper_funcs_cleanup_fb() - \
+ *	Implements &struct drm_plane_helper_funcs.cleanup_fb
+ * @plane:	a DRM plane.
+ * @old_state:	the plane's old state
+ *
+ * During plane updates, this function unpins the GEM VRAM
+ * objects of the plane's old framebuffer from VRAM. Complements
+ * drm_gem_vram_plane_helper_funcs_prepare_fb().
+ */
+void
+drm_gem_vram_plane_helper_funcs_cleanup_fb(struct drm_plane *plane,
+					   struct drm_plane_state *old_state)
+{
+	size_t i;
+	struct drm_gem_vram_object *gbo;
+
+	if (!old_state->fb)
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(old_state->fb->obj); ++i) {
+		if (!old_state->fb->obj[i])
+			continue;
+		gbo = drm_gem_vram_of_gem(old_state->fb->obj[i]);
+		drm_gem_vram_unpin(gbo);
+	}
+}
+EXPORT_SYMBOL(drm_gem_vram_plane_helper_funcs_cleanup_fb);
+
 /*
  * PRIME helpers
  */
diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
index 354a9cd358a3..19ba5c0a1fd0 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -13,6 +13,8 @@
 #include <linux/kernel.h> /* for container_of() */
 
 struct drm_mode_create_dumb;
+struct drm_plane;
+struct drm_plane_state;
 struct drm_vram_mm_funcs;
 struct filp;
 struct vm_area_struct;
@@ -124,6 +126,16 @@ int drm_gem_vram_driver_dumb_mmap_offset(struct drm_file *file,
 					 struct drm_device *dev,
 					 uint32_t handle, uint64_t *offset);
 
+/*
+ * Helpers for struct drm_plane_helper_funcs
+ */
+int
+drm_gem_vram_plane_helper_funcs_prepare_fb(struct drm_plane *plane,
+					   struct drm_plane_state *new_state);
+void
+drm_gem_vram_plane_helper_funcs_cleanup_fb(struct drm_plane *plane,
+					   struct drm_plane_state *old_state);
+
 /**
  * define DRM_GEM_VRAM_DRIVER - default callback functions for \
 	&struct drm_driver
-- 
2.23.0

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/5] drm/bochs: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers
  2019-10-22 10:25 [PATCH 0/5] drm/vram: Provide helpers for prepare_fb() and cleanup_fb() Thomas Zimmermann
  2019-10-22 10:25 ` [PATCH 1/5] drm/simple-kms-helper: Pass plane to " Thomas Zimmermann
  2019-10-22 10:25 ` [PATCH 2/5] drm/vram-helpers: Add helpers for struct drm_plane_helper_funcs Thomas Zimmermann
@ 2019-10-22 10:25 ` Thomas Zimmermann
  2019-10-22 10:25 ` [PATCH 4/5] drm/hisilicon/hibmc: Use GEM VRAM's prepare_fb() and cleanup_fb() helpers Thomas Zimmermann
  2019-10-22 10:25 ` [PATCH 5/5] drm/vboxvideo: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers Thomas Zimmermann
  4 siblings, 0 replies; 9+ messages in thread
From: Thomas Zimmermann @ 2019-10-22 10:25 UTC (permalink / raw)
  To: kraxel, noralf, sam, daniel, airlied, joel, andrew,
	z.liuxinliang, zourongrong, kong.kongxinwei, puck.chen,
	linus.walleij, marex, stefan, shawnguo, s.hauer, kernel,
	festevam, linux-imx, eric, david, hdegoede,
	oleksandr_andrushchenko, liviu.dudau, ck.hu
  Cc: Thomas Zimmermann, dri-devel

GEM VRAM provides an implementation for prepare_fb() and cleanup_fb()
of struct drm_simple_display_pipe_funcs. Switch over bochs.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/bochs/bochs_kms.c | 26 ++------------------------
 1 file changed, 2 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
index 0891640491eb..d77e17bb43ba 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -69,33 +69,11 @@ static void bochs_pipe_update(struct drm_simple_display_pipe *pipe,
 	}
 }
 
-static int bochs_pipe_prepare_fb(struct drm_plane *plane,
-				 struct drm_plane_state *new_state)
-{
-	struct drm_gem_vram_object *gbo;
-
-	if (!new_state->fb)
-		return 0;
-	gbo = drm_gem_vram_of_gem(new_state->fb->obj[0]);
-	return drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
-}
-
-static void bochs_pipe_cleanup_fb(struct drm_plane *plane,
-				  struct drm_plane_state *old_state)
-{
-	struct drm_gem_vram_object *gbo;
-
-	if (!old_state->fb)
-		return;
-	gbo = drm_gem_vram_of_gem(old_state->fb->obj[0]);
-	drm_gem_vram_unpin(gbo);
-}
-
 static const struct drm_simple_display_pipe_funcs bochs_pipe_funcs = {
 	.enable	    = bochs_pipe_enable,
 	.update	    = bochs_pipe_update,
-	.prepare_fb = bochs_pipe_prepare_fb,
-	.cleanup_fb = bochs_pipe_cleanup_fb,
+	.prepare_fb = drm_gem_vram_plane_helper_funcs_prepare_fb,
+	.cleanup_fb = drm_gem_vram_plane_helper_funcs_cleanup_fb,
 };
 
 static int bochs_connector_get_modes(struct drm_connector *connector)
-- 
2.23.0

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/5] drm/hisilicon/hibmc: Use GEM VRAM's prepare_fb() and cleanup_fb() helpers
  2019-10-22 10:25 [PATCH 0/5] drm/vram: Provide helpers for prepare_fb() and cleanup_fb() Thomas Zimmermann
                   ` (2 preceding siblings ...)
  2019-10-22 10:25 ` [PATCH 3/5] drm/bochs: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers Thomas Zimmermann
@ 2019-10-22 10:25 ` Thomas Zimmermann
  2019-10-22 10:25 ` [PATCH 5/5] drm/vboxvideo: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers Thomas Zimmermann
  4 siblings, 0 replies; 9+ messages in thread
From: Thomas Zimmermann @ 2019-10-22 10:25 UTC (permalink / raw)
  To: kraxel, noralf, sam, daniel, airlied, joel, andrew,
	z.liuxinliang, zourongrong, kong.kongxinwei, puck.chen,
	linus.walleij, marex, stefan, shawnguo, s.hauer, kernel,
	festevam, linux-imx, eric, david, hdegoede,
	oleksandr_andrushchenko, liviu.dudau, ck.hu
  Cc: Thomas Zimmermann, dri-devel

This patch implements prepare_fb() and cleanup_fb() in hibmc with the
GEM VRAM helpers. In the current code, pinning the BO is performed by
hibmc_plane_atomic_update(), where the operation does not belong.

This patch also fixes a bug where the pinned BO was never unpinned.
Pinning multiple BOs would have exhaused the available VRAM and further
pin operations would have failed, leaving the display in a corrupt
state.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
index cc4c41748cfb..6d87648df53c 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
@@ -96,7 +96,6 @@ static void hibmc_plane_atomic_update(struct drm_plane *plane,
 {
 	struct drm_plane_state	*state	= plane->state;
 	u32 reg;
-	int ret;
 	s64 gpu_addr = 0;
 	unsigned int line_l;
 	struct hibmc_drm_private *priv = plane->dev->dev_private;
@@ -109,16 +108,9 @@ static void hibmc_plane_atomic_update(struct drm_plane *plane,
 	hibmc_fb = to_hibmc_framebuffer(state->fb);
 	gbo = drm_gem_vram_of_gem(hibmc_fb->obj);
 
-	ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
-	if (ret) {
-		DRM_ERROR("failed to pin bo: %d", ret);
-		return;
-	}
 	gpu_addr = drm_gem_vram_offset(gbo);
-	if (gpu_addr < 0) {
-		drm_gem_vram_unpin(gbo);
-		return;
-	}
+	if (WARN_ON_ONCE(gpu_addr < 0))
+		return; /* Bug: we didn't pin the BO to VRAM in prepare_fb. */
 
 	writel(gpu_addr, priv->mmio + HIBMC_CRT_FB_ADDRESS);
 
@@ -157,6 +149,8 @@ static struct drm_plane_funcs hibmc_plane_funcs = {
 };
 
 static const struct drm_plane_helper_funcs hibmc_plane_helper_funcs = {
+	.prepare_fb	= drm_gem_vram_plane_helper_funcs_prepare_fb,
+	.cleanup_fb	= drm_gem_vram_plane_helper_funcs_cleanup_fb,
 	.atomic_check = hibmc_plane_atomic_check,
 	.atomic_update = hibmc_plane_atomic_update,
 };
-- 
2.23.0

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/5] drm/vboxvideo: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers
  2019-10-22 10:25 [PATCH 0/5] drm/vram: Provide helpers for prepare_fb() and cleanup_fb() Thomas Zimmermann
                   ` (3 preceding siblings ...)
  2019-10-22 10:25 ` [PATCH 4/5] drm/hisilicon/hibmc: Use GEM VRAM's prepare_fb() and cleanup_fb() helpers Thomas Zimmermann
@ 2019-10-22 10:25 ` Thomas Zimmermann
  4 siblings, 0 replies; 9+ messages in thread
From: Thomas Zimmermann @ 2019-10-22 10:25 UTC (permalink / raw)
  To: kraxel, noralf, sam, daniel, airlied, joel, andrew,
	z.liuxinliang, zourongrong, kong.kongxinwei, puck.chen,
	linus.walleij, marex, stefan, shawnguo, s.hauer, kernel,
	festevam, linux-imx, eric, david, hdegoede,
	oleksandr_andrushchenko, liviu.dudau, ck.hu
  Cc: Thomas Zimmermann, dri-devel

GEM VRAM provides an implementation for prepare_fb() and cleanup_fb()
of struct drm_plane_helper_funcs. Switch over vboxvideo.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/vboxvideo/vbox_mode.c | 61 ++-------------------------
 1 file changed, 4 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
index b5604d32122e..b93001cfa0e6 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
@@ -334,35 +334,6 @@ static void vbox_primary_atomic_disable(struct drm_plane *plane,
 				    old_state->src_y >> 16);
 }
 
-static int vbox_primary_prepare_fb(struct drm_plane *plane,
-				   struct drm_plane_state *new_state)
-{
-	struct drm_gem_vram_object *gbo;
-	int ret;
-
-	if (!new_state->fb)
-		return 0;
-
-	gbo = drm_gem_vram_of_gem(new_state->fb->obj[0]);
-	ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
-	if (ret)
-		DRM_WARN("Error %d pinning new fb, out of video mem?\n", ret);
-
-	return ret;
-}
-
-static void vbox_primary_cleanup_fb(struct drm_plane *plane,
-				    struct drm_plane_state *old_state)
-{
-	struct drm_gem_vram_object *gbo;
-
-	if (!old_state->fb)
-		return;
-
-	gbo = drm_gem_vram_of_gem(old_state->fb->obj[0]);
-	drm_gem_vram_unpin(gbo);
-}
-
 static int vbox_cursor_atomic_check(struct drm_plane *plane,
 				    struct drm_plane_state *new_state)
 {
@@ -492,30 +463,6 @@ static void vbox_cursor_atomic_disable(struct drm_plane *plane,
 	mutex_unlock(&vbox->hw_mutex);
 }
 
-static int vbox_cursor_prepare_fb(struct drm_plane *plane,
-				  struct drm_plane_state *new_state)
-{
-	struct drm_gem_vram_object *gbo;
-
-	if (!new_state->fb)
-		return 0;
-
-	gbo = drm_gem_vram_of_gem(new_state->fb->obj[0]);
-	return drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_SYSTEM);
-}
-
-static void vbox_cursor_cleanup_fb(struct drm_plane *plane,
-				   struct drm_plane_state *old_state)
-{
-	struct drm_gem_vram_object *gbo;
-
-	if (!plane->state->fb)
-		return;
-
-	gbo = drm_gem_vram_of_gem(plane->state->fb->obj[0]);
-	drm_gem_vram_unpin(gbo);
-}
-
 static const u32 vbox_cursor_plane_formats[] = {
 	DRM_FORMAT_ARGB8888,
 };
@@ -524,8 +471,8 @@ static const struct drm_plane_helper_funcs vbox_cursor_helper_funcs = {
 	.atomic_check	= vbox_cursor_atomic_check,
 	.atomic_update	= vbox_cursor_atomic_update,
 	.atomic_disable	= vbox_cursor_atomic_disable,
-	.prepare_fb	= vbox_cursor_prepare_fb,
-	.cleanup_fb	= vbox_cursor_cleanup_fb,
+	.prepare_fb	= drm_gem_vram_plane_helper_funcs_prepare_fb,
+	.cleanup_fb	= drm_gem_vram_plane_helper_funcs_cleanup_fb,
 };
 
 static const struct drm_plane_funcs vbox_cursor_plane_funcs = {
@@ -546,8 +493,8 @@ static const struct drm_plane_helper_funcs vbox_primary_helper_funcs = {
 	.atomic_check = vbox_primary_atomic_check,
 	.atomic_update = vbox_primary_atomic_update,
 	.atomic_disable = vbox_primary_atomic_disable,
-	.prepare_fb = vbox_primary_prepare_fb,
-	.cleanup_fb = vbox_primary_cleanup_fb,
+	.prepare_fb	= drm_gem_vram_plane_helper_funcs_prepare_fb,
+	.cleanup_fb	= drm_gem_vram_plane_helper_funcs_cleanup_fb,
 };
 
 static const struct drm_plane_funcs vbox_primary_plane_funcs = {
-- 
2.23.0

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] drm/simple-kms-helper: Pass plane to prepare_fb() and cleanup_fb()
  2019-10-22 10:25 ` [PATCH 1/5] drm/simple-kms-helper: Pass plane to " Thomas Zimmermann
@ 2019-10-22 14:14   ` Daniel Vetter
  2019-10-22 14:41     ` Thomas Zimmermann
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2019-10-22 14:14 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: airlied, liviu.dudau, kraxel, david, marex,
	oleksandr_andrushchenko, sam, z.liuxinliang, kong.kongxinwei,
	linux-imx, joel, puck.chen, s.hauer, hdegoede, dri-devel, andrew,
	kernel, zourongrong, shawnguo

On Tue, Oct 22, 2019 at 12:25:16PM +0200, Thomas Zimmermann wrote:
> Passing the plane structure to prepare_fb() and cleanup_fb() of
> struct drm_simple_display_pipe_funcs unifies the interface with
> struct drm_plane_helper_funcs. Implementations of these functions
> can now be shared between simple-pipeline and 'full-pipeline' drivers.
> 
> Before, the functions received the simple display pipeline's structure
> as their first argument. Apparently no implementation needed that argument
> and anyone interested can still get it from the plane via container_of().
> 
> As a side effect, drm_gem_fb_simple_display_pipe_prepare_fb() has been
> removed.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Hm ....

I see your point, but having a vfunc which takes the wrong type just feels
wrong. Imo I'd just have added a simple pipe wrapper for the vram prepare.

But this should also be fine if we tune it at bit:
- Add a note to the kerneldoc explaining why the function signature is
  "wrong".
- Add a drm_simple_display_plane_to_pipe helper for upcasting and
  reference that one in the above note.

With that I think we still have a nice&clean design here. Still leaning
towards "just add another helper to plug in".

Remaining parts of the series look really nice, but lets settle this
question here first.
-Daniel

> ---
>  drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c     |  2 +-
>  drivers/gpu/drm/bochs/bochs_kms.c            |  4 ++--
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 22 --------------------
>  drivers/gpu/drm/drm_simple_kms_helper.c      |  4 ++--
>  drivers/gpu/drm/mcde/mcde_display.c          |  2 +-
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c            |  2 +-
>  drivers/gpu/drm/pl111/pl111_display.c        |  2 +-
>  drivers/gpu/drm/tiny/hx8357d.c               |  2 +-
>  drivers/gpu/drm/tiny/ili9225.c               |  2 +-
>  drivers/gpu/drm/tiny/ili9341.c               |  2 +-
>  drivers/gpu/drm/tiny/mi0283qt.c              |  2 +-
>  drivers/gpu/drm/tiny/repaper.c               |  2 +-
>  drivers/gpu/drm/tiny/st7586.c                |  2 +-
>  drivers/gpu/drm/tiny/st7735r.c               |  2 +-
>  drivers/gpu/drm/tve200/tve200_display.c      |  2 +-
>  drivers/gpu/drm/xen/xen_drm_front_kms.c      |  2 +-
>  include/drm/drm_gem_framebuffer_helper.h     |  4 +---
>  include/drm/drm_plane.h                      |  2 +-
>  include/drm/drm_simple_kms_helper.h          |  6 +++---
>  19 files changed, 22 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
> index 2184b8be6fd4..0fe72f46f397 100644
> --- a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
> @@ -219,7 +219,7 @@ static const struct drm_simple_display_pipe_funcs aspeed_gfx_funcs = {
>  	.enable		= aspeed_gfx_pipe_enable,
>  	.disable	= aspeed_gfx_pipe_disable,
>  	.update		= aspeed_gfx_pipe_update,
> -	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
> +	.prepare_fb	= drm_gem_fb_prepare_fb,
>  	.enable_vblank	= aspeed_gfx_enable_vblank,
>  	.disable_vblank	= aspeed_gfx_disable_vblank,
>  };
> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
> index 02a9c1ed165b..0891640491eb 100644
> --- a/drivers/gpu/drm/bochs/bochs_kms.c
> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> @@ -69,7 +69,7 @@ static void bochs_pipe_update(struct drm_simple_display_pipe *pipe,
>  	}
>  }
>  
> -static int bochs_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
> +static int bochs_pipe_prepare_fb(struct drm_plane *plane,
>  				 struct drm_plane_state *new_state)
>  {
>  	struct drm_gem_vram_object *gbo;
> @@ -80,7 +80,7 @@ static int bochs_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
>  	return drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
>  }
>  
> -static void bochs_pipe_cleanup_fb(struct drm_simple_display_pipe *pipe,
> +static void bochs_pipe_cleanup_fb(struct drm_plane *plane,
>  				  struct drm_plane_state *old_state)
>  {
>  	struct drm_gem_vram_object *gbo;
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index b9bcd310ca2d..c7ea288bef0a 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -300,25 +300,3 @@ int drm_gem_fb_prepare_fb(struct drm_plane *plane,
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_prepare_fb);
> -
> -/**
> - * drm_gem_fb_simple_display_pipe_prepare_fb - prepare_fb helper for
> - *     &drm_simple_display_pipe
> - * @pipe: Simple display pipe
> - * @plane_state: Plane state
> - *
> - * This function uses drm_gem_fb_prepare_fb() to extract the exclusive fence
> - * from &drm_gem_object.resv and attaches it to plane state for the atomic
> - * helper to wait on. This is necessary to correctly implement implicit
> - * synchronization for any buffers shared as a struct &dma_buf. Drivers can use
> - * this as their &drm_simple_display_pipe_funcs.prepare_fb callback.
> - *
> - * See drm_atomic_set_fence_for_plane() for a discussion of implicit and
> - * explicit fencing in atomic modeset updates.
> - */
> -int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
> -					      struct drm_plane_state *plane_state)
> -{
> -	return drm_gem_fb_prepare_fb(&pipe->plane, plane_state);
> -}
> -EXPORT_SYMBOL(drm_gem_fb_simple_display_pipe_prepare_fb);
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> index 046055719245..e51f336e67f0 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -173,7 +173,7 @@ static int drm_simple_kms_plane_prepare_fb(struct drm_plane *plane,
>  	if (!pipe->funcs || !pipe->funcs->prepare_fb)
>  		return 0;
>  
> -	return pipe->funcs->prepare_fb(pipe, state);
> +	return pipe->funcs->prepare_fb(&pipe->plane, state);
>  }
>  
>  static void drm_simple_kms_plane_cleanup_fb(struct drm_plane *plane,
> @@ -185,7 +185,7 @@ static void drm_simple_kms_plane_cleanup_fb(struct drm_plane *plane,
>  	if (!pipe->funcs || !pipe->funcs->cleanup_fb)
>  		return;
>  
> -	pipe->funcs->cleanup_fb(pipe, state);
> +	pipe->funcs->cleanup_fb(&pipe->plane, state);
>  }
>  
>  static bool drm_simple_kms_format_mod_supported(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/mcde/mcde_display.c b/drivers/gpu/drm/mcde/mcde_display.c
> index 751454ae3cd1..31efbdcced5d 100644
> --- a/drivers/gpu/drm/mcde/mcde_display.c
> +++ b/drivers/gpu/drm/mcde/mcde_display.c
> @@ -1097,7 +1097,7 @@ static struct drm_simple_display_pipe_funcs mcde_display_funcs = {
>  	.enable = mcde_display_enable,
>  	.disable = mcde_display_disable,
>  	.update = mcde_display_update,
> -	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> +	.prepare_fb = drm_gem_fb_prepare_fb,
>  };
>  
>  int mcde_display_init(struct drm_device *drm)
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index 497cf443a9af..547388b32c64 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -186,7 +186,7 @@ static struct drm_simple_display_pipe_funcs mxsfb_funcs = {
>  	.enable		= mxsfb_pipe_enable,
>  	.disable	= mxsfb_pipe_disable,
>  	.update		= mxsfb_pipe_update,
> -	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
> +	.prepare_fb	= drm_gem_fb_prepare_fb,
>  	.enable_vblank	= mxsfb_pipe_enable_vblank,
>  	.disable_vblank	= mxsfb_pipe_disable_vblank,
>  };
> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index 024771a4083e..e564c838a081 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -441,7 +441,7 @@ static struct drm_simple_display_pipe_funcs pl111_display_funcs = {
>  	.enable = pl111_display_enable,
>  	.disable = pl111_display_disable,
>  	.update = pl111_display_update,
> -	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> +	.prepare_fb = drm_gem_fb_prepare_fb,
>  };
>  
>  static int pl111_clk_div_choose_div(struct clk_hw *hw, unsigned long rate,
> diff --git a/drivers/gpu/drm/tiny/hx8357d.c b/drivers/gpu/drm/tiny/hx8357d.c
> index 9af8ff84974f..235223190c04 100644
> --- a/drivers/gpu/drm/tiny/hx8357d.c
> +++ b/drivers/gpu/drm/tiny/hx8357d.c
> @@ -183,7 +183,7 @@ static const struct drm_simple_display_pipe_funcs hx8357d_pipe_funcs = {
>  	.enable = yx240qv29_enable,
>  	.disable = mipi_dbi_pipe_disable,
>  	.update = mipi_dbi_pipe_update,
> -	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> +	.prepare_fb = drm_gem_fb_prepare_fb,
>  };
>  
>  static const struct drm_display_mode yx350hv15_mode = {
> diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
> index c66acc566c2b..39b0d425359a 100644
> --- a/drivers/gpu/drm/tiny/ili9225.c
> +++ b/drivers/gpu/drm/tiny/ili9225.c
> @@ -342,7 +342,7 @@ static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = {
>  	.enable		= ili9225_pipe_enable,
>  	.disable	= ili9225_pipe_disable,
>  	.update		= ili9225_pipe_update,
> -	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
> +	.prepare_fb	= drm_gem_fb_prepare_fb,
>  };
>  
>  static const struct drm_display_mode ili9225_mode = {
> diff --git a/drivers/gpu/drm/tiny/ili9341.c b/drivers/gpu/drm/tiny/ili9341.c
> index 33b51dc7faa8..d107b6172bbb 100644
> --- a/drivers/gpu/drm/tiny/ili9341.c
> +++ b/drivers/gpu/drm/tiny/ili9341.c
> @@ -139,7 +139,7 @@ static const struct drm_simple_display_pipe_funcs ili9341_pipe_funcs = {
>  	.enable = yx240qv29_enable,
>  	.disable = mipi_dbi_pipe_disable,
>  	.update = mipi_dbi_pipe_update,
> -	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> +	.prepare_fb = drm_gem_fb_prepare_fb,
>  };
>  
>  static const struct drm_display_mode yx240qv29_mode = {
> diff --git a/drivers/gpu/drm/tiny/mi0283qt.c b/drivers/gpu/drm/tiny/mi0283qt.c
> index e2cfd9a17143..811143a17c77 100644
> --- a/drivers/gpu/drm/tiny/mi0283qt.c
> +++ b/drivers/gpu/drm/tiny/mi0283qt.c
> @@ -143,7 +143,7 @@ static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = {
>  	.enable = mi0283qt_enable,
>  	.disable = mipi_dbi_pipe_disable,
>  	.update = mipi_dbi_pipe_update,
> -	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> +	.prepare_fb = drm_gem_fb_prepare_fb,
>  };
>  
>  static const struct drm_display_mode mi0283qt_mode = {
> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
> index 76d179200775..9228d813d3d2 100644
> --- a/drivers/gpu/drm/tiny/repaper.c
> +++ b/drivers/gpu/drm/tiny/repaper.c
> @@ -874,7 +874,7 @@ static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = {
>  	.enable = repaper_pipe_enable,
>  	.disable = repaper_pipe_disable,
>  	.update = repaper_pipe_update,
> -	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> +	.prepare_fb = drm_gem_fb_prepare_fb,
>  };
>  
>  static int repaper_connector_get_modes(struct drm_connector *connector)
> diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
> index 3cc21a1b30c8..7b837c46bbfc 100644
> --- a/drivers/gpu/drm/tiny/st7586.c
> +++ b/drivers/gpu/drm/tiny/st7586.c
> @@ -281,7 +281,7 @@ static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = {
>  	.enable		= st7586_pipe_enable,
>  	.disable	= st7586_pipe_disable,
>  	.update		= st7586_pipe_update,
> -	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
> +	.prepare_fb	= drm_gem_fb_prepare_fb,
>  };
>  
>  static const struct drm_display_mode st7586_mode = {
> diff --git a/drivers/gpu/drm/tiny/st7735r.c b/drivers/gpu/drm/tiny/st7735r.c
> index 3f4487c71684..4801d56a6820 100644
> --- a/drivers/gpu/drm/tiny/st7735r.c
> +++ b/drivers/gpu/drm/tiny/st7735r.c
> @@ -113,7 +113,7 @@ static const struct drm_simple_display_pipe_funcs jd_t18003_t01_pipe_funcs = {
>  	.enable		= jd_t18003_t01_pipe_enable,
>  	.disable	= mipi_dbi_pipe_disable,
>  	.update		= mipi_dbi_pipe_update,
> -	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
> +	.prepare_fb	= drm_gem_fb_prepare_fb,
>  };
>  
>  static const struct drm_display_mode jd_t18003_t01_mode = {
> diff --git a/drivers/gpu/drm/tve200/tve200_display.c b/drivers/gpu/drm/tve200/tve200_display.c
> index d733bbc4ac0e..6173c61166bb 100644
> --- a/drivers/gpu/drm/tve200/tve200_display.c
> +++ b/drivers/gpu/drm/tve200/tve200_display.c
> @@ -297,7 +297,7 @@ static const struct drm_simple_display_pipe_funcs tve200_display_funcs = {
>  	.enable = tve200_display_enable,
>  	.disable = tve200_display_disable,
>  	.update = tve200_display_update,
> -	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> +	.prepare_fb = drm_gem_fb_prepare_fb,
>  	.enable_vblank = tve200_display_enable_vblank,
>  	.disable_vblank = tve200_display_disable_vblank,
>  };
> diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c
> index 21ad1c359b61..5689bc91633a 100644
> --- a/drivers/gpu/drm/xen/xen_drm_front_kms.c
> +++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c
> @@ -289,7 +289,7 @@ static const struct drm_simple_display_pipe_funcs display_funcs = {
>  	.mode_valid = display_mode_valid,
>  	.enable = display_enable,
>  	.disable = display_disable,
> -	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> +	.prepare_fb = drm_gem_fb_prepare_fb,
>  	.update = display_update,
>  };
>  
> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> index d9f13fd25b0a..1b5a2ff43b4a 100644
> --- a/include/drm/drm_gem_framebuffer_helper.h
> +++ b/include/drm/drm_gem_framebuffer_helper.h
> @@ -10,7 +10,6 @@ struct drm_gem_object;
>  struct drm_mode_fb_cmd2;
>  struct drm_plane;
>  struct drm_plane_state;
> -struct drm_simple_display_pipe;
>  
>  struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
>  					  unsigned int plane);
> @@ -31,6 +30,5 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>  
>  int drm_gem_fb_prepare_fb(struct drm_plane *plane,
>  			  struct drm_plane_state *state);
> -int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
> -					      struct drm_plane_state *plane_state);
> +
>  #endif
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 3f396d94afe4..4065661eb497 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -75,7 +75,7 @@ struct drm_plane_state {
>  	 *
>  	 * Drivers should store any implicit fence in this from their
>  	 * &drm_plane_helper_funcs.prepare_fb callback. See drm_gem_fb_prepare_fb()
> -	 * and drm_gem_fb_simple_display_pipe_prepare_fb() for suitable helpers.
> +	 * for a suitable helper.
>  	 */
>  	struct dma_fence *fence;
>  
> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
> index 4d89cd0a60db..9d6ae8c9769f 100644
> --- a/include/drm/drm_simple_kms_helper.h
> +++ b/include/drm/drm_simple_kms_helper.h
> @@ -114,9 +114,9 @@ struct drm_simple_display_pipe_funcs {
>  	 * more details.
>  	 *
>  	 * Drivers which always have their buffers pinned should use
> -	 * drm_gem_fb_simple_display_pipe_prepare_fb() for this hook.
> +	 * drm_gem_fb_prepare_fb() for this hook.
>  	 */
> -	int (*prepare_fb)(struct drm_simple_display_pipe *pipe,
> +	int (*prepare_fb)(struct drm_plane *plane,
>  			  struct drm_plane_state *plane_state);
>  
>  	/**
> @@ -126,7 +126,7 @@ struct drm_simple_display_pipe_funcs {
>  	 * the documentation for the &drm_plane_helper_funcs.cleanup_fb hook for
>  	 * more details.
>  	 */
> -	void (*cleanup_fb)(struct drm_simple_display_pipe *pipe,
> +	void (*cleanup_fb)(struct drm_plane *plane,
>  			   struct drm_plane_state *plane_state);
>  
>  	/**
> -- 
> 2.23.0
> 

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] drm/simple-kms-helper: Pass plane to prepare_fb() and cleanup_fb()
  2019-10-22 14:14   ` Daniel Vetter
@ 2019-10-22 14:41     ` Thomas Zimmermann
  2019-10-22 15:48       ` Daniel Vetter
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Zimmermann @ 2019-10-22 14:41 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: airlied, liviu.dudau, kraxel, marex, oleksandr_andrushchenko,
	sam, z.liuxinliang, kong.kongxinwei, linux-imx, joel, david,
	puck.chen, s.hauer, hdegoede, dri-devel, andrew, kernel,
	zourongrong, shawnguo

Hi

Am 22.10.19 um 16:14 schrieb Daniel Vetter:
> On Tue, Oct 22, 2019 at 12:25:16PM +0200, Thomas Zimmermann wrote:
>> Passing the plane structure to prepare_fb() and cleanup_fb() of
>> struct drm_simple_display_pipe_funcs unifies the interface with
>> struct drm_plane_helper_funcs. Implementations of these functions
>> can now be shared between simple-pipeline and 'full-pipeline' drivers.
>>
>> Before, the functions received the simple display pipeline's structure
>> as their first argument. Apparently no implementation needed that argument
>> and anyone interested can still get it from the plane via container_of().
>>
>> As a side effect, drm_gem_fb_simple_display_pipe_prepare_fb() has been
>> removed.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> Hm ....
> 
> I see your point, but having a vfunc which takes the wrong type just feels
> wrong. Imo I'd just have added a simple pipe wrapper for the vram prepare.

There's already mode_valid(), [1] which uses a similar exception. With
existing 'prior art,' I didn't see the point of adding additional
wrappers. Removing an existing wrapper function looked tempting, however. :)

>
> But this should also be fine if we tune it at bit:
> - Add a note to the kerneldoc explaining why the function signature is
>   "wrong".
> - Add a drm_simple_display_plane_to_pipe helper for upcasting and
>   reference that one in the above note.

Sure, no problem.

Best regards
Thomas

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/drm/drm_simple_kms_helper.h#n52

> With that I think we still have a nice&clean design here. Still leaning
> towards "just add another helper to plug in".
> 
> Remaining parts of the series look really nice, but lets settle this
> question here first.
> -Daniel
> 
>> ---
>>  drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c     |  2 +-
>>  drivers/gpu/drm/bochs/bochs_kms.c            |  4 ++--
>>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 22 --------------------
>>  drivers/gpu/drm/drm_simple_kms_helper.c      |  4 ++--
>>  drivers/gpu/drm/mcde/mcde_display.c          |  2 +-
>>  drivers/gpu/drm/mxsfb/mxsfb_drv.c            |  2 +-
>>  drivers/gpu/drm/pl111/pl111_display.c        |  2 +-
>>  drivers/gpu/drm/tiny/hx8357d.c               |  2 +-
>>  drivers/gpu/drm/tiny/ili9225.c               |  2 +-
>>  drivers/gpu/drm/tiny/ili9341.c               |  2 +-
>>  drivers/gpu/drm/tiny/mi0283qt.c              |  2 +-
>>  drivers/gpu/drm/tiny/repaper.c               |  2 +-
>>  drivers/gpu/drm/tiny/st7586.c                |  2 +-
>>  drivers/gpu/drm/tiny/st7735r.c               |  2 +-
>>  drivers/gpu/drm/tve200/tve200_display.c      |  2 +-
>>  drivers/gpu/drm/xen/xen_drm_front_kms.c      |  2 +-
>>  include/drm/drm_gem_framebuffer_helper.h     |  4 +---
>>  include/drm/drm_plane.h                      |  2 +-
>>  include/drm/drm_simple_kms_helper.h          |  6 +++---
>>  19 files changed, 22 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
>> index 2184b8be6fd4..0fe72f46f397 100644
>> --- a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
>> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
>> @@ -219,7 +219,7 @@ static const struct drm_simple_display_pipe_funcs aspeed_gfx_funcs = {
>>  	.enable		= aspeed_gfx_pipe_enable,
>>  	.disable	= aspeed_gfx_pipe_disable,
>>  	.update		= aspeed_gfx_pipe_update,
>> -	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
>> +	.prepare_fb	= drm_gem_fb_prepare_fb,
>>  	.enable_vblank	= aspeed_gfx_enable_vblank,
>>  	.disable_vblank	= aspeed_gfx_disable_vblank,
>>  };
>> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
>> index 02a9c1ed165b..0891640491eb 100644
>> --- a/drivers/gpu/drm/bochs/bochs_kms.c
>> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
>> @@ -69,7 +69,7 @@ static void bochs_pipe_update(struct drm_simple_display_pipe *pipe,
>>  	}
>>  }
>>  
>> -static int bochs_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
>> +static int bochs_pipe_prepare_fb(struct drm_plane *plane,
>>  				 struct drm_plane_state *new_state)
>>  {
>>  	struct drm_gem_vram_object *gbo;
>> @@ -80,7 +80,7 @@ static int bochs_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
>>  	return drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
>>  }
>>  
>> -static void bochs_pipe_cleanup_fb(struct drm_simple_display_pipe *pipe,
>> +static void bochs_pipe_cleanup_fb(struct drm_plane *plane,
>>  				  struct drm_plane_state *old_state)
>>  {
>>  	struct drm_gem_vram_object *gbo;
>> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>> index b9bcd310ca2d..c7ea288bef0a 100644
>> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>> @@ -300,25 +300,3 @@ int drm_gem_fb_prepare_fb(struct drm_plane *plane,
>>  	return 0;
>>  }
>>  EXPORT_SYMBOL_GPL(drm_gem_fb_prepare_fb);
>> -
>> -/**
>> - * drm_gem_fb_simple_display_pipe_prepare_fb - prepare_fb helper for
>> - *     &drm_simple_display_pipe
>> - * @pipe: Simple display pipe
>> - * @plane_state: Plane state
>> - *
>> - * This function uses drm_gem_fb_prepare_fb() to extract the exclusive fence
>> - * from &drm_gem_object.resv and attaches it to plane state for the atomic
>> - * helper to wait on. This is necessary to correctly implement implicit
>> - * synchronization for any buffers shared as a struct &dma_buf. Drivers can use
>> - * this as their &drm_simple_display_pipe_funcs.prepare_fb callback.
>> - *
>> - * See drm_atomic_set_fence_for_plane() for a discussion of implicit and
>> - * explicit fencing in atomic modeset updates.
>> - */
>> -int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
>> -					      struct drm_plane_state *plane_state)
>> -{
>> -	return drm_gem_fb_prepare_fb(&pipe->plane, plane_state);
>> -}
>> -EXPORT_SYMBOL(drm_gem_fb_simple_display_pipe_prepare_fb);
>> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
>> index 046055719245..e51f336e67f0 100644
>> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
>> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
>> @@ -173,7 +173,7 @@ static int drm_simple_kms_plane_prepare_fb(struct drm_plane *plane,
>>  	if (!pipe->funcs || !pipe->funcs->prepare_fb)
>>  		return 0;
>>  
>> -	return pipe->funcs->prepare_fb(pipe, state);
>> +	return pipe->funcs->prepare_fb(&pipe->plane, state);
>>  }
>>  
>>  static void drm_simple_kms_plane_cleanup_fb(struct drm_plane *plane,
>> @@ -185,7 +185,7 @@ static void drm_simple_kms_plane_cleanup_fb(struct drm_plane *plane,
>>  	if (!pipe->funcs || !pipe->funcs->cleanup_fb)
>>  		return;
>>  
>> -	pipe->funcs->cleanup_fb(pipe, state);
>> +	pipe->funcs->cleanup_fb(&pipe->plane, state);
>>  }
>>  
>>  static bool drm_simple_kms_format_mod_supported(struct drm_plane *plane,
>> diff --git a/drivers/gpu/drm/mcde/mcde_display.c b/drivers/gpu/drm/mcde/mcde_display.c
>> index 751454ae3cd1..31efbdcced5d 100644
>> --- a/drivers/gpu/drm/mcde/mcde_display.c
>> +++ b/drivers/gpu/drm/mcde/mcde_display.c
>> @@ -1097,7 +1097,7 @@ static struct drm_simple_display_pipe_funcs mcde_display_funcs = {
>>  	.enable = mcde_display_enable,
>>  	.disable = mcde_display_disable,
>>  	.update = mcde_display_update,
>> -	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
>> +	.prepare_fb = drm_gem_fb_prepare_fb,
>>  };
>>  
>>  int mcde_display_init(struct drm_device *drm)
>> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
>> index 497cf443a9af..547388b32c64 100644
>> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
>> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
>> @@ -186,7 +186,7 @@ static struct drm_simple_display_pipe_funcs mxsfb_funcs = {
>>  	.enable		= mxsfb_pipe_enable,
>>  	.disable	= mxsfb_pipe_disable,
>>  	.update		= mxsfb_pipe_update,
>> -	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
>> +	.prepare_fb	= drm_gem_fb_prepare_fb,
>>  	.enable_vblank	= mxsfb_pipe_enable_vblank,
>>  	.disable_vblank	= mxsfb_pipe_disable_vblank,
>>  };
>> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
>> index 024771a4083e..e564c838a081 100644
>> --- a/drivers/gpu/drm/pl111/pl111_display.c
>> +++ b/drivers/gpu/drm/pl111/pl111_display.c
>> @@ -441,7 +441,7 @@ static struct drm_simple_display_pipe_funcs pl111_display_funcs = {
>>  	.enable = pl111_display_enable,
>>  	.disable = pl111_display_disable,
>>  	.update = pl111_display_update,
>> -	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
>> +	.prepare_fb = drm_gem_fb_prepare_fb,
>>  };
>>  
>>  static int pl111_clk_div_choose_div(struct clk_hw *hw, unsigned long rate,
>> diff --git a/drivers/gpu/drm/tiny/hx8357d.c b/drivers/gpu/drm/tiny/hx8357d.c
>> index 9af8ff84974f..235223190c04 100644
>> --- a/drivers/gpu/drm/tiny/hx8357d.c
>> +++ b/drivers/gpu/drm/tiny/hx8357d.c
>> @@ -183,7 +183,7 @@ static const struct drm_simple_display_pipe_funcs hx8357d_pipe_funcs = {
>>  	.enable = yx240qv29_enable,
>>  	.disable = mipi_dbi_pipe_disable,
>>  	.update = mipi_dbi_pipe_update,
>> -	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
>> +	.prepare_fb = drm_gem_fb_prepare_fb,
>>  };
>>  
>>  static const struct drm_display_mode yx350hv15_mode = {
>> diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
>> index c66acc566c2b..39b0d425359a 100644
>> --- a/drivers/gpu/drm/tiny/ili9225.c
>> +++ b/drivers/gpu/drm/tiny/ili9225.c
>> @@ -342,7 +342,7 @@ static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = {
>>  	.enable		= ili9225_pipe_enable,
>>  	.disable	= ili9225_pipe_disable,
>>  	.update		= ili9225_pipe_update,
>> -	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
>> +	.prepare_fb	= drm_gem_fb_prepare_fb,
>>  };
>>  
>>  static const struct drm_display_mode ili9225_mode = {
>> diff --git a/drivers/gpu/drm/tiny/ili9341.c b/drivers/gpu/drm/tiny/ili9341.c
>> index 33b51dc7faa8..d107b6172bbb 100644
>> --- a/drivers/gpu/drm/tiny/ili9341.c
>> +++ b/drivers/gpu/drm/tiny/ili9341.c
>> @@ -139,7 +139,7 @@ static const struct drm_simple_display_pipe_funcs ili9341_pipe_funcs = {
>>  	.enable = yx240qv29_enable,
>>  	.disable = mipi_dbi_pipe_disable,
>>  	.update = mipi_dbi_pipe_update,
>> -	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
>> +	.prepare_fb = drm_gem_fb_prepare_fb,
>>  };
>>  
>>  static const struct drm_display_mode yx240qv29_mode = {
>> diff --git a/drivers/gpu/drm/tiny/mi0283qt.c b/drivers/gpu/drm/tiny/mi0283qt.c
>> index e2cfd9a17143..811143a17c77 100644
>> --- a/drivers/gpu/drm/tiny/mi0283qt.c
>> +++ b/drivers/gpu/drm/tiny/mi0283qt.c
>> @@ -143,7 +143,7 @@ static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = {
>>  	.enable = mi0283qt_enable,
>>  	.disable = mipi_dbi_pipe_disable,
>>  	.update = mipi_dbi_pipe_update,
>> -	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
>> +	.prepare_fb = drm_gem_fb_prepare_fb,
>>  };
>>  
>>  static const struct drm_display_mode mi0283qt_mode = {
>> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
>> index 76d179200775..9228d813d3d2 100644
>> --- a/drivers/gpu/drm/tiny/repaper.c
>> +++ b/drivers/gpu/drm/tiny/repaper.c
>> @@ -874,7 +874,7 @@ static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = {
>>  	.enable = repaper_pipe_enable,
>>  	.disable = repaper_pipe_disable,
>>  	.update = repaper_pipe_update,
>> -	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
>> +	.prepare_fb = drm_gem_fb_prepare_fb,
>>  };
>>  
>>  static int repaper_connector_get_modes(struct drm_connector *connector)
>> diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
>> index 3cc21a1b30c8..7b837c46bbfc 100644
>> --- a/drivers/gpu/drm/tiny/st7586.c
>> +++ b/drivers/gpu/drm/tiny/st7586.c
>> @@ -281,7 +281,7 @@ static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = {
>>  	.enable		= st7586_pipe_enable,
>>  	.disable	= st7586_pipe_disable,
>>  	.update		= st7586_pipe_update,
>> -	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
>> +	.prepare_fb	= drm_gem_fb_prepare_fb,
>>  };
>>  
>>  static const struct drm_display_mode st7586_mode = {
>> diff --git a/drivers/gpu/drm/tiny/st7735r.c b/drivers/gpu/drm/tiny/st7735r.c
>> index 3f4487c71684..4801d56a6820 100644
>> --- a/drivers/gpu/drm/tiny/st7735r.c
>> +++ b/drivers/gpu/drm/tiny/st7735r.c
>> @@ -113,7 +113,7 @@ static const struct drm_simple_display_pipe_funcs jd_t18003_t01_pipe_funcs = {
>>  	.enable		= jd_t18003_t01_pipe_enable,
>>  	.disable	= mipi_dbi_pipe_disable,
>>  	.update		= mipi_dbi_pipe_update,
>> -	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
>> +	.prepare_fb	= drm_gem_fb_prepare_fb,
>>  };
>>  
>>  static const struct drm_display_mode jd_t18003_t01_mode = {
>> diff --git a/drivers/gpu/drm/tve200/tve200_display.c b/drivers/gpu/drm/tve200/tve200_display.c
>> index d733bbc4ac0e..6173c61166bb 100644
>> --- a/drivers/gpu/drm/tve200/tve200_display.c
>> +++ b/drivers/gpu/drm/tve200/tve200_display.c
>> @@ -297,7 +297,7 @@ static const struct drm_simple_display_pipe_funcs tve200_display_funcs = {
>>  	.enable = tve200_display_enable,
>>  	.disable = tve200_display_disable,
>>  	.update = tve200_display_update,
>> -	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
>> +	.prepare_fb = drm_gem_fb_prepare_fb,
>>  	.enable_vblank = tve200_display_enable_vblank,
>>  	.disable_vblank = tve200_display_disable_vblank,
>>  };
>> diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c
>> index 21ad1c359b61..5689bc91633a 100644
>> --- a/drivers/gpu/drm/xen/xen_drm_front_kms.c
>> +++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c
>> @@ -289,7 +289,7 @@ static const struct drm_simple_display_pipe_funcs display_funcs = {
>>  	.mode_valid = display_mode_valid,
>>  	.enable = display_enable,
>>  	.disable = display_disable,
>> -	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
>> +	.prepare_fb = drm_gem_fb_prepare_fb,
>>  	.update = display_update,
>>  };
>>  
>> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
>> index d9f13fd25b0a..1b5a2ff43b4a 100644
>> --- a/include/drm/drm_gem_framebuffer_helper.h
>> +++ b/include/drm/drm_gem_framebuffer_helper.h
>> @@ -10,7 +10,6 @@ struct drm_gem_object;
>>  struct drm_mode_fb_cmd2;
>>  struct drm_plane;
>>  struct drm_plane_state;
>> -struct drm_simple_display_pipe;
>>  
>>  struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
>>  					  unsigned int plane);
>> @@ -31,6 +30,5 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>>  
>>  int drm_gem_fb_prepare_fb(struct drm_plane *plane,
>>  			  struct drm_plane_state *state);
>> -int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
>> -					      struct drm_plane_state *plane_state);
>> +
>>  #endif
>> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
>> index 3f396d94afe4..4065661eb497 100644
>> --- a/include/drm/drm_plane.h
>> +++ b/include/drm/drm_plane.h
>> @@ -75,7 +75,7 @@ struct drm_plane_state {
>>  	 *
>>  	 * Drivers should store any implicit fence in this from their
>>  	 * &drm_plane_helper_funcs.prepare_fb callback. See drm_gem_fb_prepare_fb()
>> -	 * and drm_gem_fb_simple_display_pipe_prepare_fb() for suitable helpers.
>> +	 * for a suitable helper.
>>  	 */
>>  	struct dma_fence *fence;
>>  
>> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
>> index 4d89cd0a60db..9d6ae8c9769f 100644
>> --- a/include/drm/drm_simple_kms_helper.h
>> +++ b/include/drm/drm_simple_kms_helper.h
>> @@ -114,9 +114,9 @@ struct drm_simple_display_pipe_funcs {
>>  	 * more details.
>>  	 *
>>  	 * Drivers which always have their buffers pinned should use
>> -	 * drm_gem_fb_simple_display_pipe_prepare_fb() for this hook.
>> +	 * drm_gem_fb_prepare_fb() for this hook.
>>  	 */
>> -	int (*prepare_fb)(struct drm_simple_display_pipe *pipe,
>> +	int (*prepare_fb)(struct drm_plane *plane,
>>  			  struct drm_plane_state *plane_state);
>>  
>>  	/**
>> @@ -126,7 +126,7 @@ struct drm_simple_display_pipe_funcs {
>>  	 * the documentation for the &drm_plane_helper_funcs.cleanup_fb hook for
>>  	 * more details.
>>  	 */
>> -	void (*cleanup_fb)(struct drm_simple_display_pipe *pipe,
>> +	void (*cleanup_fb)(struct drm_plane *plane,
>>  			   struct drm_plane_state *plane_state);
>>  
>>  	/**
>> -- 
>> 2.23.0
>>
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] drm/simple-kms-helper: Pass plane to prepare_fb() and cleanup_fb()
  2019-10-22 14:41     ` Thomas Zimmermann
@ 2019-10-22 15:48       ` Daniel Vetter
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2019-10-22 15:48 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Dave Airlie, Liviu Dudau, Gerd Hoffmann, Marek Vasut,
	Oleksandr Andrushchenko, Sam Ravnborg, Xinliang Liu, Xinwei Kong,
	dl-linux-imx, Joel Stanley, David Lechner, Feng Chen,
	Sascha Hauer, Hans de Goede, dri-devel, Andrew Jeffery,
	Sascha Hauer, Rongrong Zou, Shawn Guo

On Tue, Oct 22, 2019 at 4:41 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 22.10.19 um 16:14 schrieb Daniel Vetter:
> > On Tue, Oct 22, 2019 at 12:25:16PM +0200, Thomas Zimmermann wrote:
> >> Passing the plane structure to prepare_fb() and cleanup_fb() of
> >> struct drm_simple_display_pipe_funcs unifies the interface with
> >> struct drm_plane_helper_funcs. Implementations of these functions
> >> can now be shared between simple-pipeline and 'full-pipeline' drivers.
> >>
> >> Before, the functions received the simple display pipeline's structure
> >> as their first argument. Apparently no implementation needed that argument
> >> and anyone interested can still get it from the plane via container_of().
> >>
> >> As a side effect, drm_gem_fb_simple_display_pipe_prepare_fb() has been
> >> removed.
> >>
> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> >
> > Hm ....
> >
> > I see your point, but having a vfunc which takes the wrong type just feels
> > wrong. Imo I'd just have added a simple pipe wrapper for the vram prepare.
>
> There's already mode_valid(), [1] which uses a similar exception. With
> existing 'prior art,' I didn't see the point of adding additional
> wrappers. Removing an existing wrapper function looked tempting, however. :)

Huh I totally missed that one. I guess that should get bikeshedded,
I'll type a patch.
-Daniel

> > But this should also be fine if we tune it at bit:
> > - Add a note to the kerneldoc explaining why the function signature is
> >   "wrong".
> > - Add a drm_simple_display_plane_to_pipe helper for upcasting and
> >   reference that one in the above note.
>
> Sure, no problem.
>
> Best regards
> Thomas
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/drm/drm_simple_kms_helper.h#n52
>
> > With that I think we still have a nice&clean design here. Still leaning
> > towards "just add another helper to plug in".
> >
> > Remaining parts of the series look really nice, but lets settle this
> > question here first.
> > -Daniel
> >
> >> ---
> >>  drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c     |  2 +-
> >>  drivers/gpu/drm/bochs/bochs_kms.c            |  4 ++--
> >>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 22 --------------------
> >>  drivers/gpu/drm/drm_simple_kms_helper.c      |  4 ++--
> >>  drivers/gpu/drm/mcde/mcde_display.c          |  2 +-
> >>  drivers/gpu/drm/mxsfb/mxsfb_drv.c            |  2 +-
> >>  drivers/gpu/drm/pl111/pl111_display.c        |  2 +-
> >>  drivers/gpu/drm/tiny/hx8357d.c               |  2 +-
> >>  drivers/gpu/drm/tiny/ili9225.c               |  2 +-
> >>  drivers/gpu/drm/tiny/ili9341.c               |  2 +-
> >>  drivers/gpu/drm/tiny/mi0283qt.c              |  2 +-
> >>  drivers/gpu/drm/tiny/repaper.c               |  2 +-
> >>  drivers/gpu/drm/tiny/st7586.c                |  2 +-
> >>  drivers/gpu/drm/tiny/st7735r.c               |  2 +-
> >>  drivers/gpu/drm/tve200/tve200_display.c      |  2 +-
> >>  drivers/gpu/drm/xen/xen_drm_front_kms.c      |  2 +-
> >>  include/drm/drm_gem_framebuffer_helper.h     |  4 +---
> >>  include/drm/drm_plane.h                      |  2 +-
> >>  include/drm/drm_simple_kms_helper.h          |  6 +++---
> >>  19 files changed, 22 insertions(+), 46 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
> >> index 2184b8be6fd4..0fe72f46f397 100644
> >> --- a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
> >> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c
> >> @@ -219,7 +219,7 @@ static const struct drm_simple_display_pipe_funcs aspeed_gfx_funcs = {
> >>      .enable         = aspeed_gfx_pipe_enable,
> >>      .disable        = aspeed_gfx_pipe_disable,
> >>      .update         = aspeed_gfx_pipe_update,
> >> -    .prepare_fb     = drm_gem_fb_simple_display_pipe_prepare_fb,
> >> +    .prepare_fb     = drm_gem_fb_prepare_fb,
> >>      .enable_vblank  = aspeed_gfx_enable_vblank,
> >>      .disable_vblank = aspeed_gfx_disable_vblank,
> >>  };
> >> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
> >> index 02a9c1ed165b..0891640491eb 100644
> >> --- a/drivers/gpu/drm/bochs/bochs_kms.c
> >> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> >> @@ -69,7 +69,7 @@ static void bochs_pipe_update(struct drm_simple_display_pipe *pipe,
> >>      }
> >>  }
> >>
> >> -static int bochs_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
> >> +static int bochs_pipe_prepare_fb(struct drm_plane *plane,
> >>                               struct drm_plane_state *new_state)
> >>  {
> >>      struct drm_gem_vram_object *gbo;
> >> @@ -80,7 +80,7 @@ static int bochs_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
> >>      return drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
> >>  }
> >>
> >> -static void bochs_pipe_cleanup_fb(struct drm_simple_display_pipe *pipe,
> >> +static void bochs_pipe_cleanup_fb(struct drm_plane *plane,
> >>                                struct drm_plane_state *old_state)
> >>  {
> >>      struct drm_gem_vram_object *gbo;
> >> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> >> index b9bcd310ca2d..c7ea288bef0a 100644
> >> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> >> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> >> @@ -300,25 +300,3 @@ int drm_gem_fb_prepare_fb(struct drm_plane *plane,
> >>      return 0;
> >>  }
> >>  EXPORT_SYMBOL_GPL(drm_gem_fb_prepare_fb);
> >> -
> >> -/**
> >> - * drm_gem_fb_simple_display_pipe_prepare_fb - prepare_fb helper for
> >> - *     &drm_simple_display_pipe
> >> - * @pipe: Simple display pipe
> >> - * @plane_state: Plane state
> >> - *
> >> - * This function uses drm_gem_fb_prepare_fb() to extract the exclusive fence
> >> - * from &drm_gem_object.resv and attaches it to plane state for the atomic
> >> - * helper to wait on. This is necessary to correctly implement implicit
> >> - * synchronization for any buffers shared as a struct &dma_buf. Drivers can use
> >> - * this as their &drm_simple_display_pipe_funcs.prepare_fb callback.
> >> - *
> >> - * See drm_atomic_set_fence_for_plane() for a discussion of implicit and
> >> - * explicit fencing in atomic modeset updates.
> >> - */
> >> -int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
> >> -                                          struct drm_plane_state *plane_state)
> >> -{
> >> -    return drm_gem_fb_prepare_fb(&pipe->plane, plane_state);
> >> -}
> >> -EXPORT_SYMBOL(drm_gem_fb_simple_display_pipe_prepare_fb);
> >> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> >> index 046055719245..e51f336e67f0 100644
> >> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> >> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> >> @@ -173,7 +173,7 @@ static int drm_simple_kms_plane_prepare_fb(struct drm_plane *plane,
> >>      if (!pipe->funcs || !pipe->funcs->prepare_fb)
> >>              return 0;
> >>
> >> -    return pipe->funcs->prepare_fb(pipe, state);
> >> +    return pipe->funcs->prepare_fb(&pipe->plane, state);
> >>  }
> >>
> >>  static void drm_simple_kms_plane_cleanup_fb(struct drm_plane *plane,
> >> @@ -185,7 +185,7 @@ static void drm_simple_kms_plane_cleanup_fb(struct drm_plane *plane,
> >>      if (!pipe->funcs || !pipe->funcs->cleanup_fb)
> >>              return;
> >>
> >> -    pipe->funcs->cleanup_fb(pipe, state);
> >> +    pipe->funcs->cleanup_fb(&pipe->plane, state);
> >>  }
> >>
> >>  static bool drm_simple_kms_format_mod_supported(struct drm_plane *plane,
> >> diff --git a/drivers/gpu/drm/mcde/mcde_display.c b/drivers/gpu/drm/mcde/mcde_display.c
> >> index 751454ae3cd1..31efbdcced5d 100644
> >> --- a/drivers/gpu/drm/mcde/mcde_display.c
> >> +++ b/drivers/gpu/drm/mcde/mcde_display.c
> >> @@ -1097,7 +1097,7 @@ static struct drm_simple_display_pipe_funcs mcde_display_funcs = {
> >>      .enable = mcde_display_enable,
> >>      .disable = mcde_display_disable,
> >>      .update = mcde_display_update,
> >> -    .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> >> +    .prepare_fb = drm_gem_fb_prepare_fb,
> >>  };
> >>
> >>  int mcde_display_init(struct drm_device *drm)
> >> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> >> index 497cf443a9af..547388b32c64 100644
> >> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> >> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> >> @@ -186,7 +186,7 @@ static struct drm_simple_display_pipe_funcs mxsfb_funcs = {
> >>      .enable         = mxsfb_pipe_enable,
> >>      .disable        = mxsfb_pipe_disable,
> >>      .update         = mxsfb_pipe_update,
> >> -    .prepare_fb     = drm_gem_fb_simple_display_pipe_prepare_fb,
> >> +    .prepare_fb     = drm_gem_fb_prepare_fb,
> >>      .enable_vblank  = mxsfb_pipe_enable_vblank,
> >>      .disable_vblank = mxsfb_pipe_disable_vblank,
> >>  };
> >> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> >> index 024771a4083e..e564c838a081 100644
> >> --- a/drivers/gpu/drm/pl111/pl111_display.c
> >> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> >> @@ -441,7 +441,7 @@ static struct drm_simple_display_pipe_funcs pl111_display_funcs = {
> >>      .enable = pl111_display_enable,
> >>      .disable = pl111_display_disable,
> >>      .update = pl111_display_update,
> >> -    .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> >> +    .prepare_fb = drm_gem_fb_prepare_fb,
> >>  };
> >>
> >>  static int pl111_clk_div_choose_div(struct clk_hw *hw, unsigned long rate,
> >> diff --git a/drivers/gpu/drm/tiny/hx8357d.c b/drivers/gpu/drm/tiny/hx8357d.c
> >> index 9af8ff84974f..235223190c04 100644
> >> --- a/drivers/gpu/drm/tiny/hx8357d.c
> >> +++ b/drivers/gpu/drm/tiny/hx8357d.c
> >> @@ -183,7 +183,7 @@ static const struct drm_simple_display_pipe_funcs hx8357d_pipe_funcs = {
> >>      .enable = yx240qv29_enable,
> >>      .disable = mipi_dbi_pipe_disable,
> >>      .update = mipi_dbi_pipe_update,
> >> -    .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> >> +    .prepare_fb = drm_gem_fb_prepare_fb,
> >>  };
> >>
> >>  static const struct drm_display_mode yx350hv15_mode = {
> >> diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
> >> index c66acc566c2b..39b0d425359a 100644
> >> --- a/drivers/gpu/drm/tiny/ili9225.c
> >> +++ b/drivers/gpu/drm/tiny/ili9225.c
> >> @@ -342,7 +342,7 @@ static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = {
> >>      .enable         = ili9225_pipe_enable,
> >>      .disable        = ili9225_pipe_disable,
> >>      .update         = ili9225_pipe_update,
> >> -    .prepare_fb     = drm_gem_fb_simple_display_pipe_prepare_fb,
> >> +    .prepare_fb     = drm_gem_fb_prepare_fb,
> >>  };
> >>
> >>  static const struct drm_display_mode ili9225_mode = {
> >> diff --git a/drivers/gpu/drm/tiny/ili9341.c b/drivers/gpu/drm/tiny/ili9341.c
> >> index 33b51dc7faa8..d107b6172bbb 100644
> >> --- a/drivers/gpu/drm/tiny/ili9341.c
> >> +++ b/drivers/gpu/drm/tiny/ili9341.c
> >> @@ -139,7 +139,7 @@ static const struct drm_simple_display_pipe_funcs ili9341_pipe_funcs = {
> >>      .enable = yx240qv29_enable,
> >>      .disable = mipi_dbi_pipe_disable,
> >>      .update = mipi_dbi_pipe_update,
> >> -    .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> >> +    .prepare_fb = drm_gem_fb_prepare_fb,
> >>  };
> >>
> >>  static const struct drm_display_mode yx240qv29_mode = {
> >> diff --git a/drivers/gpu/drm/tiny/mi0283qt.c b/drivers/gpu/drm/tiny/mi0283qt.c
> >> index e2cfd9a17143..811143a17c77 100644
> >> --- a/drivers/gpu/drm/tiny/mi0283qt.c
> >> +++ b/drivers/gpu/drm/tiny/mi0283qt.c
> >> @@ -143,7 +143,7 @@ static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = {
> >>      .enable = mi0283qt_enable,
> >>      .disable = mipi_dbi_pipe_disable,
> >>      .update = mipi_dbi_pipe_update,
> >> -    .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> >> +    .prepare_fb = drm_gem_fb_prepare_fb,
> >>  };
> >>
> >>  static const struct drm_display_mode mi0283qt_mode = {
> >> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
> >> index 76d179200775..9228d813d3d2 100644
> >> --- a/drivers/gpu/drm/tiny/repaper.c
> >> +++ b/drivers/gpu/drm/tiny/repaper.c
> >> @@ -874,7 +874,7 @@ static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = {
> >>      .enable = repaper_pipe_enable,
> >>      .disable = repaper_pipe_disable,
> >>      .update = repaper_pipe_update,
> >> -    .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> >> +    .prepare_fb = drm_gem_fb_prepare_fb,
> >>  };
> >>
> >>  static int repaper_connector_get_modes(struct drm_connector *connector)
> >> diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
> >> index 3cc21a1b30c8..7b837c46bbfc 100644
> >> --- a/drivers/gpu/drm/tiny/st7586.c
> >> +++ b/drivers/gpu/drm/tiny/st7586.c
> >> @@ -281,7 +281,7 @@ static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = {
> >>      .enable         = st7586_pipe_enable,
> >>      .disable        = st7586_pipe_disable,
> >>      .update         = st7586_pipe_update,
> >> -    .prepare_fb     = drm_gem_fb_simple_display_pipe_prepare_fb,
> >> +    .prepare_fb     = drm_gem_fb_prepare_fb,
> >>  };
> >>
> >>  static const struct drm_display_mode st7586_mode = {
> >> diff --git a/drivers/gpu/drm/tiny/st7735r.c b/drivers/gpu/drm/tiny/st7735r.c
> >> index 3f4487c71684..4801d56a6820 100644
> >> --- a/drivers/gpu/drm/tiny/st7735r.c
> >> +++ b/drivers/gpu/drm/tiny/st7735r.c
> >> @@ -113,7 +113,7 @@ static const struct drm_simple_display_pipe_funcs jd_t18003_t01_pipe_funcs = {
> >>      .enable         = jd_t18003_t01_pipe_enable,
> >>      .disable        = mipi_dbi_pipe_disable,
> >>      .update         = mipi_dbi_pipe_update,
> >> -    .prepare_fb     = drm_gem_fb_simple_display_pipe_prepare_fb,
> >> +    .prepare_fb     = drm_gem_fb_prepare_fb,
> >>  };
> >>
> >>  static const struct drm_display_mode jd_t18003_t01_mode = {
> >> diff --git a/drivers/gpu/drm/tve200/tve200_display.c b/drivers/gpu/drm/tve200/tve200_display.c
> >> index d733bbc4ac0e..6173c61166bb 100644
> >> --- a/drivers/gpu/drm/tve200/tve200_display.c
> >> +++ b/drivers/gpu/drm/tve200/tve200_display.c
> >> @@ -297,7 +297,7 @@ static const struct drm_simple_display_pipe_funcs tve200_display_funcs = {
> >>      .enable = tve200_display_enable,
> >>      .disable = tve200_display_disable,
> >>      .update = tve200_display_update,
> >> -    .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> >> +    .prepare_fb = drm_gem_fb_prepare_fb,
> >>      .enable_vblank = tve200_display_enable_vblank,
> >>      .disable_vblank = tve200_display_disable_vblank,
> >>  };
> >> diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c
> >> index 21ad1c359b61..5689bc91633a 100644
> >> --- a/drivers/gpu/drm/xen/xen_drm_front_kms.c
> >> +++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c
> >> @@ -289,7 +289,7 @@ static const struct drm_simple_display_pipe_funcs display_funcs = {
> >>      .mode_valid = display_mode_valid,
> >>      .enable = display_enable,
> >>      .disable = display_disable,
> >> -    .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
> >> +    .prepare_fb = drm_gem_fb_prepare_fb,
> >>      .update = display_update,
> >>  };
> >>
> >> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> >> index d9f13fd25b0a..1b5a2ff43b4a 100644
> >> --- a/include/drm/drm_gem_framebuffer_helper.h
> >> +++ b/include/drm/drm_gem_framebuffer_helper.h
> >> @@ -10,7 +10,6 @@ struct drm_gem_object;
> >>  struct drm_mode_fb_cmd2;
> >>  struct drm_plane;
> >>  struct drm_plane_state;
> >> -struct drm_simple_display_pipe;
> >>
> >>  struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
> >>                                        unsigned int plane);
> >> @@ -31,6 +30,5 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
> >>
> >>  int drm_gem_fb_prepare_fb(struct drm_plane *plane,
> >>                        struct drm_plane_state *state);
> >> -int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
> >> -                                          struct drm_plane_state *plane_state);
> >> +
> >>  #endif
> >> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> >> index 3f396d94afe4..4065661eb497 100644
> >> --- a/include/drm/drm_plane.h
> >> +++ b/include/drm/drm_plane.h
> >> @@ -75,7 +75,7 @@ struct drm_plane_state {
> >>       *
> >>       * Drivers should store any implicit fence in this from their
> >>       * &drm_plane_helper_funcs.prepare_fb callback. See drm_gem_fb_prepare_fb()
> >> -     * and drm_gem_fb_simple_display_pipe_prepare_fb() for suitable helpers.
> >> +     * for a suitable helper.
> >>       */
> >>      struct dma_fence *fence;
> >>
> >> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
> >> index 4d89cd0a60db..9d6ae8c9769f 100644
> >> --- a/include/drm/drm_simple_kms_helper.h
> >> +++ b/include/drm/drm_simple_kms_helper.h
> >> @@ -114,9 +114,9 @@ struct drm_simple_display_pipe_funcs {
> >>       * more details.
> >>       *
> >>       * Drivers which always have their buffers pinned should use
> >> -     * drm_gem_fb_simple_display_pipe_prepare_fb() for this hook.
> >> +     * drm_gem_fb_prepare_fb() for this hook.
> >>       */
> >> -    int (*prepare_fb)(struct drm_simple_display_pipe *pipe,
> >> +    int (*prepare_fb)(struct drm_plane *plane,
> >>                        struct drm_plane_state *plane_state);
> >>
> >>      /**
> >> @@ -126,7 +126,7 @@ struct drm_simple_display_pipe_funcs {
> >>       * the documentation for the &drm_plane_helper_funcs.cleanup_fb hook for
> >>       * more details.
> >>       */
> >> -    void (*cleanup_fb)(struct drm_simple_display_pipe *pipe,
> >> +    void (*cleanup_fb)(struct drm_plane *plane,
> >>                         struct drm_plane_state *plane_state);
> >>
> >>      /**
> >> --
> >> 2.23.0
> >>
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-10-22 15:48 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-22 10:25 [PATCH 0/5] drm/vram: Provide helpers for prepare_fb() and cleanup_fb() Thomas Zimmermann
2019-10-22 10:25 ` [PATCH 1/5] drm/simple-kms-helper: Pass plane to " Thomas Zimmermann
2019-10-22 14:14   ` Daniel Vetter
2019-10-22 14:41     ` Thomas Zimmermann
2019-10-22 15:48       ` Daniel Vetter
2019-10-22 10:25 ` [PATCH 2/5] drm/vram-helpers: Add helpers for struct drm_plane_helper_funcs Thomas Zimmermann
2019-10-22 10:25 ` [PATCH 3/5] drm/bochs: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers Thomas Zimmermann
2019-10-22 10:25 ` [PATCH 4/5] drm/hisilicon/hibmc: Use GEM VRAM's prepare_fb() and cleanup_fb() helpers Thomas Zimmermann
2019-10-22 10:25 ` [PATCH 5/5] drm/vboxvideo: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers Thomas Zimmermann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.