[PATCH 0/4] Remainder of 'Nuke legacy state' patch series.

[PATCH 0/4] Remainder of 'Nuke legacy state' patch series.

[Maarten Lankhorst]

Some patches from that series didn't get applied, so resending it
here with some fixes.

First patch moves legacy primary state updates outside of the commit
hook, which leaves only stuff related to plane hw updates inside it.

Patch 2 is a fixup required for patch 3, and patch 4 is a minor
optimization on top which used to be part of patch 3.

Maarten Lankhorst (4):
  drm/i915: Update legacy primary state outside the commit hook, v2.
  drm/i915: Only commit active planes when updating planes during reset.
  drm/i915: Only run commit when crtc is active, v2.
  drm/i915: Only call commit_planes when there are things to commit.

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

-- 
2.1.0

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

[PATCH 1/4] drm/i915: Update legacy primary state outside the commit hook, v2.

[Maarten Lankhorst]

This should allow not running plane commit when the crtc is off.
While the atomic helpers update those, crtc->x/y is only updated
during modesets, and primary plane is updated after this function
returns.

Unfortunately non-atomic watermarks and fbc still depend on this
state inside i915, so it has to be kept in sync.

Changes since v1:
- Add comment that the legacy state is updated for fbc.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2a1fab3eb285..b8cd2aec3bf8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12206,6 +12206,18 @@ intel_modeset_update_crtc_state(struct drm_atomic_state *state)
 			crtc->hwmode = crtc->state->adjusted_mode;
 		else
 			crtc->hwmode.crtc_clock = 0;
+
+		/*
+		 * Update legacy state to satisfy fbc code. This can
+		 * be removed when fbc uses the atomic state.
+		 */
+		if (drm_atomic_get_existing_plane_state(state, crtc->primary)) {
+			struct drm_plane_state *plane_state = crtc->primary->state;
+
+			crtc->primary->fb = plane_state->fb;
+			crtc->x = plane_state->src_x >> 16;
+			crtc->y = plane_state->src_y >> 16;
+		}
 	}
 }
 
@@ -13442,15 +13454,8 @@ intel_commit_primary_plane(struct drm_plane *plane,
 	struct drm_framebuffer *fb = state->base.fb;
 	struct drm_device *dev = plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc;
-	struct drm_rect *src = &state->src;
 
 	crtc = crtc ? crtc : plane->crtc;
-	intel_crtc = to_intel_crtc(crtc);
-
-	plane->fb = fb;
-	crtc->x = src->x1 >> 16;
-	crtc->y = src->y1 >> 16;
 
 	if (!crtc->state->active)
 		return;
-- 
2.1.0

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

[PATCH 2/4] drm/i915: Only commit active planes when updating planes during reset.

[Maarten Lankhorst]

In the next commit commit_plane will no longer check if the crtc is active.
To prevent issues with legacy page flips the check should be performed inside
update_primary_planes.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b8cd2aec3bf8..e7b094c22e47 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3154,10 +3154,9 @@ static void intel_update_primary_planes(struct drm_device *dev)
 		struct intel_plane_state *plane_state;
 
 		drm_modeset_lock_crtc(crtc, &plane->base);
-
 		plane_state = to_intel_plane_state(plane->base.state);
 
-		if (plane_state->base.fb)
+		if (crtc->state->active && plane_state->base.fb)
 			plane->commit_plane(&plane->base, plane_state);
 
 		drm_modeset_unlock_crtc(crtc);
-- 
2.1.0

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

[PATCH 3/4] drm/i915: Only run commit when crtc is active, v2.

[Maarten Lankhorst]

The crtc->active guards are no longer needed now that all state
updates are outside the commit.

Changes since v1:
- Only check crtc->state->active before calling commit_planes_on_crtc.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 15 +++++----------
 drivers/gpu/drm/i915/intel_sprite.c  |  3 ---
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e7b094c22e47..f64ea430b976 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13146,7 +13146,8 @@ static int intel_atomic_commit(struct drm_device *dev,
 		if (!modeset)
 			intel_pre_plane_update(intel_crtc);
 
-		drm_atomic_helper_commit_planes_on_crtc(crtc_state);
+		if (crtc->state->active)
+			drm_atomic_helper_commit_planes_on_crtc(crtc_state);
 
 		if (put_domains)
 			modeset_put_power_domains(dev_priv, put_domains);
@@ -13456,9 +13457,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
 
 	crtc = crtc ? crtc : plane->crtc;
 
-	if (!crtc->state->active)
-		return;
-
 	dev_priv->display.update_primary_plane(crtc, fb,
 					       state->src.x1 >> 16,
 					       state->src.y1 >> 16);
@@ -13487,8 +13485,7 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
 		intel_update_watermarks(crtc);
 
 	/* Perform vblank evasion around commit operation */
-	if (crtc->state->active)
-		intel_pipe_update_start(intel_crtc);
+	intel_pipe_update_start(intel_crtc);
 
 	if (modeset)
 		return;
@@ -13504,8 +13501,7 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc,
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	if (crtc->state->active)
-		intel_pipe_update_end(intel_crtc);
+	intel_pipe_update_end(intel_crtc);
 }
 
 /**
@@ -13688,8 +13684,7 @@ intel_commit_cursor_plane(struct drm_plane *plane,
 	intel_crtc->cursor_bo = obj;
 
 update:
-	if (crtc->state->active)
-		intel_crtc_update_cursor(crtc, state->visible);
+	intel_crtc_update_cursor(crtc, state->visible);
 }
 
 static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 79f7cc247ab7..bb54b5b787c3 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -935,9 +935,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
 
 	crtc = crtc ? crtc : plane->crtc;
 
-	if (!crtc->state->active)
-		return;
-
 	if (state->visible) {
 		intel_plane->update_plane(plane, crtc, fb,
 					  state->dst.x1, state->dst.y1,
-- 
2.1.0

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

[PATCH 4/4] drm/i915: Only call commit_planes when there are things to commit.

[Maarten Lankhorst]

The atomic helpers set planes_changed on a crtc_state if there is
any plane_state bound to that crtc. If there's none and there is
no pipe update required the crtc has nothing to update, so vblank
evasion can be skipped.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f64ea430b976..9754ee2bb37c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13146,7 +13146,8 @@ static int intel_atomic_commit(struct drm_device *dev,
 		if (!modeset)
 			intel_pre_plane_update(intel_crtc);
 
-		if (crtc->state->active)
+		if (crtc->state->active &&
+		    (crtc->state->planes_changed || update_pipe))
 			drm_atomic_helper_commit_planes_on_crtc(crtc_state);
 
 		if (put_domains)
-- 
2.1.0

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

Re: [PATCH 4/4] drm/i915: Only call commit_planes when there are things to commit.

[Ville Syrjälä]

On Wed, Sep 23, 2015 at 04:29:39PM +0200, Maarten Lankhorst wrote:
> The atomic helpers set planes_changed on a crtc_state if there is
> any plane_state bound to that crtc. If there's none and there is
> no pipe update required the crtc has nothing to update, so vblank
> evasion can be skipped.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f64ea430b976..9754ee2bb37c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13146,7 +13146,8 @@ static int intel_atomic_commit(struct drm_device *dev,
>  		if (!modeset)
>  			intel_pre_plane_update(intel_crtc);
>  
> -		if (crtc->state->active)
> +		if (crtc->state->active &&
> +		    (crtc->state->planes_changed || update_pipe))
>  			drm_atomic_helper_commit_planes_on_crtc(crtc_state);
>  
>  		if (put_domains)

Series lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

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

Re: [PATCH 4/4] drm/i915: Only call commit_planes when there are things to commit.

[Daniel Vetter]

On Fri, Oct 16, 2015 at 03:47:37PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 23, 2015 at 04:29:39PM +0200, Maarten Lankhorst wrote:
> > The atomic helpers set planes_changed on a crtc_state if there is
> > any plane_state bound to that crtc. If there's none and there is
> > no pipe update required the crtc has nothing to update, so vblank
> > evasion can be skipped.
> > 
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index f64ea430b976..9754ee2bb37c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13146,7 +13146,8 @@ static int intel_atomic_commit(struct drm_device *dev,
> >  		if (!modeset)
> >  			intel_pre_plane_update(intel_crtc);
> >  
> > -		if (crtc->state->active)
> > +		if (crtc->state->active &&
> > +		    (crtc->state->planes_changed || update_pipe))
> >  			drm_atomic_helper_commit_planes_on_crtc(crtc_state);
> >  
> >  		if (put_domains)
> 
> Series lgtm
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

All four merged to dinq, thanks.
-Daniel

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx