[PATCH 0/5] drm/i915: Assortment of plane fixes

[PATCH 0/5] drm/i915: Assortment of plane fixes

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

I found another breakage from my SKL plane check stuff when testing out Xv
on SKL, and we still have that 90/270 rotation fix which I already sent out
earlier. I also spotted a harmless bug in the VLV/CHV sprite code so I
figured I'd include the fix for that one as well. And Chris+smatch found
that my error handling was crap, so that too needs fixing. And just for kicks
I tossed in a small cleanup for the gen2/3 DSPADDR calculation.

Entire series available here:
git://github.com/vsyrjala/linux.git plane_fixes

Ville Syrjälä (5):
  drm/i915: Fix scaling check for 90/270 degree plane rotation
  drm/i915: Ignore bogus plane coordinates on SKL when the plane is not
    visible
  drm/i915: Grab the rotation from the passed plane state for VLV
    sprites
  drm/i915: Fix error handling for cursor/sprite plane create failure
  drm/i915: Use intel_fb_gtt_offset() also for gen2/3 primary plane

 drivers/gpu/drm/i915/intel_display.c | 29 ++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
 2 files changed, 19 insertions(+), 12 deletions(-)

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/5] drm/i915: Fix scaling check for 90/270 degree plane rotation

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Starting from commit b63a16f6cd89 ("drm/i915: Compute display surface
offset in the plane check hook for SKL+") we've already rotated the src
coordinates by 270 degrees by the time we check if a scaler is needed
or not, so we must not account for the rotation a second time.
Previously we did these steps in the opposite order and hence the
scaler check had to deal with rotation itself. The double rotation
handling causes us to enable a scaler pretty much every time 90/270
degree plane rotation is requested, leading to fuzzier fonts and whatnot.

Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: drm-intel-fixes@lists.freedesktop.org
Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Fixes: b63a16f6cd89 ("drm/i915: Compute display surface offset in the plane check hook for SKL+")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 92ab01f33208..40f6f08e0732 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4670,7 +4670,7 @@ static void cpt_verify_modeset(struct drm_device *dev, int pipe)
 
 static int
 skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
-		  unsigned scaler_user, int *scaler_id, unsigned int rotation,
+		  unsigned scaler_user, int *scaler_id,
 		  int src_w, int src_h, int dst_w, int dst_h)
 {
 	struct intel_crtc_scaler_state *scaler_state =
@@ -4679,9 +4679,12 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 		to_intel_crtc(crtc_state->base.crtc);
 	int need_scaling;
 
-	need_scaling = drm_rotation_90_or_270(rotation) ?
-		(src_h != dst_w || src_w != dst_h):
-		(src_w != dst_w || src_h != dst_h);
+	/*
+	 * Src coordinates are already rotated by 270 degrees for
+	 * the 90/270 degree plane rotation cases (to match the
+	 * GTT mapping), hence no need to account for rotation here.
+	 */
+	need_scaling = src_w != dst_w || src_h != dst_h;
 
 	/*
 	 * if plane is being disabled or scaler is no more required or force detach
@@ -4748,7 +4751,7 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state)
 		      intel_crtc->pipe, SKL_CRTC_INDEX);
 
 	return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX,
-		&state->scaler_state.scaler_id, DRM_ROTATE_0,
+		&state->scaler_state.scaler_id,
 		state->pipe_src_w, state->pipe_src_h,
 		adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay);
 }
@@ -4782,7 +4785,6 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 	ret = skl_update_scaler(crtc_state, force_detach,
 				drm_plane_index(&intel_plane->base),
 				&plane_state->scaler_id,
-				plane_state->base.rotation,
 				drm_rect_width(&plane_state->base.src) >> 16,
 				drm_rect_height(&plane_state->base.src) >> 16,
 				drm_rect_width(&plane_state->base.dst),
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/5] drm/i915: Ignore bogus plane coordinates on SKL when the plane is not visible

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

When the plane is invisible we may have all sorts of bogus stuff
in the coordinates, which we must ignore or else we might fail the
plane update. This started to happen on SKL when I moved the plane
offset computation to happen in the check phase. Previously we
happily ignored it all since we never called the update_plane hook
with an invisible plane.

Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Cc: drm-intel-fixes@lists.freedesktop.org
Fixes: b63a16f6cd89 ("drm/i915: Compute display surface offset in the plane check hook for SKL+")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 40f6f08e0732..e8c40a69bbc4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2979,6 +2979,9 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state)
 	unsigned int rotation = plane_state->base.rotation;
 	int ret;
 
+	if (!plane_state->base.visible)
+		return 0;
+
 	/* Rotate src coordinates to match rotated GTT view */
 	if (drm_rotation_90_or_270(rotation))
 		drm_rect_rotate(&plane_state->base.src,
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/5] drm/i915: Grab the rotation from the passed plane state for VLV sprites

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use the passed in plane_state instead of plane->state in
vlv_update_plane(). Currently the two are one and the same, but if we
start queuing up multiple plane updates they might not be.

Looks like this was rebase fail on my part.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Fixes: 8d0deca8c6e0 ("drm/i915: Pass 90/270 vs. 0/180 rotation info for intel_gen4_compute_page_offset()")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index df0fbb4b15a3..5e4eb7cafef0 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -362,7 +362,7 @@ vlv_update_plane(struct drm_plane *dplane,
 	int plane = intel_plane->plane;
 	u32 sprctl;
 	u32 sprsurf_offset, linear_offset;
-	unsigned int rotation = dplane->state->rotation;
+	unsigned int rotation = plane_state->base.rotation;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	int crtc_x = plane_state->base.dst.x1;
 	int crtc_y = plane_state->base.dst.y1;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/5] drm/i915: Fix error handling for cursor/sprite plane create failure

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

intel_cursor_plane_create() and intel_sprite_plane_create() return
an error pointer, so let's not mistakenly look for a NULL pointer.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
References: https://lists.freedesktop.org/archives/intel-gfx/2016-November/110690.html
Fixes: b079bd17e301 ("drm/i915: Bail if plane/crtc init fails")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e8c40a69bbc4..817ed3f320f7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15294,14 +15294,14 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 		struct intel_plane *plane;
 
 		plane = intel_sprite_plane_create(dev_priv, pipe, sprite);
-		if (!plane) {
+		if (IS_ERR(plane)) {
 			ret = PTR_ERR(plane);
 			goto fail;
 		}
 	}
 
 	cursor = intel_cursor_plane_create(dev_priv, pipe);
-	if (!cursor) {
+	if (IS_ERR(cursor)) {
 		ret = PTR_ERR(cursor);
 		goto fail;
 	}
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/5] drm/i915: Use intel_fb_gtt_offset() also for gen2/3 primary plane

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The code to determine the primary plane offset for gen2/3 looks
different than the code for gen4+, but in fact it's doing the same
thing. Let's make it uniform. Allows us to eliminate the 'obj' from
the list of local variables as well.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 817ed3f320f7..3d519d701e1c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3017,7 +3017,6 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_framebuffer *fb = plane_state->base.fb;
-	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	int plane = intel_crtc->plane;
 	u32 linear_offset;
 	u32 dspcntr;
@@ -3112,8 +3111,11 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 			   intel_crtc->dspaddr_offset);
 		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
 		I915_WRITE(DSPLINOFF(plane), linear_offset);
-	} else
-		I915_WRITE(DSPADDR(plane), i915_gem_object_ggtt_offset(obj, NULL) + linear_offset);
+	} else {
+		I915_WRITE(DSPADDR(plane),
+			   intel_fb_gtt_offset(fb, rotation) +
+			   intel_crtc->dspaddr_offset);
+	}
 	POSTING_READ(reg);
 }
 
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Assortment of plane fixes

[Patchwork]

== Series Details ==

Series: drm/i915: Assortment of plane fixes
URL   : https://patchwork.freedesktop.org/series/14936/
State : success

== Summary ==

Series 14936v1 drm/i915: Assortment of plane fixes
https://patchwork.freedesktop.org/api/1.0/series/14936/revisions/1/mbox/


fi-bdw-5557u     total:241  pass:226  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:241  pass:201  dwarn:0   dfail:0   fail:0   skip:40 
fi-bxt-t5700     total:241  pass:213  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-j1900     total:241  pass:213  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-n2820     total:241  pass:209  dwarn:0   dfail:0   fail:0   skip:32 
fi-hsw-4770      total:241  pass:221  dwarn:0   dfail:0   fail:0   skip:20 
fi-hsw-4770r     total:241  pass:221  dwarn:0   dfail:0   fail:0   skip:20 
fi-ilk-650       total:241  pass:188  dwarn:0   dfail:0   fail:0   skip:53 
fi-ivb-3520m     total:241  pass:219  dwarn:0   dfail:0   fail:0   skip:22 
fi-ivb-3770      total:241  pass:219  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6260u     total:241  pass:227  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:241  pass:220  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6700k     total:241  pass:219  dwarn:1   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:241  pass:227  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:241  pass:209  dwarn:0   dfail:0   fail:0   skip:32 
fi-snb-2600      total:241  pass:208  dwarn:0   dfail:0   fail:0   skip:33 

fb7fc6fcfc660044f50b98e63ab608afdf4eb1ef drm-intel-nightly: 2016y-11m-07d-17h-05m-46s UTC integration manifest
5f28874 drm/i915: Use intel_fb_gtt_offset() also for gen2/3 primary plane
9a0f5d5 drm/i915: Fix error handling for cursor/sprite plane create failure
81aa895 drm/i915: Grab the rotation from the passed plane state for VLV sprites
191f03b drm/i915: Ignore bogus plane coordinates on SKL when the plane is not visible
3466563 drm/i915: Fix scaling check for 90/270 degree plane rotation

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2926/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/5] drm/i915: Fix error handling for cursor/sprite plane create failure

[Chris Wilson]

On Mon, Nov 07, 2016 at 10:20:56PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> intel_cursor_plane_create() and intel_sprite_plane_create() return
> an error pointer, so let's not mistakenly look for a NULL pointer.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
> References: https://lists.freedesktop.org/archives/intel-gfx/2016-November/110690.html
> Fixes: b079bd17e301 ("drm/i915: Bail if plane/crtc init fails")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] drm/i915: Use intel_fb_gtt_offset() also for gen2/3 primary plane

[Chris Wilson]

On Mon, Nov 07, 2016 at 10:20:57PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The code to determine the primary plane offset for gen2/3 looks
> different than the code for gen4+, but in fact it's doing the same
> thing. Let's make it uniform. Allows us to eliminate the 'obj' from
> the list of local variables as well.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 817ed3f320f7..3d519d701e1c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3017,7 +3017,6 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_framebuffer *fb = plane_state->base.fb;
> -	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	int plane = intel_crtc->plane;
>  	u32 linear_offset;
>  	u32 dspcntr;
> @@ -3112,8 +3111,11 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
>  			   intel_crtc->dspaddr_offset);
>  		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
>  		I915_WRITE(DSPLINOFF(plane), linear_offset);
> -	} else
> -		I915_WRITE(DSPADDR(plane), i915_gem_object_ggtt_offset(obj, NULL) + linear_offset);
> +	} else {
> +		I915_WRITE(DSPADDR(plane),
> +			   intel_fb_gtt_offset(fb, rotation) +
> +			   intel_crtc->dspaddr_offset);

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Though the reader might notice the same offset being passed to both gen4+
DSPSURF and gen3- DSPADDR, and might reasonably ask if they should be
sharing a variable.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/5] drm/i915: Grab the rotation from the passed plane state for VLV sprites

[Chris Wilson]

On Mon, Nov 07, 2016 at 10:20:55PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Use the passed in plane_state instead of plane->state in
> vlv_update_plane(). Currently the two are one and the same, but if we
> start queuing up multiple plane updates they might not be.
> 
> Looks like this was rebase fail on my part.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Fixes: 8d0deca8c6e0 ("drm/i915: Pass 90/270 vs. 0/180 rotation info for intel_gen4_compute_page_offset()")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/5] drm/i915: Fix scaling check for 90/270 degree plane rotation

[Chris Wilson]

On Mon, Nov 07, 2016 at 10:20:53PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Starting from commit b63a16f6cd89 ("drm/i915: Compute display surface
> offset in the plane check hook for SKL+") we've already rotated the src
> coordinates by 270 degrees by the time we check if a scaler is needed
> or not, so we must not account for the rotation a second time.
> Previously we did these steps in the opposite order and hence the
> scaler check had to deal with rotation itself. The double rotation
> handling causes us to enable a scaler pretty much every time 90/270
> degree plane rotation is requested, leading to fuzzier fonts and whatnot.
> 
> Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: drm-intel-fixes@lists.freedesktop.org
> Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Fixes: b63a16f6cd89 ("drm/i915: Compute display surface offset in the plane check hook for SKL+")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I believe

	Tested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

skl_plane_downscale_amount() ?
skl_plane_relative_data_rate() ?
skl_ddb_min_alloc()
skl_compute_plane_wm()

What's the best way of keeping the reader aware of whether or not the
coordinate transform has been applied? Similar to how to let the user
know the rect is in fixed_16_16 or int32.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/5] drm/i915: Fix scaling check for 90/270 degree plane rotation

[Ville Syrjälä]

On Mon, Nov 07, 2016 at 09:28:58PM +0000, Chris Wilson wrote:
> On Mon, Nov 07, 2016 at 10:20:53PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Starting from commit b63a16f6cd89 ("drm/i915: Compute display surface
> > offset in the plane check hook for SKL+") we've already rotated the src
> > coordinates by 270 degrees by the time we check if a scaler is needed
> > or not, so we must not account for the rotation a second time.
> > Previously we did these steps in the opposite order and hence the
> > scaler check had to deal with rotation itself. The double rotation
> > handling causes us to enable a scaler pretty much every time 90/270
> > degree plane rotation is requested, leading to fuzzier fonts and whatnot.
> > 
> > Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: drm-intel-fixes@lists.freedesktop.org
> > Reported-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Fixes: b63a16f6cd89 ("drm/i915: Compute display surface offset in the plane check hook for SKL+")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I believe
> 
> 	Tested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Thanks, forgot to amend it.

> 
> skl_plane_downscale_amount() ?
> skl_plane_relative_data_rate() ?
> skl_ddb_min_alloc()
> skl_compute_plane_wm()

Oh dear. I may have broken the world. Looks like a full blown review
of all uses of the src rect is required.

> What's the best way of keeping the reader aware of whether or not the
> coordinate transform has been applied? Similar to how to let the user
> know the rect is in fixed_16_16 or int32.

That's a good question. I guess for the fixed point stuff we could
define a new type, but then we can't use the same functions to massage
it, which doesn't sound all that nice. Hmm. Maybe we need to add some
metadata to the struct?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] drm/i915: Use intel_fb_gtt_offset() also for gen2/3 primary plane

[Ville Syrjälä]

On Mon, Nov 07, 2016 at 09:20:05PM +0000, Chris Wilson wrote:
> On Mon, Nov 07, 2016 at 10:20:57PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The code to determine the primary plane offset for gen2/3 looks
> > different than the code for gen4+, but in fact it's doing the same
> > thing. Let's make it uniform. Allows us to eliminate the 'obj' from
> > the list of local variables as well.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 817ed3f320f7..3d519d701e1c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3017,7 +3017,6 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> >  	struct drm_framebuffer *fb = plane_state->base.fb;
> > -	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> >  	int plane = intel_crtc->plane;
> >  	u32 linear_offset;
> >  	u32 dspcntr;
> > @@ -3112,8 +3111,11 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
> >  			   intel_crtc->dspaddr_offset);
> >  		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
> >  		I915_WRITE(DSPLINOFF(plane), linear_offset);
> > -	} else
> > -		I915_WRITE(DSPADDR(plane), i915_gem_object_ggtt_offset(obj, NULL) + linear_offset);
> > +	} else {
> > +		I915_WRITE(DSPADDR(plane),
> > +			   intel_fb_gtt_offset(fb, rotation) +
> > +			   intel_crtc->dspaddr_offset);
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Patches 3-5 pushed to dinq. Thanks for the review.

> 
> Though the reader might notice the same offset being passed to both gen4+
> DSPSURF and gen3- DSPADDR, and might reasonably ask if they should be
> sharing a variable.

Perhaps. I think I'll be merging the pre-ilk and ilk codepaths soon,
so I'll see if I can reorganize some of this offset stuff at the same
time a bit.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/5] drm/i915: Ignore bogus plane coordinates on SKL when the plane is not visible

[Matt Roper]

On Mon, Nov 07, 2016 at 10:20:54PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> When the plane is invisible we may have all sorts of bogus stuff
> in the coordinates, which we must ignore or else we might fail the
> plane update. This started to happen on SKL when I moved the plane
> offset computation to happen in the check phase. Previously we
> happily ignored it all since we never called the update_plane hook
> with an invisible plane.
> 
> Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> Cc: drm-intel-fixes@lists.freedesktop.org
> Fixes: b63a16f6cd89 ("drm/i915: Compute display surface offset in the plane check hook for SKL+")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Months later, but

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

and

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98258
Testcase: igt/pm_rpm/legacy-planes
Testcase: igt/pm_rpm/universal-planes


> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 40f6f08e0732..e8c40a69bbc4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2979,6 +2979,9 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state)
>  	unsigned int rotation = plane_state->base.rotation;
>  	int ret;
>  
> +	if (!plane_state->base.visible)
> +		return 0;
> +
>  	/* Rotate src coordinates to match rotated GTT view */
>  	if (drm_rotation_90_or_270(rotation))
>  		drm_rect_rotate(&plane_state->base.src,
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/5] drm/i915: Ignore bogus plane coordinates on SKL when the plane is not visible

[Matt Roper]

On Tue, Jan 17, 2017 at 02:05:07PM -0800, Matt Roper wrote:
> On Mon, Nov 07, 2016 at 10:20:54PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > When the plane is invisible we may have all sorts of bogus stuff
> > in the coordinates, which we must ignore or else we might fail the
> > plane update. This started to happen on SKL when I moved the plane
> > offset computation to happen in the check phase. Previously we
> > happily ignored it all since we never called the update_plane hook
> > with an invisible plane.
> > 
> > Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> > Cc: drm-intel-fixes@lists.freedesktop.org
> > Fixes: b63a16f6cd89 ("drm/i915: Compute display surface offset in the plane check hook for SKL+")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Months later, but
> 
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> 
> and
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98258
> Testcase: igt/pm_rpm/legacy-planes
> Testcase: igt/pm_rpm/universal-planes

Your CI results were clean back when you submitted this, and the
functionally identical patch I posted last week also got clean results,
so pushed to dinq.  Thanks for the patch.


Matt

> 
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 40f6f08e0732..e8c40a69bbc4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2979,6 +2979,9 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state)
> >  	unsigned int rotation = plane_state->base.rotation;
> >  	int ret;
> >  
> > +	if (!plane_state->base.visible)
> > +		return 0;
> > +
> >  	/* Rotate src coordinates to match rotated GTT view */
> >  	if (drm_rotation_90_or_270(rotation))
> >  		drm_rect_rotate(&plane_state->base.src,
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx