diff mbox

[9/9] drm/i915: split intel_pipe_set_base() into check() and commit()

Message ID 1409247613-14232-9-git-send-email-gustavo@padovan.org (mailing list archive)
State New, archived
Headers show

Commit Message

Gustavo Padovan Aug. 28, 2014, 5:40 p.m. UTC
From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Take out some parts of  code that can fail from it and move them to
intel_pipe_check_base(), the only failure point in intel_pipe_set_base()
now is the fb pinning procudure.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c | 39 ++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

Comments

Ville Syrjälä Aug. 29, 2014, 8:06 a.m. UTC | #1
On Thu, Aug 28, 2014 at 02:40:13PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> Take out some parts of  code that can fail from it and move them to
> intel_pipe_check_base(), the only failure point in intel_pipe_set_base()
> now is the fb pinning procudure.

I'd like to see intel_pipe_set_base() replaced entirely with the
primary plane setplane. Right now we have duplicated code paths all over
the place due to intel_pipe_set_base().

After that's done the same plane config treatment should be applied to
primary planes as I suggested for sprites.

> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 39 ++++++++++++++++++++++++++++--------
>  1 file changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index eb6febf..ead2f24 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2661,17 +2661,11 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
>  }
>  
>  static int
> -intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
> +intel_pipe_check_base(struct drm_crtc *crtc, int x, int y,
>  		    struct drm_framebuffer *fb)
>  {
>  	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	enum pipe pipe = intel_crtc->pipe;
> -	struct drm_framebuffer *old_fb = crtc->primary->fb;
> -	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> -	struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
> -	int ret;
>  
>  	if (intel_crtc_has_pending_flip(crtc)) {
>  		DRM_ERROR("pipe is still busy with an old pageflip\n");
> @@ -2691,6 +2685,22 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
>  		return -EINVAL;
>  	}
>  
> +	return 0;
> +}
> +
> +static int
> +intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
> +		    struct drm_framebuffer *fb)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	enum pipe pipe = intel_crtc->pipe;
> +	struct drm_framebuffer *old_fb = crtc->primary->fb;
> +	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> +	struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
> +	int ret;
> +
>  	mutex_lock(&dev->struct_mutex);
>  	ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
>  	if (ret == 0)
> @@ -9868,6 +9878,10 @@ free_work:
>  	if (ret == -EIO) {
>  out_hang:
>  		intel_crtc_wait_for_pending_flips(crtc);
> +		ret = intel_pipe_check_base(crtc, crtc->x, crtc->y, fb);
> +		if (ret)
> +			return ret;
> +
>  		ret = intel_pipe_set_base(crtc, crtc->x, crtc->y, fb);
>  		if (ret == 0 && event)
>  			drm_send_vblank_event(dev, pipe, event);
> @@ -11396,6 +11410,10 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>  
>  		intel_crtc_wait_for_pending_flips(set->crtc);
>  
> +		ret = intel_pipe_check_base(set->crtc, set->x, set->y, set->fb);
> +		if (ret)
> +			goto fail;
> +
>  		ret = intel_pipe_set_base(set->crtc,
>  					  set->x, set->y, set->fb);
>  
> @@ -11620,12 +11638,17 @@ intel_check_primary_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  		.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
>  		.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
>  	};
> +	int ret;
>  
> -	return drm_plane_helper_check_update(plane, crtc, fb,
> +	ret = drm_plane_helper_check_update(plane, crtc, fb,
>  					    &src, &dest, &clip,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    false, true, visible);
> +	if (ret)
> +		return ret;
> +
> +	return intel_pipe_check_base(crtc, src.x1, src.y1, fb);
>  }
>  
>  static int
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index eb6febf..ead2f24 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2661,17 +2661,11 @@  static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
 }
 
 static int
-intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
+intel_pipe_check_base(struct drm_crtc *crtc, int x, int y,
 		    struct drm_framebuffer *fb)
 {
 	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	enum pipe pipe = intel_crtc->pipe;
-	struct drm_framebuffer *old_fb = crtc->primary->fb;
-	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
-	struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
-	int ret;
 
 	if (intel_crtc_has_pending_flip(crtc)) {
 		DRM_ERROR("pipe is still busy with an old pageflip\n");
@@ -2691,6 +2685,22 @@  intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 		return -EINVAL;
 	}
 
+	return 0;
+}
+
+static int
+intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
+		    struct drm_framebuffer *fb)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	enum pipe pipe = intel_crtc->pipe;
+	struct drm_framebuffer *old_fb = crtc->primary->fb;
+	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+	struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
+	int ret;
+
 	mutex_lock(&dev->struct_mutex);
 	ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
 	if (ret == 0)
@@ -9868,6 +9878,10 @@  free_work:
 	if (ret == -EIO) {
 out_hang:
 		intel_crtc_wait_for_pending_flips(crtc);
+		ret = intel_pipe_check_base(crtc, crtc->x, crtc->y, fb);
+		if (ret)
+			return ret;
+
 		ret = intel_pipe_set_base(crtc, crtc->x, crtc->y, fb);
 		if (ret == 0 && event)
 			drm_send_vblank_event(dev, pipe, event);
@@ -11396,6 +11410,10 @@  static int intel_crtc_set_config(struct drm_mode_set *set)
 
 		intel_crtc_wait_for_pending_flips(set->crtc);
 
+		ret = intel_pipe_check_base(set->crtc, set->x, set->y, set->fb);
+		if (ret)
+			goto fail;
+
 		ret = intel_pipe_set_base(set->crtc,
 					  set->x, set->y, set->fb);
 
@@ -11620,12 +11638,17 @@  intel_check_primary_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 		.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
 		.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
 	};
+	int ret;
 
-	return drm_plane_helper_check_update(plane, crtc, fb,
+	ret = drm_plane_helper_check_update(plane, crtc, fb,
 					    &src, &dest, &clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    false, true, visible);
+	if (ret)
+		return ret;
+
+	return intel_pipe_check_base(crtc, src.x1, src.y1, fb);
 }
 
 static int