On Wed, Sep 03, 2014 at 05:10:16PM -0300, Gustavo Padovan wrote:

From: Gustavo Padovan gustavo.padovan@collabora.co.uk

Due to the upcoming atomic modesetting feature we need to separate some update functions into a check step that can fail and a commit step that should, ideally, never fail.

This commit splits intel_update_plane() and its commit part can still fail due to the fb pinning procedure.

Crap. I wrote a reply and somehow it seems to have gotten lost before I sent it out. I'll try to recall it all again...

Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk

drivers/gpu/drm/i915/intel_sprite.c | 253 +++++++++++++++++++++--------------- 1 file changed, 150 insertions(+), 103 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 07a74ef..7b0d1a9 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -845,57 +845,24 @@ static bool colorkey_enabled(struct intel_plane *intel_plane) }

static int -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,

•    struct drm_framebuffer *fb, int crtc_x, int crtc_y,
  

•    unsigned int crtc_w, unsigned int crtc_h,
  

•    uint32_t src_x, uint32_t src_y,
  

•    uint32_t src_w, uint32_t src_h)
  

+intel_check_sprite_plane(struct drm_plane *plane,

• 	 struct intel_plane_state *state)
  

{

• struct drm_device *dev = plane->dev;
• struct intel_crtc *intel_crtc = to_intel_crtc(crtc);

• struct intel_crtc *intel_crtc = to_intel_crtc(state->crtc); struct intel_plane *intel_plane = to_intel_plane(plane);

• enum pipe pipe = intel_crtc->pipe;

• struct drm_framebuffer *fb = state->fb; struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); struct drm_i915_gem_object *obj = intel_fb->obj;

• struct drm_i915_gem_object *old_obj = intel_plane->obj;
• int ret;
• bool primary_enabled;
• bool visible;

• int crtc_x, crtc_y;
• unsigned int crtc_w, crtc_h;
• uint32_t src_x, src_y, src_w, src_h;
• struct drm_rect *src = &state->src;
• struct drm_rect *dst = &state->dst;
• struct drm_rect *orig_src = &state->orig_src;
• const struct drm_rect *clip = &state->clip; int hscale, vscale; int max_scale, min_scale; int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);

• struct drm_rect src = {

• /* sample coordinates in 16.16 fixed point */
  

• .x1 = src_x,
  

• .x2 = src_x + src_w,
  

• .y1 = src_y,
  

• .y2 = src_y + src_h,
  

• };

• struct drm_rect dst = {

• /* integer pixels */
  

• .x1 = crtc_x,
  

• .x2 = crtc_x + crtc_w,
  

• .y1 = crtc_y,
  

• .y2 = crtc_y + crtc_h,
  

• };

• const struct drm_rect clip = {

• .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
  

• .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
  

• };

• const struct {

• int crtc_x, crtc_y;
  

• unsigned int crtc_w, crtc_h;
  

• uint32_t src_x, src_y, src_w, src_h;
  

• } orig = {

• .crtc_x = crtc_x,
  

• .crtc_y = crtc_y,
  

• .crtc_w = crtc_w,
  

• .crtc_h = crtc_h,
  

• .src_x = src_x,
  

• .src_y = src_y,
  

• .src_w = src_w,
  

• .src_h = src_h,
  

• };

  /* Don't modify another pipe's plane */ if (intel_plane->pipe != intel_crtc->pipe) {

@@ -927,55 +894,55 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, max_scale = intel_plane->max_downscale << 16; min_scale = intel_plane->can_scale ? 1 : (1 << 16);

• drm_rect_rotate(&src, fb->width << 16, fb->height << 16,

• drm_rect_rotate(src, fb->width << 16, fb->height << 16, intel_plane->rotation);

• hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale);

• hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale); BUG_ON(hscale < 0);

• vscale = drm_rect_calc_vscale_relaxed(&src, &dst, min_scale, max_scale);

• vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale); BUG_ON(vscale < 0);

• visible = drm_rect_clip_scaled(&src, &dst, &clip, hscale, vscale);

• state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);

• crtc_x = dst.x1;
• crtc_y = dst.y1;
• crtc_w = drm_rect_width(&dst);
• crtc_h = drm_rect_height(&dst);

• crtc_x = dst->x1;
• crtc_y = dst->y1;
• crtc_w = drm_rect_width(dst);
• crtc_h = drm_rect_height(dst);

• if (visible) {

• if (state->visible) { /* check again in case clipping clamped the results */

• hscale = drm_rect_calc_hscale(&src, &dst, min_scale, max_scale);
  

• hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
  

  if (hscale < 0) { DRM_DEBUG_KMS("Horizontal scaling factor out of limits\n");

• 	drm_rect_debug_print(&src, true);
  

• 	drm_rect_debug_print(&dst, false);
  

• 	drm_rect_debug_print(src, true);
  

• 	drm_rect_debug_print(dst, false);
  
  return hscale;
  

  }

• vscale = drm_rect_calc_vscale(&src, &dst, min_scale, max_scale);
  

• vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
  

  if (vscale < 0) { DRM_DEBUG_KMS("Vertical scaling factor out of limits\n");

• 	drm_rect_debug_print(&src, true);
  

• 	drm_rect_debug_print(&dst, false);
  

• 	drm_rect_debug_print(src, true);
  

• 	drm_rect_debug_print(dst, false);
  
  return vscale;
  

  }

  /* Make the source viewport size an exact multiple of the scaling factors. */

• drm_rect_adjust_size(&src,
  

• 		     drm_rect_width(&dst) * hscale - drm_rect_width(&src),
  

• 		     drm_rect_height(&dst) * vscale - drm_rect_height(&src));
  

• drm_rect_adjust_size(src,
  

• 		     drm_rect_width(dst) * hscale - drm_rect_width(src),
  

• 		     drm_rect_height(dst) * vscale - drm_rect_height(src));
  
  

• drm_rect_rotate_inv(&src, fb->width << 16, fb->height << 16,
  

• drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
  	    intel_plane->rotation);
  
  

  /* sanity check to make sure the src viewport wasn't enlarged */

• WARN_ON(src.x1 < (int) src_x ||
  

• 	src.y1 < (int) src_y ||
  

• 	src.x2 > (int) (src_x + src_w) ||
  

• 	src.y2 > (int) (src_y + src_h));
  

• WARN_ON(src->x1 < (int) orig_src->x1 ||
  

• 	src->y1 < (int) orig_src->y1 ||
  

• 	src->x2 > (int) orig_src->x2 ||
  

• 	src->y2 > (int) orig_src->y2);
  
  

  /*

  • Hardware doesn't handle subpixel coordinates.

@@ -983,10 +950,10 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, * increase the source viewport size, because that could * push the downscaling factor out of bounds. */

• src_x = src.x1 >> 16;
  

• src_w = drm_rect_width(&src) >> 16;
  

• src_y = src.y1 >> 16;
  

• src_h = drm_rect_height(&src) >> 16;
  

• src_x = src->x1 >> 16;
  

• src_w = drm_rect_width(src) >> 16;
  

• src_y = src->y1 >> 16;
  

• src_h = drm_rect_height(src) >> 16;
  
  

  if (format_is_yuv(fb->pixel_format)) { src_x &= ~1;

@@ -1000,44 +967,74 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, crtc_w &= ~1;

	if (crtc_w == 0)

• 		visible = false;
  

• 		state->visible = false;
  

  }

• }

• /* Check size restrictions when scaling */

• if (visible && (src_w != crtc_w || src_h != crtc_h)) {

I'd leave the code structure as is here to avoid changing the indentation. It just makes it harder to review.

Also in theory the "Source dimensions exceed hardware limits" check might get triggered for an invisible plane. I think that should be impossible given fugded scaling coordinates we do, but better safe than sorry.

• unsigned int width_bytes;
  

• /* Check size restrictions when scaling */
  

• if ((src_w != crtc_w || src_h != crtc_h)) {
  

• 	unsigned int width_bytes;
  
  

• WARN_ON(!intel_plane->can_scale);
  

• 	WARN_ON(!intel_plane->can_scale);
  
  

• /* FIXME interlacing min height is 6 */
  

• 	/* FIXME interlacing min height is 6 */
  
  

• if (crtc_w < 3 || crtc_h < 3)
  

• 	visible = false;
  

• 	if (crtc_w < 3 || crtc_h < 3)
  

• 		state->visible = false;
  
  

• if (src_w < 3 || src_h < 3)
  

• 	visible = false;
  

• 	if (src_w < 3 || src_h < 3)
  

• 		state->visible = false;
  
  

• width_bytes = ((src_x * pixel_size) & 63) + src_w * pixel_size;
  

• 	width_bytes = ((src_x * pixel_size) & 63) +
  

• 				src_w * pixel_size;
  
  

• if (src_w > 2048 || src_h > 2048 ||
  

•     width_bytes > 4096 || fb->pitches[0] > 4096) {
  

• 	DRM_DEBUG_KMS("Source dimensions exceed hardware limits\n");
  

• 	return -EINVAL;
  

• 	if (src_w > 2048 || src_h > 2048 ||
  

• 	    width_bytes > 4096 || fb->pitches[0] > 4096) {
  

• 		DRM_DEBUG_KMS("Source dimensions exceed hardware limits\n");
  

• 		return -EINVAL;
  

• 	}
  

  }

• src->x1 = src_x;
  

• src->x2 = src_x + src_w;
  

• src->y1 = src_y;
  

• src->y2 = src_y + src_h;
  

• dst->x1 = crtc_x;
  

• dst->x2 = crtc_x + crtc_w;
  

• dst->y1 = crtc_y;
  

• dst->y2 = crtc_y + crtc_h;
  

  }

• dst.x1 = crtc_x;
• dst.x2 = crtc_x + crtc_w;
• dst.y1 = crtc_y;
• dst.y2 = crtc_y + crtc_h;

We need to keep the dst coordinate assignment outside the 'if(visible)' block to keep the primary_enabled calculation happy in commit().

And now I have nagging feeling I spotted something else when I wrote my original reply. But now I can't spot anything else. So let's hope that's just one of those "did I remember to turn off the stove/lock the door?" type of feelings.

• return 0;

+}

+static int +intel_commit_sprite_plane(struct drm_plane *plane,

• 	  struct intel_plane_state *state)
  

+{

• struct drm_device *dev = plane->dev;

• struct drm_crtc *crtc = state->crtc;

• struct intel_crtc *intel_crtc = to_intel_crtc(crtc);

• struct intel_plane *intel_plane = to_intel_plane(plane);

• enum pipe pipe = intel_crtc->pipe;

• struct drm_framebuffer *fb = state->fb;

• struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);

• struct drm_i915_gem_object *obj = intel_fb->obj;

• struct drm_i915_gem_object *old_obj = intel_plane->obj;

• int crtc_x, crtc_y;

• unsigned int crtc_w, crtc_h;

• uint32_t src_x, src_y, src_w, src_h;

• struct drm_rect *dst = &state->dst;

• const struct drm_rect *clip = &state->clip;

• bool primary_enabled;

• int ret;

  /*

  • If the sprite is completely covering the primary plane,
  • we can disable the primary and save power.

  */

• primary_enabled = !drm_rect_equals(&dst, &clip) || colorkey_enabled(intel_plane);
• WARN_ON(!primary_enabled && !visible && intel_crtc->active);

• primary_enabled = !drm_rect_equals(dst, clip) || colorkey_enabled(intel_plane);

• WARN_ON(!primary_enabled && !state->visible && intel_crtc->active);

  mutex_lock(&dev->struct_mutex);

@@ -1055,14 +1052,14 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, if (ret) return ret;

• intel_plane->crtc_x = orig.crtc_x;
• intel_plane->crtc_y = orig.crtc_y;
• intel_plane->crtc_w = orig.crtc_w;
• intel_plane->crtc_h = orig.crtc_h;
• intel_plane->src_x = orig.src_x;
• intel_plane->src_y = orig.src_y;
• intel_plane->src_w = orig.src_w;
• intel_plane->src_h = orig.src_h;

• intel_plane->crtc_x = state->orig_dst.x1;

• intel_plane->crtc_y = state->orig_dst.y1;

• intel_plane->crtc_w = drm_rect_width(&state->orig_dst);

• intel_plane->crtc_h = drm_rect_height(&state->orig_dst);

• intel_plane->src_x = state->orig_src.x1;

• intel_plane->src_y = state->orig_src.y1;

• intel_plane->src_w = drm_rect_width(&state->orig_src);

• intel_plane->src_h = drm_rect_height(&state->orig_src); intel_plane->obj = obj;

  if (intel_crtc->active) {

@@ -1076,12 +1073,22 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, if (primary_was_enabled && !primary_enabled) intel_pre_disable_primary(crtc);

• if (visible)
  

• if (state->visible) {
  

• 	crtc_x = state->dst.x1;
  

• 	crtc_y = state->dst.x2;
  

• 	crtc_w = drm_rect_width(&state->dst);
  

• 	crtc_h = drm_rect_height(&state->dst);
  

• 	src_x = state->src.x1;
  

• 	src_y = state->src.y1;
  

• 	src_w = drm_rect_width(&state->src);
  

• 	src_h = drm_rect_height(&state->src);
  intel_plane->update_plane(plane, crtc, fb, obj,
  			  crtc_x, crtc_y, crtc_w, crtc_h,
  			  src_x, src_y, src_w, src_h);
  

• else
  

• } else {
  intel_plane->disable_plane(plane, crtc);
  

• }
  

• intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_SPRITE(pipe));

@@ -1109,6 +1116,46 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, }

static int +intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,

•    struct drm_framebuffer *fb, int crtc_x, int crtc_y,
  

•    unsigned int crtc_w, unsigned int crtc_h,
  

•    uint32_t src_x, uint32_t src_y,
  

•    uint32_t src_w, uint32_t src_h)
  

+{

• struct intel_plane_state state;
• struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
• int ret;
• state.crtc = crtc;
• state.fb = fb;
• /* sample coordinates in 16.16 fixed point */
• state.src.x1 = src_x;
• state.src.x2 = src_x + src_w;
• state.src.y1 = src_y;
• state.src.y2 = src_y + src_h;
• /* integer pixels */
• state.dst.x1 = crtc_x;
• state.dst.x2 = crtc_x + crtc_w;
• state.dst.y1 = crtc_y;
• state.dst.y2 = crtc_y + crtc_h;
• state.clip.x1 = 0;
• state.clip.y1 = 0;
• state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0;
• state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0;
• state.orig_src = state.src;
• state.orig_dst = state.dst;
• ret = intel_check_sprite_plane(plane, &state);
• if (ret)

• return ret;
  

• return intel_commit_sprite_plane(plane, &state);

+}

+static int intel_disable_plane(struct drm_plane *plane) { struct drm_device *dev = plane->dev; -- 1.9.3

On Wed, Sep 03, 2014 at 05:10:17PM -0300, Gustavo Padovan wrote:

From: Gustavo Padovan gustavo.padovan@collabora.co.uk

Due to the upcoming atomic modesetting feature we need to separate some update functions into a check step that can fail and a commit step that should, ideally, never fail.

The commit part can still fail, but that should be solved in another upcoming patch.

Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk

drivers/gpu/drm/i915/intel_display.c | 104 ++++++++++++++++++++++------------- 1 file changed, 67 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 22d3902..c3f1967 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11896,51 +11896,42 @@ intel_cursor_plane_disable(struct drm_plane *plane) }

static int -intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,

• 	  struct drm_framebuffer *fb, int crtc_x, int crtc_y,
  

• 	  unsigned int crtc_w, unsigned int crtc_h,
  

• 	  uint32_t src_x, uint32_t src_y,
  

• 	  uint32_t src_w, uint32_t src_h)
  

+intel_check_cursor_plane(struct drm_plane *plane,

• 	 struct intel_plane_state *state)
  

{

• struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
• struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
• struct drm_i915_gem_object *obj = intel_fb->obj;
• struct drm_rect dest = {

• /* integer pixels */
  

• .x1 = crtc_x,
  

• .y1 = crtc_y,
  

• .x2 = crtc_x + crtc_w,
  

• .y2 = crtc_y + crtc_h,
  

• };
• struct drm_rect src = {

• /* 16.16 fixed point */
  

• .x1 = src_x,
  

• .y1 = src_y,
  

• .x2 = src_x + src_w,
  

• .y2 = src_y + src_h,
  

• };
• const struct drm_rect clip = {

• /* integer pixels */
  

• .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
  

• .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
  

• };
• bool visible;
• int ret;

• struct drm_crtc *crtc = state->crtc;
• struct drm_framebuffer *fb = state->fb;
• struct drm_rect *dest = &state->dst;
• struct drm_rect *src = &state->src;
• const struct drm_rect *clip = &state->clip;

• ret = drm_plane_helper_check_update(plane, crtc, fb,

• 			    &src, &dest, &clip,
  

• return drm_plane_helper_check_update(plane, crtc, fb,

• 			    src, dest, clip,
  		    DRM_PLANE_HELPER_NO_SCALING,
  		    DRM_PLANE_HELPER_NO_SCALING,
  

• 			    true, true, &visible);
  

• if (ret)

• return ret;
  

• 			    true, true, &state->visible);
  

+}

• crtc->cursor_x = crtc_x;
• crtc->cursor_y = crtc_y;

+static int +intel_commit_cursor_plane(struct drm_plane *plane,

• 	  struct intel_plane_state *state)
  

+{

• struct drm_crtc *crtc = state->crtc;
• struct drm_framebuffer *fb = state->fb;
• struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
• struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
• struct drm_i915_gem_object *obj = intel_fb->obj;
• struct drm_rect *dest = &state->dst;
• int crtc_w, crtc_h;
• crtc->cursor_x = state->dst.x1;
• crtc->cursor_y = state->dst.y1; if (fb != crtc->cursor->fb) {

• crtc_w = drm_rect_width(dest);
  

• crtc_h = drm_rect_height(dest);
  

These would need to be the original unclipped coordinates since we program the cursor hardware with those and the hardware clips itself.

return intel_crtc_cursor_set_obj(crtc, obj, crtc_w, crtc_h);

} else {

• intel_crtc_update_cursor(crtc, visible);
  

• intel_crtc_update_cursor(crtc, state->visible);
  
  

  intel_frontbuffer_flip(crtc->dev, INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe));

@@ -11948,6 +11939,45 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, return 0; } }

+static int +intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,

• 	  struct drm_framebuffer *fb, int crtc_x, int crtc_y,
  

• 	  unsigned int crtc_w, unsigned int crtc_h,
  

• 	  uint32_t src_x, uint32_t src_y,
  

• 	  uint32_t src_w, uint32_t src_h)
  

+{

• struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
• struct intel_plane_state state;
• int ret;
• state.crtc = crtc;
• state.fb = fb;
• /* sample coordinates in 16.16 fixed point */
• state.src.x1 = src_x;
• state.src.x2 = src_x + src_w;
• state.src.y1 = src_y;
• state.src.y2 = src_y + src_h;
• /* integer pixels */
• state.dst.x1 = crtc_x;
• state.dst.x2 = crtc_x + crtc_w;
• state.dst.y1 = crtc_y;
• state.dst.y2 = crtc_y + crtc_h;
• state.clip.x1 = 0;
• state.clip.y1 = 0;
• state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0;
• state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0;
• ret = intel_check_cursor_plane(plane, &state);
• if (ret)

• return ret;
  

• return intel_commit_cursor_plane(plane, &state);

+}

static const struct drm_plane_funcs intel_cursor_plane_funcs = { .update_plane = intel_cursor_plane_update, .disable_plane = intel_cursor_plane_disable, -- 1.9.3

On Wed, Sep 03, 2014 at 05:10:18PM -0300, Gustavo Padovan wrote:

From: Gustavo Padovan gustavo.padovan@collabora.co.uk

As a preparation for atomic updates we need to split the code to check everything we are going to commit first. This patch starts the work to split intel_primary_plane_setplane() into check() and commit() parts.

More work is expected on this to get a better split of the two steps. Ideally the commit() step should never fail.

Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk

drivers/gpu/drm/i915/intel_display.c | 130 +++++++++++++++++++---------------- 1 file changed, 72 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c3f1967..1e3985b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11663,63 +11663,37 @@ disable_unpin: }

static int -intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,

• 	     struct drm_framebuffer *fb, int crtc_x, int crtc_y,
  

• 	     unsigned int crtc_w, unsigned int crtc_h,
  

• 	     uint32_t src_x, uint32_t src_y,
  

• 	     uint32_t src_w, uint32_t src_h)
  

+intel_check_primary_plane(struct drm_plane *plane,

• 	  struct intel_plane_state *state)
  

+{

• struct drm_crtc *crtc = state->crtc;
• struct drm_framebuffer *fb = state->fb;
• struct drm_rect *dest = &state->dst;
• struct drm_rect *src = &state->src;
• const struct drm_rect *clip = &state->clip;
• return drm_plane_helper_check_update(plane, crtc, fb,

• 			    src, dest, clip,
  

• 			    DRM_PLANE_HELPER_NO_SCALING,
  

• 			    DRM_PLANE_HELPER_NO_SCALING,
  

• 			    false, true, &state->visible);
  

+}

+static int +intel_commit_primary_plane(struct drm_plane *plane,

• 	   struct intel_plane_state *state)
  

{

• struct drm_crtc *crtc = state->crtc;
• struct drm_framebuffer *fb = state->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); struct drm_i915_gem_object *obj = intel_fb_obj(fb); struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);

• struct drm_rect dest = {

• /* integer pixels */
  

• .x1 = crtc_x,
  

• .y1 = crtc_y,
  

• .x2 = crtc_x + crtc_w,
  

• .y2 = crtc_y + crtc_h,
  

• };
• struct drm_rect src = {

• /* 16.16 fixed point */
  

• .x1 = src_x,
  

• .y1 = src_y,
  

• .x2 = src_x + src_w,
  

• .y2 = src_y + src_h,
  

• };
• const struct drm_rect clip = {

• /* integer pixels */
  

• .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
  

• .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
  

• };
• const struct {

• int crtc_x, crtc_y;
  

• unsigned int crtc_w, crtc_h;
  

• uint32_t src_x, src_y, src_w, src_h;
  

• } orig = {

• .crtc_x = crtc_x,
  

• .crtc_y = crtc_y,
  

• .crtc_w = crtc_w,
  

• .crtc_h = crtc_h,
  

• .src_x = src_x,
  

• .src_y = src_y,
  

• .src_w = src_w,
  

• .src_h = src_h,
  

• }; struct intel_plane *intel_plane = to_intel_plane(plane);
• bool visible;

• struct drm_rect *src = &state->src; int ret;

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

• /*
  • If the CRTC isn't enabled, we're just pinning the framebuffer,
  • updating the fb pointer, and returning without touching the

@@ -11754,7 +11728,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, * happens if userspace explicitly disables the plane by passing fb=0 * because plane->fb still gets set and pinned. */

• if (!visible) {

• if (!state->visible) { mutex_lock(&dev->struct_mutex);

  /*

@@ -11801,7 +11775,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, intel_disable_fbc(dev); } }

• ret = intel_pipe_set_base(crtc, src.x1, src.y1, fb);
  

• ret = intel_pipe_set_base(crtc, src->x1, src->y1, fb);
  

  if (ret) return ret;

@@ -11809,19 +11783,59 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, intel_enable_primary_hw_plane(plane, crtc); }

• intel_plane->crtc_x = orig.crtc_x;
• intel_plane->crtc_y = orig.crtc_y;
• intel_plane->crtc_w = orig.crtc_w;
• intel_plane->crtc_h = orig.crtc_h;
• intel_plane->src_x = orig.src_x;
• intel_plane->src_y = orig.src_y;
• intel_plane->src_w = orig.src_w;
• intel_plane->src_h = orig.src_h;

• intel_plane->crtc_x = state->orig_dst.x1;

• intel_plane->crtc_y = state->orig_dst.y1;

• intel_plane->crtc_w = drm_rect_width(&state->orig_dst);

• intel_plane->crtc_h = drm_rect_height(&state->orig_dst);

• intel_plane->src_x = state->orig_src.x1;

• intel_plane->src_y = state->orig_src.y1;

• intel_plane->src_w = drm_rect_width(&state->orig_src);

• intel_plane->src_h = drm_rect_height(&state->orig_src); intel_plane->obj = obj;

  return 0;

}

+static int +intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,

• 	     struct drm_framebuffer *fb, int crtc_x, int crtc_y,
  

• 	     unsigned int crtc_w, unsigned int crtc_h,
  

• 	     uint32_t src_x, uint32_t src_y,
  

• 	     uint32_t src_w, uint32_t src_h)
  

+{

• struct intel_plane_state state;
• struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
• int ret;
• state.crtc = crtc;
• state.fb = fb;
• /* sample coordinates in 16.16 fixed point */
• state.src.x1 = src_x;
• state.src.x2 = src_x + src_w;
• state.src.y1 = src_y;
• state.src.y2 = src_y + src_h;
• /* integer pixels */
• state.dst.x1 = crtc_x;
• state.dst.x2 = crtc_x + crtc_w;
• state.dst.y1 = crtc_y;
• state.dst.y2 = crtc_y + crtc_h;
• state.clip.x1 = 0;
• state.clip.y1 = 0;
• state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0;
• state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0;

Forgot to populate orig_src/dst here.

• ret = intel_check_primary_plane(plane, &state);
• if (ret)

• return ret;
  

• intel_commit_primary_plane(plane, &state);
• return 0;

+}

/* Common destruction function for both primary and cursor planes */ static void intel_plane_destroy(struct drm_plane *plane) { -- 1.9.3