[1/8] drm/bochs: phase 1 - use the transitional helpers

Commit Message

John Hunter July 16, 2015, 12:20 p.m. UTC

From: Zhao Junwang <[e-mail]>

-register driver's own primary plane
-use drm_crtc_init_with_planes instead of drm_crtc_init

-split ->mode_set into:
	1. set the new hw mode
	2. update the primary plane (This is done by ->set_base)
-move what ->set_base do into ->atomic_update

-the new atomic infrastructure needs the ->mode_set_nofb callback
 to update CRTC timings before setting any plane

-since the ->cleanup_fb can't fail, set the interruptible argument
 of the ttm_bo_reserve to false, this make sure the ttm_bo_reserve
 can't fail

v2: -add a few checks to plane's ->atomic_check, using
     drm_plane_helper_check_update

v3: -polish the atomic_check, it does too much in v2, remove the
     ->disable_plane and ->set_config

v4: -use plane->state instead of old_state in
     bochs_plane_atomic_update

v5: -just return 0 in plane ->atomic_check, i.e. remove what we
     do in v2-v4

Cc: Maarten Lankhorst <[e-mail]>
Cc: Daniel Vetter <[e-mail]>
Signed-off-by: Zhao Junwang <[e-mail]>
---
 drivers/gpu/drm/bochs/bochs.h     |    2 +
 drivers/gpu/drm/bochs/bochs_kms.c |  159 ++++++++++++++++++++++++-------------
 2 files changed, 108 insertions(+), 53 deletions(-)

Comments

Gerd Hoffmann July 16, 2015, 3:49 p.m. UTC | #1

On Do, 2015-07-16 at 20:20 +0800, John Hunter wrote:
> --- a/drivers/gpu/drm/bochs/bochs_kms.c
> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> @@ -10,6 +10,11 @@
>  
>  static int defx = 1024;
>  static int defy = 768;
> +static u64 gpu_addr = 0;

That isn't going to fly.  Guess you could it that into struct
bochs_framebuffer.

cheers,
  Gerd

Daniel Vetter July 16, 2015, 6:22 p.m. UTC | #2

On Thu, Jul 16, 2015 at 05:49:43PM +0200, Gerd Hoffmann wrote:
> On Do, 2015-07-16 at 20:20 +0800, John Hunter wrote:
> > --- a/drivers/gpu/drm/bochs/bochs_kms.c
> > +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> > @@ -10,6 +10,11 @@
> >  
> >  static int defx = 1024;
> >  static int defy = 768;
> > +static u64 gpu_addr = 0;
> 
> That isn't going to fly.  Guess you could it that into struct
> bochs_framebuffer.

Right I completely missed this in the prelimary review I've done. Simplest
solution would be to remove gpu_addr and just lookup the bo offset again
using bochs_bo_gpu_offset.
-Daniel

Patch

diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
index 71f2687..2f10480 100644
--- a/drivers/gpu/drm/bochs/bochs.h
+++ b/drivers/gpu/drm/bochs/bochs.h
@@ -5,6 +5,7 @@ 
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_fb_helper.h>
 
 #include <drm/drm_gem.h>
@@ -72,6 +73,7 @@  struct bochs_device {
 
 	/* drm */
 	struct drm_device  *dev;
+	struct drm_plane primary;
 	struct drm_crtc crtc;
 	struct drm_encoder encoder;
 	struct drm_connector connector;
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
index 26bcd03..1b66dd3 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -10,6 +10,11 @@ 
 
 static int defx = 1024;
 static int defy = 768;
+static u64 gpu_addr = 0;
+
+static const uint32_t bochs_primary_formats[] = {
+	DRM_FORMAT_XRGB8888,
+};
 
 module_param(defx, int, 0444);
 module_param(defy, int, 0444);
@@ -37,59 +42,12 @@  static bool bochs_crtc_mode_fixup(struct drm_crtc *crtc,
 	return true;
 }
 
-static int bochs_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
-				    struct drm_framebuffer *old_fb)
+static void bochs_crtc_mode_set_nofb(struct drm_crtc *crtc)
 {
 	struct bochs_device *bochs =
 		container_of(crtc, struct bochs_device, crtc);
-	struct bochs_framebuffer *bochs_fb;
-	struct bochs_bo *bo;
-	u64 gpu_addr = 0;
-	int ret;
 
-	if (old_fb) {
-		bochs_fb = to_bochs_framebuffer(old_fb);
-		bo = gem_to_bochs_bo(bochs_fb->obj);
-		ret = ttm_bo_reserve(&bo->bo, true, false, false, NULL);
-		if (ret) {
-			DRM_ERROR("failed to reserve old_fb bo\n");
-		} else {
-			bochs_bo_unpin(bo);
-			ttm_bo_unreserve(&bo->bo);
-		}
-	}
-
-	if (WARN_ON(crtc->primary->fb == NULL))
-		return -EINVAL;
-
-	bochs_fb = to_bochs_framebuffer(crtc->primary->fb);
-	bo = gem_to_bochs_bo(bochs_fb->obj);
-	ret = ttm_bo_reserve(&bo->bo, true, false, false, NULL);
-	if (ret)
-		return ret;
-
-	ret = bochs_bo_pin(bo, TTM_PL_FLAG_VRAM, &gpu_addr);
-	if (ret) {
-		ttm_bo_unreserve(&bo->bo);
-		return ret;
-	}
-
-	ttm_bo_unreserve(&bo->bo);
-	bochs_hw_setbase(bochs, x, y, gpu_addr);
-	return 0;
-}
-
-static int bochs_crtc_mode_set(struct drm_crtc *crtc,
-			       struct drm_display_mode *mode,
-			       struct drm_display_mode *adjusted_mode,
-			       int x, int y, struct drm_framebuffer *old_fb)
-{
-	struct bochs_device *bochs =
-		container_of(crtc, struct bochs_device, crtc);
-
-	bochs_hw_setmode(bochs, mode);
-	bochs_crtc_mode_set_base(crtc, x, y, old_fb);
-	return 0;
+	bochs_hw_setmode(bochs, &crtc->mode);
 }
 
 static void bochs_crtc_prepare(struct drm_crtc *crtc)
@@ -116,7 +74,7 @@  static int bochs_crtc_page_flip(struct drm_crtc *crtc,
 	unsigned long irqflags;
 
 	crtc->primary->fb = fb;
-	bochs_crtc_mode_set_base(crtc, 0, 0, old_fb);
+	drm_helper_crtc_mode_set_base(crtc, 0, 0, old_fb);
 	if (event) {
 		spin_lock_irqsave(&bochs->dev->event_lock, irqflags);
 		drm_send_vblank_event(bochs->dev, -1, event);
@@ -136,8 +94,9 @@  static const struct drm_crtc_funcs bochs_crtc_funcs = {
 static const struct drm_crtc_helper_funcs bochs_helper_funcs = {
 	.dpms = bochs_crtc_dpms,
 	.mode_fixup = bochs_crtc_mode_fixup,
-	.mode_set = bochs_crtc_mode_set,
-	.mode_set_base = bochs_crtc_mode_set_base,
+	.mode_set = drm_helper_crtc_mode_set,
+	.mode_set_base = drm_helper_crtc_mode_set_base,
+	.mode_set_nofb = bochs_crtc_mode_set_nofb,
 	.prepare = bochs_crtc_prepare,
 	.commit = bochs_crtc_commit,
 };
@@ -146,12 +105,105 @@  static void bochs_crtc_init(struct drm_device *dev)
 {
 	struct bochs_device *bochs = dev->dev_private;
 	struct drm_crtc *crtc = &bochs->crtc;
+	struct drm_plane *primary = &bochs->primary;
 
-	drm_crtc_init(dev, crtc, &bochs_crtc_funcs);
+	drm_crtc_init_with_planes(dev, crtc, primary, NULL, &bochs_crtc_funcs);
 	drm_mode_crtc_set_gamma_size(crtc, 256);
 	drm_crtc_helper_add(crtc, &bochs_helper_funcs);
 }
 
+static int bochs_plane_prepare_fb(struct drm_plane *plane,
+			  struct drm_framebuffer *fb,
+			  const struct drm_plane_state *new_state)
+{
+	struct bochs_framebuffer *bochs_fb;
+	struct bochs_bo *bo;
+	int ret;
+
+	if (WARN_ON(plane->fb == NULL))
+		return -EINVAL;
+
+	bochs_fb = to_bochs_framebuffer(plane->fb);
+	bo = gem_to_bochs_bo(bochs_fb->obj);
+	ttm_bo_reserve(&bo->bo, true, false, false, NULL);
+
+	ret = bochs_bo_pin(bo, TTM_PL_FLAG_VRAM, &gpu_addr);
+	if (ret) {
+		ttm_bo_unreserve(&bo->bo);
+		return ret;
+	}
+
+	ttm_bo_unreserve(&bo->bo);
+	return 0;
+}
+
+static void bochs_plane_cleanup_fb(struct drm_plane *plane,
+			   struct drm_framebuffer *old_fb,
+			   const struct drm_plane_state *old_state)
+{
+	struct bochs_framebuffer *bochs_fb;
+	struct bochs_bo *bo;
+
+	bochs_fb = to_bochs_framebuffer(old_fb);
+	bo = gem_to_bochs_bo(bochs_fb->obj);
+
+	/*
+	 * Since cleanup can't fail, change the interruptible argument i.e. the
+	 * second argument to false, this make sure ttm_bo_unpin can't fail.
+	 */
+	ttm_bo_reserve(&bo->bo, false, false, false, NULL);
+	bochs_bo_unpin(bo);
+	ttm_bo_unreserve(&bo->bo);
+}
+
+static int bochs_plane_atomic_check(struct drm_plane *plane,
+			    struct drm_plane_state *plane_state)
+{
+	return 0;
+}
+
+static void bochs_plane_atomic_update(struct drm_plane *plane,
+			      struct drm_plane_state *old_state)
+{
+	struct bochs_device *bochs =
+		container_of(plane, struct bochs_device, primary);
+	int x = plane->state->src_x >> 16;
+	int y = plane->state->src_y >> 16;
+
+	bochs_hw_setbase(bochs, x, y, gpu_addr);
+}
+
+static const struct drm_plane_funcs bochs_plane_funcs = {
+	.update_plane = drm_plane_helper_update,
+	.disable_plane = drm_plane_helper_disable,
+};
+
+static const struct drm_plane_helper_funcs bochs_plane_helper_funcs = {
+	.prepare_fb = bochs_plane_prepare_fb,
+	.cleanup_fb = bochs_plane_cleanup_fb,
+	.atomic_check = bochs_plane_atomic_check,
+	.atomic_update = bochs_plane_atomic_update,
+};
+
+static void bochs_plane_init(struct drm_device *dev)
+{
+	struct bochs_device *bochs = dev->dev_private;
+	struct drm_plane *primary = &bochs->primary;
+	int ret;
+
+	ret = drm_universal_plane_init(dev, primary, 0,
+				       &bochs_plane_funcs,
+				       bochs_primary_formats,
+				       ARRAY_SIZE(bochs_primary_formats),
+				       DRM_PLANE_TYPE_PRIMARY);
+	if (ret) {
+		DRM_DEBUG_KMS("Failed to init primary plane init");
+		return;
+	}
+
+	drm_plane_helper_add(primary, &bochs_plane_helper_funcs);
+}
+
 static bool bochs_encoder_mode_fixup(struct drm_encoder *encoder,
 				     const struct drm_display_mode *mode,
 				     struct drm_display_mode *adjusted_mode)
@@ -285,6 +337,7 @@  int bochs_kms_init(struct bochs_device *bochs)
 
 	bochs->dev->mode_config.funcs = (void *)&bochs_mode_funcs;
 
+	bochs_plane_init(bochs->dev);
 	bochs_crtc_init(bochs->dev);
 	bochs_encoder_init(bochs->dev);
 	bochs_connector_init(bochs->dev);