[PATCH 1/5] drm: Add __drm_atomic_helper_crtc_state_reset() & co.

[PATCH 1/5] drm: Add __drm_atomic_helper_crtc_state_reset() & co.

[Ville Syrjala]

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

Annoyingly __drm_atomic_helper_crtc_reset() does two
totally separate things:
a) reset the state to defaults values
b) assign the crtc->state pointer

I just want a) without the b) so let's split out part
a) into __drm_atomic_helper_crtc_state_reset(). And
of course we'll do the same thing for planes and connectors.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 70 ++++++++++++++++++++---
 include/drm/drm_atomic_state_helper.h     |  6 ++
 2 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index d0a937fb0c56..a972068d58cf 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -57,6 +57,22 @@
  * for these functions.
  */
 
+/**
+ * __drm_atomic_helper_crtc_state_reset - reset the CRTC state
+ * @crtc_state: atomic CRTC state, must not be NULL
+ * @crtc: CRTC object, must not be NULL
+ *
+ * Initializes the newly allocated @crtc_state with default
+ * values. This is useful for drivers that subclass the CRTC state.
+ */
+void
+__drm_atomic_helper_crtc_state_reset(struct drm_crtc_state *crtc_state,
+				     struct drm_crtc *crtc)
+{
+	crtc_state->crtc = crtc;
+}
+EXPORT_SYMBOL(__drm_atomic_helper_crtc_state_reset);
+
 /**
  * __drm_atomic_helper_crtc_reset - reset state on CRTC
  * @crtc: drm CRTC
@@ -74,7 +90,7 @@ __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
 			       struct drm_crtc_state *crtc_state)
 {
 	if (crtc_state)
-		crtc_state->crtc = crtc;
+		__drm_atomic_helper_crtc_state_reset(crtc_state, crtc);
 
 	crtc->state = crtc_state;
 }
@@ -212,23 +228,43 @@ void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
 EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
 
 /**
- * __drm_atomic_helper_plane_reset - resets planes state to default values
+ * __drm_atomic_helper_plane_state_reset - resets plane state to default values
+ * @plane_state: atomic plane state, must not be NULL
  * @plane: plane object, must not be NULL
- * @state: atomic plane state, must not be NULL
  *
- * Initializes plane state to default. This is useful for drivers that subclass
- * the plane state.
+ * Initializes the newly allocated @plane_state with default
+ * values. This is useful for drivers that subclass the CRTC state.
  */
-void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
-				     struct drm_plane_state *state)
+void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *state,
+					   struct drm_plane *plane)
 {
 	state->plane = plane;
 	state->rotation = DRM_MODE_ROTATE_0;
 
 	state->alpha = DRM_BLEND_ALPHA_OPAQUE;
 	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
+}
+EXPORT_SYMBOL(__drm_atomic_helper_plane_state_reset);
 
-	plane->state = state;
+/**
+ * __drm_atomic_helper_plane_reset - reset state on plane
+ * @plane: drm plane
+ * @plane_state: plane state to assign
+ *
+ * Initializes the newly allocated @plane_state and assigns it to
+ * the &drm_crtc->state pointer of @plane, usually required when
+ * initializing the drivers or when called from the &drm_plane_funcs.reset
+ * hook.
+ *
+ * This is useful for drivers that subclass the plane state.
+ */
+void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
+				     struct drm_plane_state *plane_state)
+{
+	if (plane_state)
+		__drm_atomic_helper_plane_state_reset(plane_state, plane);
+
+	plane->state = plane_state;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_reset);
 
@@ -335,6 +371,22 @@ void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,
 }
 EXPORT_SYMBOL(drm_atomic_helper_plane_destroy_state);
 
+/**
+ * __drm_atomic_helper_connector_state_reset - reset the connector state
+ * @conn__state: atomic connector state, must not be NULL
+ * @connector: connectotr object, must not be NULL
+ *
+ * Initializes the newly allocated @conn_state with default
+ * values. This is useful for drivers that subclass the connector state.
+ */
+void
+__drm_atomic_helper_connector_state_reset(struct drm_connector_state *conn_state,
+					  struct drm_connector *connector)
+{
+	conn_state->connector = connector;
+}
+EXPORT_SYMBOL(__drm_atomic_helper_connector_state_reset);
+
 /**
  * __drm_atomic_helper_connector_reset - reset state on connector
  * @connector: drm connector
@@ -352,7 +404,7 @@ __drm_atomic_helper_connector_reset(struct drm_connector *connector,
 				    struct drm_connector_state *conn_state)
 {
 	if (conn_state)
-		conn_state->connector = connector;
+		__drm_atomic_helper_connector_state_reset(conn_state, connector);
 
 	connector->state = conn_state;
 }
diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
index e4577cc11689..8171dea4cc22 100644
--- a/include/drm/drm_atomic_state_helper.h
+++ b/include/drm/drm_atomic_state_helper.h
@@ -37,6 +37,8 @@ struct drm_private_state;
 struct drm_modeset_acquire_ctx;
 struct drm_device;
 
+void __drm_atomic_helper_crtc_state_reset(struct drm_crtc_state *state,
+					  struct drm_crtc *crtc);
 void __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
 				    struct drm_crtc_state *state);
 void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc);
@@ -48,6 +50,8 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state);
 void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
 					  struct drm_crtc_state *state);
 
+void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *state,
+					   struct drm_plane *plane);
 void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
 				     struct drm_plane_state *state);
 void drm_atomic_helper_plane_reset(struct drm_plane *plane);
@@ -59,6 +63,8 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state);
 void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,
 					  struct drm_plane_state *state);
 
+void __drm_atomic_helper_connector_state_reset(struct drm_connector_state *conn_state,
+					       struct drm_connector *connector);
 void __drm_atomic_helper_connector_reset(struct drm_connector *connector,
 					 struct drm_connector_state *conn_state);
 void drm_atomic_helper_connector_reset(struct drm_connector *connector);
-- 
2.23.0

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

[PATCH 1/5] drm: Add __drm_atomic_helper_crtc_state_reset() & co.

[Ville Syrjala]

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

Annoyingly __drm_atomic_helper_crtc_reset() does two
totally separate things:
a) reset the state to defaults values
b) assign the crtc->state pointer

I just want a) without the b) so let's split out part
a) into __drm_atomic_helper_crtc_state_reset(). And
of course we'll do the same thing for planes and connectors.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 70 ++++++++++++++++++++---
 include/drm/drm_atomic_state_helper.h     |  6 ++
 2 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index d0a937fb0c56..a972068d58cf 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -57,6 +57,22 @@
  * for these functions.
  */
 
+/**
+ * __drm_atomic_helper_crtc_state_reset - reset the CRTC state
+ * @crtc_state: atomic CRTC state, must not be NULL
+ * @crtc: CRTC object, must not be NULL
+ *
+ * Initializes the newly allocated @crtc_state with default
+ * values. This is useful for drivers that subclass the CRTC state.
+ */
+void
+__drm_atomic_helper_crtc_state_reset(struct drm_crtc_state *crtc_state,
+				     struct drm_crtc *crtc)
+{
+	crtc_state->crtc = crtc;
+}
+EXPORT_SYMBOL(__drm_atomic_helper_crtc_state_reset);
+
 /**
  * __drm_atomic_helper_crtc_reset - reset state on CRTC
  * @crtc: drm CRTC
@@ -74,7 +90,7 @@ __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
 			       struct drm_crtc_state *crtc_state)
 {
 	if (crtc_state)
-		crtc_state->crtc = crtc;
+		__drm_atomic_helper_crtc_state_reset(crtc_state, crtc);
 
 	crtc->state = crtc_state;
 }
@@ -212,23 +228,43 @@ void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
 EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
 
 /**
- * __drm_atomic_helper_plane_reset - resets planes state to default values
+ * __drm_atomic_helper_plane_state_reset - resets plane state to default values
+ * @plane_state: atomic plane state, must not be NULL
  * @plane: plane object, must not be NULL
- * @state: atomic plane state, must not be NULL
  *
- * Initializes plane state to default. This is useful for drivers that subclass
- * the plane state.
+ * Initializes the newly allocated @plane_state with default
+ * values. This is useful for drivers that subclass the CRTC state.
  */
-void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
-				     struct drm_plane_state *state)
+void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *state,
+					   struct drm_plane *plane)
 {
 	state->plane = plane;
 	state->rotation = DRM_MODE_ROTATE_0;
 
 	state->alpha = DRM_BLEND_ALPHA_OPAQUE;
 	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
+}
+EXPORT_SYMBOL(__drm_atomic_helper_plane_state_reset);
 
-	plane->state = state;
+/**
+ * __drm_atomic_helper_plane_reset - reset state on plane
+ * @plane: drm plane
+ * @plane_state: plane state to assign
+ *
+ * Initializes the newly allocated @plane_state and assigns it to
+ * the &drm_crtc->state pointer of @plane, usually required when
+ * initializing the drivers or when called from the &drm_plane_funcs.reset
+ * hook.
+ *
+ * This is useful for drivers that subclass the plane state.
+ */
+void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
+				     struct drm_plane_state *plane_state)
+{
+	if (plane_state)
+		__drm_atomic_helper_plane_state_reset(plane_state, plane);
+
+	plane->state = plane_state;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_reset);
 
@@ -335,6 +371,22 @@ void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,
 }
 EXPORT_SYMBOL(drm_atomic_helper_plane_destroy_state);
 
+/**
+ * __drm_atomic_helper_connector_state_reset - reset the connector state
+ * @conn__state: atomic connector state, must not be NULL
+ * @connector: connectotr object, must not be NULL
+ *
+ * Initializes the newly allocated @conn_state with default
+ * values. This is useful for drivers that subclass the connector state.
+ */
+void
+__drm_atomic_helper_connector_state_reset(struct drm_connector_state *conn_state,
+					  struct drm_connector *connector)
+{
+	conn_state->connector = connector;
+}
+EXPORT_SYMBOL(__drm_atomic_helper_connector_state_reset);
+
 /**
  * __drm_atomic_helper_connector_reset - reset state on connector
  * @connector: drm connector
@@ -352,7 +404,7 @@ __drm_atomic_helper_connector_reset(struct drm_connector *connector,
 				    struct drm_connector_state *conn_state)
 {
 	if (conn_state)
-		conn_state->connector = connector;
+		__drm_atomic_helper_connector_state_reset(conn_state, connector);
 
 	connector->state = conn_state;
 }
diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
index e4577cc11689..8171dea4cc22 100644
--- a/include/drm/drm_atomic_state_helper.h
+++ b/include/drm/drm_atomic_state_helper.h
@@ -37,6 +37,8 @@ struct drm_private_state;
 struct drm_modeset_acquire_ctx;
 struct drm_device;
 
+void __drm_atomic_helper_crtc_state_reset(struct drm_crtc_state *state,
+					  struct drm_crtc *crtc);
 void __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
 				    struct drm_crtc_state *state);
 void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc);
@@ -48,6 +50,8 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state);
 void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
 					  struct drm_crtc_state *state);
 
+void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *state,
+					   struct drm_plane *plane);
 void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
 				     struct drm_plane_state *state);
 void drm_atomic_helper_plane_reset(struct drm_plane *plane);
@@ -59,6 +63,8 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state);
 void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,
 					  struct drm_plane_state *state);
 
+void __drm_atomic_helper_connector_state_reset(struct drm_connector_state *conn_state,
+					       struct drm_connector *connector);
 void __drm_atomic_helper_connector_reset(struct drm_connector *connector,
 					 struct drm_connector_state *conn_state);
 void drm_atomic_helper_connector_reset(struct drm_connector *connector);
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/5] drm/i915: s/intel_crtc/crtc/ in intel_crtc_init()

[Ville Syrjala]

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

Let's get rid of the redundant intel_ prefix on our variables.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 551de2baa569..8b889c9f29b5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15838,14 +15838,14 @@ static const struct drm_crtc_funcs i8xx_crtc_funcs = {
 static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 {
 	const struct drm_crtc_funcs *funcs;
-	struct intel_crtc *intel_crtc;
+	struct intel_crtc *crtc;
 	struct intel_crtc_state *crtc_state = NULL;
 	struct intel_plane *primary = NULL;
 	struct intel_plane *cursor = NULL;
 	int sprite, ret;
 
-	intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL);
-	if (!intel_crtc)
+	crtc = kzalloc(sizeof(*crtc), GFP_KERNEL);
+	if (!crtc)
 		return -ENOMEM;
 
 	crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
@@ -15853,15 +15853,15 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 		ret = -ENOMEM;
 		goto fail;
 	}
-	__drm_atomic_helper_crtc_reset(&intel_crtc->base, &crtc_state->uapi);
-	intel_crtc->config = crtc_state;
+	__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->uapi);
+	crtc->config = crtc_state;
 
 	primary = intel_primary_plane_create(dev_priv, pipe);
 	if (IS_ERR(primary)) {
 		ret = PTR_ERR(primary);
 		goto fail;
 	}
-	intel_crtc->plane_ids_mask |= BIT(primary->id);
+	crtc->plane_ids_mask |= BIT(primary->id);
 
 	for_each_sprite(dev_priv, pipe, sprite) {
 		struct intel_plane *plane;
@@ -15871,7 +15871,7 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 			ret = PTR_ERR(plane);
 			goto fail;
 		}
-		intel_crtc->plane_ids_mask |= BIT(plane->id);
+		crtc->plane_ids_mask |= BIT(plane->id);
 	}
 
 	cursor = intel_cursor_plane_create(dev_priv, pipe);
@@ -15879,7 +15879,7 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 		ret = PTR_ERR(cursor);
 		goto fail;
 	}
-	intel_crtc->plane_ids_mask |= BIT(cursor->id);
+	crtc->plane_ids_mask |= BIT(cursor->id);
 
 	if (HAS_GMCH(dev_priv)) {
 		if (IS_CHERRYVIEW(dev_priv) ||
@@ -15900,32 +15900,32 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 			funcs = &ilk_crtc_funcs;
 	}
 
-	ret = drm_crtc_init_with_planes(&dev_priv->drm, &intel_crtc->base,
+	ret = drm_crtc_init_with_planes(&dev_priv->drm, &crtc->base,
 					&primary->base, &cursor->base,
 					funcs, "pipe %c", pipe_name(pipe));
 	if (ret)
 		goto fail;
 
-	intel_crtc->pipe = pipe;
+	crtc->pipe = pipe;
 
 	/* initialize shared scalers */
-	intel_crtc_init_scalers(intel_crtc, crtc_state);
+	intel_crtc_init_scalers(crtc, crtc_state);
 
 	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->pipe_to_crtc_mapping) ||
 	       dev_priv->pipe_to_crtc_mapping[pipe] != NULL);
-	dev_priv->pipe_to_crtc_mapping[pipe] = intel_crtc;
+	dev_priv->pipe_to_crtc_mapping[pipe] = crtc;
 
 	if (INTEL_GEN(dev_priv) < 9) {
 		enum i9xx_plane_id i9xx_plane = primary->i9xx_plane;
 
 		BUG_ON(i9xx_plane >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
 		       dev_priv->plane_to_crtc_mapping[i9xx_plane] != NULL);
-		dev_priv->plane_to_crtc_mapping[i9xx_plane] = intel_crtc;
+		dev_priv->plane_to_crtc_mapping[i9xx_plane] = crtc;
 	}
 
-	intel_color_init(intel_crtc);
+	intel_color_init(crtc);
 
-	WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
+	WARN_ON(drm_crtc_index(&crtc->base) != crtc->pipe);
 
 	return 0;
 
@@ -15935,7 +15935,7 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 	 * crtcs/planes already initialized.
 	 */
 	kfree(crtc_state);
-	kfree(intel_crtc);
+	kfree(crtc);
 
 	return ret;
 }
-- 
2.23.0

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

[PATCH 2/5] drm/i915: s/intel_crtc/crtc/ in intel_crtc_init()

[Ville Syrjala]

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

Let's get rid of the redundant intel_ prefix on our variables.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 551de2baa569..8b889c9f29b5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15838,14 +15838,14 @@ static const struct drm_crtc_funcs i8xx_crtc_funcs = {
 static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 {
 	const struct drm_crtc_funcs *funcs;
-	struct intel_crtc *intel_crtc;
+	struct intel_crtc *crtc;
 	struct intel_crtc_state *crtc_state = NULL;
 	struct intel_plane *primary = NULL;
 	struct intel_plane *cursor = NULL;
 	int sprite, ret;
 
-	intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL);
-	if (!intel_crtc)
+	crtc = kzalloc(sizeof(*crtc), GFP_KERNEL);
+	if (!crtc)
 		return -ENOMEM;
 
 	crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
@@ -15853,15 +15853,15 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 		ret = -ENOMEM;
 		goto fail;
 	}
-	__drm_atomic_helper_crtc_reset(&intel_crtc->base, &crtc_state->uapi);
-	intel_crtc->config = crtc_state;
+	__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->uapi);
+	crtc->config = crtc_state;
 
 	primary = intel_primary_plane_create(dev_priv, pipe);
 	if (IS_ERR(primary)) {
 		ret = PTR_ERR(primary);
 		goto fail;
 	}
-	intel_crtc->plane_ids_mask |= BIT(primary->id);
+	crtc->plane_ids_mask |= BIT(primary->id);
 
 	for_each_sprite(dev_priv, pipe, sprite) {
 		struct intel_plane *plane;
@@ -15871,7 +15871,7 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 			ret = PTR_ERR(plane);
 			goto fail;
 		}
-		intel_crtc->plane_ids_mask |= BIT(plane->id);
+		crtc->plane_ids_mask |= BIT(plane->id);
 	}
 
 	cursor = intel_cursor_plane_create(dev_priv, pipe);
@@ -15879,7 +15879,7 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 		ret = PTR_ERR(cursor);
 		goto fail;
 	}
-	intel_crtc->plane_ids_mask |= BIT(cursor->id);
+	crtc->plane_ids_mask |= BIT(cursor->id);
 
 	if (HAS_GMCH(dev_priv)) {
 		if (IS_CHERRYVIEW(dev_priv) ||
@@ -15900,32 +15900,32 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 			funcs = &ilk_crtc_funcs;
 	}
 
-	ret = drm_crtc_init_with_planes(&dev_priv->drm, &intel_crtc->base,
+	ret = drm_crtc_init_with_planes(&dev_priv->drm, &crtc->base,
 					&primary->base, &cursor->base,
 					funcs, "pipe %c", pipe_name(pipe));
 	if (ret)
 		goto fail;
 
-	intel_crtc->pipe = pipe;
+	crtc->pipe = pipe;
 
 	/* initialize shared scalers */
-	intel_crtc_init_scalers(intel_crtc, crtc_state);
+	intel_crtc_init_scalers(crtc, crtc_state);
 
 	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->pipe_to_crtc_mapping) ||
 	       dev_priv->pipe_to_crtc_mapping[pipe] != NULL);
-	dev_priv->pipe_to_crtc_mapping[pipe] = intel_crtc;
+	dev_priv->pipe_to_crtc_mapping[pipe] = crtc;
 
 	if (INTEL_GEN(dev_priv) < 9) {
 		enum i9xx_plane_id i9xx_plane = primary->i9xx_plane;
 
 		BUG_ON(i9xx_plane >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
 		       dev_priv->plane_to_crtc_mapping[i9xx_plane] != NULL);
-		dev_priv->plane_to_crtc_mapping[i9xx_plane] = intel_crtc;
+		dev_priv->plane_to_crtc_mapping[i9xx_plane] = crtc;
 	}
 
-	intel_color_init(intel_crtc);
+	intel_color_init(crtc);
 
-	WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
+	WARN_ON(drm_crtc_index(&crtc->base) != crtc->pipe);
 
 	return 0;
 
@@ -15935,7 +15935,7 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 	 * crtcs/planes already initialized.
 	 */
 	kfree(crtc_state);
-	kfree(intel_crtc);
+	kfree(crtc);
 
 	return ret;
 }
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/5] drm/i915: Introduce intel_crtc_{alloc,free}()

[Ville Syrjala]

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

We already have alloc/free helpers for planes, add the same for
crtcs. The main benefit is we get to move all the annoying state
initialization out of the main crtc_init() flow.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 8b889c9f29b5..e6291841053f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -164,8 +164,7 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
 static void chv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
-static void intel_crtc_init_scalers(struct intel_crtc *crtc,
-				    struct intel_crtc_state *crtc_state);
+static void intel_crtc_init_scalers(struct intel_crtc_state *crtc_state);
 static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state);
 static void ironlake_pfit_disable(const struct intel_crtc_state *old_crtc_state);
 static void ironlake_pfit_enable(const struct intel_crtc_state *crtc_state);
@@ -10656,7 +10655,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	u64 power_domain_mask;
 	bool active;
 
-	intel_crtc_init_scalers(crtc, pipe_config);
+	intel_crtc_init_scalers(pipe_config);
 
 	pipe_config->master_transcoder = INVALID_TRANSCODER;
 
@@ -15746,25 +15745,12 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 	return ERR_PTR(ret);
 }
 
-static void intel_crtc_init_scalers(struct intel_crtc *crtc,
-				    struct intel_crtc_state *crtc_state)
+static void intel_crtc_init_scalers(struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc_scaler_state *scaler_state =
 		&crtc_state->scaler_state;
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	int i;
-
-	crtc->num_scalers = RUNTIME_INFO(dev_priv)->num_scalers[crtc->pipe];
-	if (!crtc->num_scalers)
-		return;
-
-	for (i = 0; i < crtc->num_scalers; i++) {
-		struct intel_scaler *scaler = &scaler_state->scalers[i];
-
-		scaler->in_use = 0;
-		scaler->mode = 0;
-	}
 
+	memset(scaler_state, 0, sizeof(*scaler_state));
 	scaler_state->scaler_id = -1;
 }
 
@@ -15835,27 +15821,49 @@ static const struct drm_crtc_funcs i8xx_crtc_funcs = {
 	.disable_vblank = i8xx_disable_vblank,
 };
 
-static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
+static struct intel_crtc *intel_crtc_alloc(void)
 {
-	const struct drm_crtc_funcs *funcs;
+	struct intel_crtc_state *crtc_state;
 	struct intel_crtc *crtc;
-	struct intel_crtc_state *crtc_state = NULL;
-	struct intel_plane *primary = NULL;
-	struct intel_plane *cursor = NULL;
-	int sprite, ret;
 
 	crtc = kzalloc(sizeof(*crtc), GFP_KERNEL);
 	if (!crtc)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
 	if (!crtc_state) {
-		ret = -ENOMEM;
-		goto fail;
+		kfree(crtc);
+		return ERR_PTR(-ENOMEM);
 	}
+
 	__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->uapi);
+	intel_crtc_init_scalers(crtc_state);
+
 	crtc->config = crtc_state;
 
+	return crtc;
+}
+
+static void intel_crtc_free(struct intel_crtc *crtc)
+{
+	intel_crtc_destroy_state(&crtc->base, crtc->base.state);
+	kfree(crtc);
+}
+
+static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
+{
+	struct intel_plane *primary, *cursor;
+	const struct drm_crtc_funcs *funcs;
+	struct intel_crtc *crtc;
+	int sprite, ret;
+
+	crtc = intel_crtc_alloc();
+	if (IS_ERR(crtc))
+		return PTR_ERR(crtc);
+
+	crtc->pipe = pipe;
+	crtc->num_scalers = RUNTIME_INFO(dev_priv)->num_scalers[pipe];
+
 	primary = intel_primary_plane_create(dev_priv, pipe);
 	if (IS_ERR(primary)) {
 		ret = PTR_ERR(primary);
@@ -15906,11 +15914,6 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 	if (ret)
 		goto fail;
 
-	crtc->pipe = pipe;
-
-	/* initialize shared scalers */
-	intel_crtc_init_scalers(crtc, crtc_state);
-
 	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->pipe_to_crtc_mapping) ||
 	       dev_priv->pipe_to_crtc_mapping[pipe] != NULL);
 	dev_priv->pipe_to_crtc_mapping[pipe] = crtc;
@@ -15930,12 +15933,7 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 	return 0;
 
 fail:
-	/*
-	 * drm_mode_config_cleanup() will free up any
-	 * crtcs/planes already initialized.
-	 */
-	kfree(crtc_state);
-	kfree(crtc);
+	intel_crtc_free(crtc);
 
 	return ret;
 }
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/5] drm/i915: Introduce intel_crtc_state_reset()

[Ville Syrjala]

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

We have a few places where we want to reset a crtc state to its
default values. Let's add a helper for that. We'll need the new
__drm_atomic_helper_crtc_state_reset() helper for this to allow
us to just reset the state itself without clobbering the
crtc->state pointer.

And while at it let's zero out the whole thing, except a few
choice member which we'll mark as "invalid". And thanks to this
we can now nuke intel_crtc_init_scalers().

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index e6291841053f..fd4120533c3f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -164,7 +164,6 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
 static void chv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
-static void intel_crtc_init_scalers(struct intel_crtc_state *crtc_state);
 static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state);
 static void ironlake_pfit_disable(const struct intel_crtc_state *old_crtc_state);
 static void ironlake_pfit_enable(const struct intel_crtc_state *crtc_state);
@@ -10655,8 +10654,6 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	u64 power_domain_mask;
 	bool active;
 
-	intel_crtc_init_scalers(pipe_config);
-
 	pipe_config->master_transcoder = INVALID_TRANSCODER;
 
 	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
@@ -11704,6 +11701,20 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
 					 &pipe_config->fdi_m_n);
 }
 
+static void intel_crtc_state_reset(struct intel_crtc_state *crtc_state,
+				   struct intel_crtc *crtc)
+{
+	memset(crtc_state, 0, sizeof(*crtc_state));
+
+	__drm_atomic_helper_crtc_state_reset(&crtc_state->uapi, &crtc->base);
+
+	crtc_state->cpu_transcoder = INVALID_TRANSCODER;
+	crtc_state->master_transcoder = INVALID_TRANSCODER;
+	crtc_state->hsw_workaround_pipe = INVALID_PIPE;
+	crtc_state->output_format = INTEL_OUTPUT_FORMAT_INVALID;
+	crtc_state->scaler_state.scaler_id = -1;
+}
+
 /* Returns the currently programmed mode of the given encoder. */
 struct drm_display_mode *
 intel_encoder_current_mode(struct intel_encoder *encoder)
@@ -11729,7 +11740,7 @@ intel_encoder_current_mode(struct intel_encoder *encoder)
 		return NULL;
 	}
 
-	crtc_state->uapi.crtc = &crtc->base;
+	intel_crtc_state_reset(crtc_state, crtc);
 
 	if (!dev_priv->display.get_pipe_config(crtc, crtc_state)) {
 		kfree(crtc_state);
@@ -13577,18 +13588,14 @@ verify_crtc_state(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_encoder *encoder;
-	struct intel_crtc_state *pipe_config;
-	struct drm_atomic_state *state;
+	struct intel_crtc_state *pipe_config = old_crtc_state;
+	struct drm_atomic_state *state = old_crtc_state->uapi.state;
 	bool active;
 
-	state = old_crtc_state->uapi.state;
 	__drm_atomic_helper_crtc_destroy_state(&old_crtc_state->uapi);
 	intel_crtc_free_hw_state(old_crtc_state);
-
-	pipe_config = old_crtc_state;
-	memset(pipe_config, 0, sizeof(*pipe_config));
-	pipe_config->uapi.crtc = &crtc->base;
-	pipe_config->uapi.state = state;
+	intel_crtc_state_reset(old_crtc_state, crtc);
+	old_crtc_state->uapi.state = state;
 
 	DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.base.id, crtc->base.name);
 
@@ -15745,15 +15752,6 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 	return ERR_PTR(ret);
 }
 
-static void intel_crtc_init_scalers(struct intel_crtc_state *crtc_state)
-{
-	struct intel_crtc_scaler_state *scaler_state =
-		&crtc_state->scaler_state;
-
-	memset(scaler_state, 0, sizeof(*scaler_state));
-	scaler_state->scaler_id = -1;
-}
-
 #define INTEL_CRTC_FUNCS \
 	.gamma_set = drm_atomic_helper_legacy_gamma_set, \
 	.set_config = drm_atomic_helper_set_config, \
@@ -15836,9 +15834,9 @@ static struct intel_crtc *intel_crtc_alloc(void)
 		return ERR_PTR(-ENOMEM);
 	}
 
-	__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->uapi);
-	intel_crtc_init_scalers(crtc_state);
+	intel_crtc_state_reset(crtc_state, crtc);
 
+	crtc->base.state = &crtc_state->uapi;
 	crtc->config = crtc_state;
 
 	return crtc;
@@ -17414,8 +17412,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 
 		__drm_atomic_helper_crtc_destroy_state(&crtc_state->uapi);
 		intel_crtc_free_hw_state(crtc_state);
-		memset(crtc_state, 0, sizeof(*crtc_state));
-		__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->uapi);
+		intel_crtc_state_reset(crtc_state, crtc);
 
 		crtc_state->hw.active = crtc_state->hw.enable =
 			dev_priv->display.get_pipe_config(crtc, crtc_state);
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/5] drm/i915: Introduce intel_plane_state_reset()

[Ville Syrjala]

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

For the sake of symmetry with the crtc stuff let's add
a helper to reset the plane state to sane default values.
For the moment this only gets caller from the plane init.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_atomic_plane.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 42b3b3449d2e..9429b8e17270 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -41,6 +41,16 @@
 #include "intel_pm.h"
 #include "intel_sprite.h"
 
+static void intel_plane_state_reset(struct intel_plane_state *plane_state,
+				    struct intel_plane *plane)
+{
+	memset(plane_state, 0, sizeof(*plane_state));
+
+	__drm_atomic_helper_plane_state_reset(&plane_state->uapi, &plane->base);
+
+	plane_state->scaler_id = -1;
+}
+
 struct intel_plane *intel_plane_alloc(void)
 {
 	struct intel_plane_state *plane_state;
@@ -56,8 +66,9 @@ struct intel_plane *intel_plane_alloc(void)
 		return ERR_PTR(-ENOMEM);
 	}
 
-	__drm_atomic_helper_plane_reset(&plane->base, &plane_state->uapi);
-	plane_state->scaler_id = -1;
+	intel_plane_state_reset(plane_state, plane);
+
+	plane->base.state = &plane_state->uapi;
 
 	return plane;
 }
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drm: Add __drm_atomic_helper_crtc_state_reset() & co.

[Daniel Vetter]

On Thu, Nov 07, 2019 at 04:24:13PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Annoyingly __drm_atomic_helper_crtc_reset() does two
> totally separate things:
> a) reset the state to defaults values
> b) assign the crtc->state pointer
> 
> I just want a) without the b) so let's split out part
> a) into __drm_atomic_helper_crtc_state_reset(). And
> of course we'll do the same thing for planes and connectors.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

And I guess

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

for merging through drm-intel, I don't expect anyone to start using this
in the next few weeks.

Cheers, Daniel

> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c | 70 ++++++++++++++++++++---
>  include/drm/drm_atomic_state_helper.h     |  6 ++
>  2 files changed, 67 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index d0a937fb0c56..a972068d58cf 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -57,6 +57,22 @@
>   * for these functions.
>   */
>  
> +/**
> + * __drm_atomic_helper_crtc_state_reset - reset the CRTC state
> + * @crtc_state: atomic CRTC state, must not be NULL
> + * @crtc: CRTC object, must not be NULL
> + *
> + * Initializes the newly allocated @crtc_state with default
> + * values. This is useful for drivers that subclass the CRTC state.
> + */
> +void
> +__drm_atomic_helper_crtc_state_reset(struct drm_crtc_state *crtc_state,
> +				     struct drm_crtc *crtc)
> +{
> +	crtc_state->crtc = crtc;
> +}
> +EXPORT_SYMBOL(__drm_atomic_helper_crtc_state_reset);
> +
>  /**
>   * __drm_atomic_helper_crtc_reset - reset state on CRTC
>   * @crtc: drm CRTC
> @@ -74,7 +90,7 @@ __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
>  			       struct drm_crtc_state *crtc_state)
>  {
>  	if (crtc_state)
> -		crtc_state->crtc = crtc;
> +		__drm_atomic_helper_crtc_state_reset(crtc_state, crtc);
>  
>  	crtc->state = crtc_state;
>  }
> @@ -212,23 +228,43 @@ void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
>  EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
>  
>  /**
> - * __drm_atomic_helper_plane_reset - resets planes state to default values
> + * __drm_atomic_helper_plane_state_reset - resets plane state to default values
> + * @plane_state: atomic plane state, must not be NULL
>   * @plane: plane object, must not be NULL
> - * @state: atomic plane state, must not be NULL
>   *
> - * Initializes plane state to default. This is useful for drivers that subclass
> - * the plane state.
> + * Initializes the newly allocated @plane_state with default
> + * values. This is useful for drivers that subclass the CRTC state.
>   */
> -void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> -				     struct drm_plane_state *state)
> +void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *state,
> +					   struct drm_plane *plane)
>  {
>  	state->plane = plane;
>  	state->rotation = DRM_MODE_ROTATE_0;
>  
>  	state->alpha = DRM_BLEND_ALPHA_OPAQUE;
>  	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> +}
> +EXPORT_SYMBOL(__drm_atomic_helper_plane_state_reset);
>  
> -	plane->state = state;
> +/**
> + * __drm_atomic_helper_plane_reset - reset state on plane
> + * @plane: drm plane
> + * @plane_state: plane state to assign
> + *
> + * Initializes the newly allocated @plane_state and assigns it to
> + * the &drm_crtc->state pointer of @plane, usually required when
> + * initializing the drivers or when called from the &drm_plane_funcs.reset
> + * hook.
> + *
> + * This is useful for drivers that subclass the plane state.
> + */
> +void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> +				     struct drm_plane_state *plane_state)
> +{
> +	if (plane_state)
> +		__drm_atomic_helper_plane_state_reset(plane_state, plane);
> +
> +	plane->state = plane_state;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_reset);
>  
> @@ -335,6 +371,22 @@ void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_plane_destroy_state);
>  
> +/**
> + * __drm_atomic_helper_connector_state_reset - reset the connector state
> + * @conn__state: atomic connector state, must not be NULL
> + * @connector: connectotr object, must not be NULL
> + *
> + * Initializes the newly allocated @conn_state with default
> + * values. This is useful for drivers that subclass the connector state.
> + */
> +void
> +__drm_atomic_helper_connector_state_reset(struct drm_connector_state *conn_state,
> +					  struct drm_connector *connector)
> +{
> +	conn_state->connector = connector;
> +}
> +EXPORT_SYMBOL(__drm_atomic_helper_connector_state_reset);
> +
>  /**
>   * __drm_atomic_helper_connector_reset - reset state on connector
>   * @connector: drm connector
> @@ -352,7 +404,7 @@ __drm_atomic_helper_connector_reset(struct drm_connector *connector,
>  				    struct drm_connector_state *conn_state)
>  {
>  	if (conn_state)
> -		conn_state->connector = connector;
> +		__drm_atomic_helper_connector_state_reset(conn_state, connector);
>  
>  	connector->state = conn_state;
>  }
> diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
> index e4577cc11689..8171dea4cc22 100644
> --- a/include/drm/drm_atomic_state_helper.h
> +++ b/include/drm/drm_atomic_state_helper.h
> @@ -37,6 +37,8 @@ struct drm_private_state;
>  struct drm_modeset_acquire_ctx;
>  struct drm_device;
>  
> +void __drm_atomic_helper_crtc_state_reset(struct drm_crtc_state *state,
> +					  struct drm_crtc *crtc);
>  void __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
>  				    struct drm_crtc_state *state);
>  void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc);
> @@ -48,6 +50,8 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state);
>  void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
>  					  struct drm_crtc_state *state);
>  
> +void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *state,
> +					   struct drm_plane *plane);
>  void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
>  				     struct drm_plane_state *state);
>  void drm_atomic_helper_plane_reset(struct drm_plane *plane);
> @@ -59,6 +63,8 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state);
>  void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,
>  					  struct drm_plane_state *state);
>  
> +void __drm_atomic_helper_connector_state_reset(struct drm_connector_state *conn_state,
> +					       struct drm_connector *connector);
>  void __drm_atomic_helper_connector_reset(struct drm_connector *connector,
>  					 struct drm_connector_state *conn_state);
>  void drm_atomic_helper_connector_reset(struct drm_connector *connector);
> -- 
> 2.23.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://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
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/5] drm: Add __drm_atomic_helper_crtc_state_reset() & co.

[Daniel Vetter]

On Thu, Nov 07, 2019 at 04:24:13PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Annoyingly __drm_atomic_helper_crtc_reset() does two
> totally separate things:
> a) reset the state to defaults values
> b) assign the crtc->state pointer
> 
> I just want a) without the b) so let's split out part
> a) into __drm_atomic_helper_crtc_state_reset(). And
> of course we'll do the same thing for planes and connectors.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

And I guess

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

for merging through drm-intel, I don't expect anyone to start using this
in the next few weeks.

Cheers, Daniel

> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c | 70 ++++++++++++++++++++---
>  include/drm/drm_atomic_state_helper.h     |  6 ++
>  2 files changed, 67 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index d0a937fb0c56..a972068d58cf 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -57,6 +57,22 @@
>   * for these functions.
>   */
>  
> +/**
> + * __drm_atomic_helper_crtc_state_reset - reset the CRTC state
> + * @crtc_state: atomic CRTC state, must not be NULL
> + * @crtc: CRTC object, must not be NULL
> + *
> + * Initializes the newly allocated @crtc_state with default
> + * values. This is useful for drivers that subclass the CRTC state.
> + */
> +void
> +__drm_atomic_helper_crtc_state_reset(struct drm_crtc_state *crtc_state,
> +				     struct drm_crtc *crtc)
> +{
> +	crtc_state->crtc = crtc;
> +}
> +EXPORT_SYMBOL(__drm_atomic_helper_crtc_state_reset);
> +
>  /**
>   * __drm_atomic_helper_crtc_reset - reset state on CRTC
>   * @crtc: drm CRTC
> @@ -74,7 +90,7 @@ __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
>  			       struct drm_crtc_state *crtc_state)
>  {
>  	if (crtc_state)
> -		crtc_state->crtc = crtc;
> +		__drm_atomic_helper_crtc_state_reset(crtc_state, crtc);
>  
>  	crtc->state = crtc_state;
>  }
> @@ -212,23 +228,43 @@ void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
>  EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
>  
>  /**
> - * __drm_atomic_helper_plane_reset - resets planes state to default values
> + * __drm_atomic_helper_plane_state_reset - resets plane state to default values
> + * @plane_state: atomic plane state, must not be NULL
>   * @plane: plane object, must not be NULL
> - * @state: atomic plane state, must not be NULL
>   *
> - * Initializes plane state to default. This is useful for drivers that subclass
> - * the plane state.
> + * Initializes the newly allocated @plane_state with default
> + * values. This is useful for drivers that subclass the CRTC state.
>   */
> -void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> -				     struct drm_plane_state *state)
> +void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *state,
> +					   struct drm_plane *plane)
>  {
>  	state->plane = plane;
>  	state->rotation = DRM_MODE_ROTATE_0;
>  
>  	state->alpha = DRM_BLEND_ALPHA_OPAQUE;
>  	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> +}
> +EXPORT_SYMBOL(__drm_atomic_helper_plane_state_reset);
>  
> -	plane->state = state;
> +/**
> + * __drm_atomic_helper_plane_reset - reset state on plane
> + * @plane: drm plane
> + * @plane_state: plane state to assign
> + *
> + * Initializes the newly allocated @plane_state and assigns it to
> + * the &drm_crtc->state pointer of @plane, usually required when
> + * initializing the drivers or when called from the &drm_plane_funcs.reset
> + * hook.
> + *
> + * This is useful for drivers that subclass the plane state.
> + */
> +void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> +				     struct drm_plane_state *plane_state)
> +{
> +	if (plane_state)
> +		__drm_atomic_helper_plane_state_reset(plane_state, plane);
> +
> +	plane->state = plane_state;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_reset);
>  
> @@ -335,6 +371,22 @@ void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_plane_destroy_state);
>  
> +/**
> + * __drm_atomic_helper_connector_state_reset - reset the connector state
> + * @conn__state: atomic connector state, must not be NULL
> + * @connector: connectotr object, must not be NULL
> + *
> + * Initializes the newly allocated @conn_state with default
> + * values. This is useful for drivers that subclass the connector state.
> + */
> +void
> +__drm_atomic_helper_connector_state_reset(struct drm_connector_state *conn_state,
> +					  struct drm_connector *connector)
> +{
> +	conn_state->connector = connector;
> +}
> +EXPORT_SYMBOL(__drm_atomic_helper_connector_state_reset);
> +
>  /**
>   * __drm_atomic_helper_connector_reset - reset state on connector
>   * @connector: drm connector
> @@ -352,7 +404,7 @@ __drm_atomic_helper_connector_reset(struct drm_connector *connector,
>  				    struct drm_connector_state *conn_state)
>  {
>  	if (conn_state)
> -		conn_state->connector = connector;
> +		__drm_atomic_helper_connector_state_reset(conn_state, connector);
>  
>  	connector->state = conn_state;
>  }
> diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
> index e4577cc11689..8171dea4cc22 100644
> --- a/include/drm/drm_atomic_state_helper.h
> +++ b/include/drm/drm_atomic_state_helper.h
> @@ -37,6 +37,8 @@ struct drm_private_state;
>  struct drm_modeset_acquire_ctx;
>  struct drm_device;
>  
> +void __drm_atomic_helper_crtc_state_reset(struct drm_crtc_state *state,
> +					  struct drm_crtc *crtc);
>  void __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
>  				    struct drm_crtc_state *state);
>  void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc);
> @@ -48,6 +50,8 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state);
>  void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
>  					  struct drm_crtc_state *state);
>  
> +void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *state,
> +					   struct drm_plane *plane);
>  void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
>  				     struct drm_plane_state *state);
>  void drm_atomic_helper_plane_reset(struct drm_plane *plane);
> @@ -59,6 +63,8 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state);
>  void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,
>  					  struct drm_plane_state *state);
>  
> +void __drm_atomic_helper_connector_state_reset(struct drm_connector_state *conn_state,
> +					       struct drm_connector *connector);
>  void __drm_atomic_helper_connector_reset(struct drm_connector *connector,
>  					 struct drm_connector_state *conn_state);
>  void drm_atomic_helper_connector_reset(struct drm_connector *connector);
> -- 
> 2.23.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH 2/5] drm/i915: s/intel_crtc/crtc/ in intel_crtc_init()

[Souza, Jose]

On Thu, 2019-11-07 at 16:24 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Let's get rid of the redundant intel_ prefix on our variables.
> 

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 32 ++++++++++------
> ----
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 551de2baa569..8b889c9f29b5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -15838,14 +15838,14 @@ static const struct drm_crtc_funcs
> i8xx_crtc_funcs = {
>  static int intel_crtc_init(struct drm_i915_private *dev_priv, enum
> pipe pipe)
>  {
>  	const struct drm_crtc_funcs *funcs;
> -	struct intel_crtc *intel_crtc;
> +	struct intel_crtc *crtc;
>  	struct intel_crtc_state *crtc_state = NULL;
>  	struct intel_plane *primary = NULL;
>  	struct intel_plane *cursor = NULL;
>  	int sprite, ret;
>  
> -	intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL);
> -	if (!intel_crtc)
> +	crtc = kzalloc(sizeof(*crtc), GFP_KERNEL);
> +	if (!crtc)
>  		return -ENOMEM;
>  
>  	crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
> @@ -15853,15 +15853,15 @@ static int intel_crtc_init(struct
> drm_i915_private *dev_priv, enum pipe pipe)
>  		ret = -ENOMEM;
>  		goto fail;
>  	}
> -	__drm_atomic_helper_crtc_reset(&intel_crtc->base, &crtc_state-
> >uapi);
> -	intel_crtc->config = crtc_state;
> +	__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->uapi);
> +	crtc->config = crtc_state;
>  
>  	primary = intel_primary_plane_create(dev_priv, pipe);
>  	if (IS_ERR(primary)) {
>  		ret = PTR_ERR(primary);
>  		goto fail;
>  	}
> -	intel_crtc->plane_ids_mask |= BIT(primary->id);
> +	crtc->plane_ids_mask |= BIT(primary->id);
>  
>  	for_each_sprite(dev_priv, pipe, sprite) {
>  		struct intel_plane *plane;
> @@ -15871,7 +15871,7 @@ static int intel_crtc_init(struct
> drm_i915_private *dev_priv, enum pipe pipe)
>  			ret = PTR_ERR(plane);
>  			goto fail;
>  		}
> -		intel_crtc->plane_ids_mask |= BIT(plane->id);
> +		crtc->plane_ids_mask |= BIT(plane->id);
>  	}
>  
>  	cursor = intel_cursor_plane_create(dev_priv, pipe);
> @@ -15879,7 +15879,7 @@ static int intel_crtc_init(struct
> drm_i915_private *dev_priv, enum pipe pipe)
>  		ret = PTR_ERR(cursor);
>  		goto fail;
>  	}
> -	intel_crtc->plane_ids_mask |= BIT(cursor->id);
> +	crtc->plane_ids_mask |= BIT(cursor->id);
>  
>  	if (HAS_GMCH(dev_priv)) {
>  		if (IS_CHERRYVIEW(dev_priv) ||
> @@ -15900,32 +15900,32 @@ static int intel_crtc_init(struct
> drm_i915_private *dev_priv, enum pipe pipe)
>  			funcs = &ilk_crtc_funcs;
>  	}
>  
> -	ret = drm_crtc_init_with_planes(&dev_priv->drm, &intel_crtc-
> >base,
> +	ret = drm_crtc_init_with_planes(&dev_priv->drm, &crtc->base,
>  					&primary->base, &cursor->base,
>  					funcs, "pipe %c",
> pipe_name(pipe));
>  	if (ret)
>  		goto fail;
>  
> -	intel_crtc->pipe = pipe;
> +	crtc->pipe = pipe;
>  
>  	/* initialize shared scalers */
> -	intel_crtc_init_scalers(intel_crtc, crtc_state);
> +	intel_crtc_init_scalers(crtc, crtc_state);
>  
>  	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->pipe_to_crtc_mapping) ||
>  	       dev_priv->pipe_to_crtc_mapping[pipe] != NULL);
> -	dev_priv->pipe_to_crtc_mapping[pipe] = intel_crtc;
> +	dev_priv->pipe_to_crtc_mapping[pipe] = crtc;
>  
>  	if (INTEL_GEN(dev_priv) < 9) {
>  		enum i9xx_plane_id i9xx_plane = primary->i9xx_plane;
>  
>  		BUG_ON(i9xx_plane >= ARRAY_SIZE(dev_priv-
> >plane_to_crtc_mapping) ||
>  		       dev_priv->plane_to_crtc_mapping[i9xx_plane] !=
> NULL);
> -		dev_priv->plane_to_crtc_mapping[i9xx_plane] =
> intel_crtc;
> +		dev_priv->plane_to_crtc_mapping[i9xx_plane] = crtc;
>  	}
>  
> -	intel_color_init(intel_crtc);
> +	intel_color_init(crtc);
>  
> -	WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
> +	WARN_ON(drm_crtc_index(&crtc->base) != crtc->pipe);
>  
>  	return 0;
>  
> @@ -15935,7 +15935,7 @@ static int intel_crtc_init(struct
> drm_i915_private *dev_priv, enum pipe pipe)
>  	 * crtcs/planes already initialized.
>  	 */
>  	kfree(crtc_state);
> -	kfree(intel_crtc);
> +	kfree(crtc);
>  
>  	return ret;
>  }
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/5] drm/i915: Introduce intel_crtc_{alloc,free}()

[Souza, Jose]

On Thu, 2019-11-07 at 16:24 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We already have alloc/free helpers for planes, add the same for
> crtcs. The main benefit is we get to move all the annoying state
> initialization out of the main crtc_init() flow.
> 

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 74 ++++++++++------
> ----
>  1 file changed, 36 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 8b889c9f29b5..e6291841053f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -164,8 +164,7 @@ static void vlv_prepare_pll(struct intel_crtc
> *crtc,
>  			    const struct intel_crtc_state
> *pipe_config);
>  static void chv_prepare_pll(struct intel_crtc *crtc,
>  			    const struct intel_crtc_state
> *pipe_config);
> -static void intel_crtc_init_scalers(struct intel_crtc *crtc,
> -				    struct intel_crtc_state
> *crtc_state);
> +static void intel_crtc_init_scalers(struct intel_crtc_state
> *crtc_state);
>  static void skylake_pfit_enable(const struct intel_crtc_state
> *crtc_state);
>  static void ironlake_pfit_disable(const struct intel_crtc_state
> *old_crtc_state);
>  static void ironlake_pfit_enable(const struct intel_crtc_state
> *crtc_state);
> @@ -10656,7 +10655,7 @@ static bool haswell_get_pipe_config(struct
> intel_crtc *crtc,
>  	u64 power_domain_mask;
>  	bool active;
>  
> -	intel_crtc_init_scalers(crtc, pipe_config);
> +	intel_crtc_init_scalers(pipe_config);
>  
>  	pipe_config->master_transcoder = INVALID_TRANSCODER;
>  
> @@ -15746,25 +15745,12 @@ intel_cursor_plane_create(struct
> drm_i915_private *dev_priv,
>  	return ERR_PTR(ret);
>  }
>  
> -static void intel_crtc_init_scalers(struct intel_crtc *crtc,
> -				    struct intel_crtc_state
> *crtc_state)
> +static void intel_crtc_init_scalers(struct intel_crtc_state
> *crtc_state)
>  {
>  	struct intel_crtc_scaler_state *scaler_state =
>  		&crtc_state->scaler_state;
> -	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	int i;
> -
> -	crtc->num_scalers = RUNTIME_INFO(dev_priv)->num_scalers[crtc-
> >pipe];
> -	if (!crtc->num_scalers)
> -		return;
> -
> -	for (i = 0; i < crtc->num_scalers; i++) {
> -		struct intel_scaler *scaler = &scaler_state-
> >scalers[i];
> -
> -		scaler->in_use = 0;
> -		scaler->mode = 0;
> -	}
>  
> +	memset(scaler_state, 0, sizeof(*scaler_state));
>  	scaler_state->scaler_id = -1;
>  }
>  
> @@ -15835,27 +15821,49 @@ static const struct drm_crtc_funcs
> i8xx_crtc_funcs = {
>  	.disable_vblank = i8xx_disable_vblank,
>  };
>  
> -static int intel_crtc_init(struct drm_i915_private *dev_priv, enum
> pipe pipe)
> +static struct intel_crtc *intel_crtc_alloc(void)
>  {
> -	const struct drm_crtc_funcs *funcs;
> +	struct intel_crtc_state *crtc_state;
>  	struct intel_crtc *crtc;
> -	struct intel_crtc_state *crtc_state = NULL;
> -	struct intel_plane *primary = NULL;
> -	struct intel_plane *cursor = NULL;
> -	int sprite, ret;
>  
>  	crtc = kzalloc(sizeof(*crtc), GFP_KERNEL);
>  	if (!crtc)
> -		return -ENOMEM;
> +		return ERR_PTR(-ENOMEM);
>  
>  	crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>  	if (!crtc_state) {
> -		ret = -ENOMEM;
> -		goto fail;
> +		kfree(crtc);
> +		return ERR_PTR(-ENOMEM);
>  	}
> +
>  	__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->uapi);
> +	intel_crtc_init_scalers(crtc_state);
> +
>  	crtc->config = crtc_state;
>  
> +	return crtc;
> +}
> +
> +static void intel_crtc_free(struct intel_crtc *crtc)
> +{
> +	intel_crtc_destroy_state(&crtc->base, crtc->base.state);
> +	kfree(crtc);
> +}
> +
> +static int intel_crtc_init(struct drm_i915_private *dev_priv, enum
> pipe pipe)
> +{
> +	struct intel_plane *primary, *cursor;
> +	const struct drm_crtc_funcs *funcs;
> +	struct intel_crtc *crtc;
> +	int sprite, ret;
> +
> +	crtc = intel_crtc_alloc();
> +	if (IS_ERR(crtc))
> +		return PTR_ERR(crtc);
> +
> +	crtc->pipe = pipe;
> +	crtc->num_scalers = RUNTIME_INFO(dev_priv)->num_scalers[pipe];
> +
>  	primary = intel_primary_plane_create(dev_priv, pipe);
>  	if (IS_ERR(primary)) {
>  		ret = PTR_ERR(primary);
> @@ -15906,11 +15914,6 @@ static int intel_crtc_init(struct
> drm_i915_private *dev_priv, enum pipe pipe)
>  	if (ret)
>  		goto fail;
>  
> -	crtc->pipe = pipe;
> -
> -	/* initialize shared scalers */
> -	intel_crtc_init_scalers(crtc, crtc_state);
> -
>  	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->pipe_to_crtc_mapping) ||
>  	       dev_priv->pipe_to_crtc_mapping[pipe] != NULL);
>  	dev_priv->pipe_to_crtc_mapping[pipe] = crtc;
> @@ -15930,12 +15933,7 @@ static int intel_crtc_init(struct
> drm_i915_private *dev_priv, enum pipe pipe)
>  	return 0;
>  
>  fail:
> -	/*
> -	 * drm_mode_config_cleanup() will free up any
> -	 * crtcs/planes already initialized.
> -	 */
> -	kfree(crtc_state);
> -	kfree(crtc);
> +	intel_crtc_free(crtc);
>  
>  	return ret;
>  }
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 4/5] drm/i915: Introduce intel_crtc_state_reset()

[Souza, Jose]

On Thu, 2019-11-07 at 16:24 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We have a few places where we want to reset a crtc state to its
> default values. Let's add a helper for that. We'll need the new
> __drm_atomic_helper_crtc_state_reset() helper for this to allow
> us to just reset the state itself without clobbering the
> crtc->state pointer.
> 
> And while at it let's zero out the whole thing, except a few
> choice member which we'll mark as "invalid". And thanks to this
> we can now nuke intel_crtc_init_scalers().
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 47 +++++++++---------
> --
>  1 file changed, 22 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index e6291841053f..fd4120533c3f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -164,7 +164,6 @@ static void vlv_prepare_pll(struct intel_crtc
> *crtc,
>  			    const struct intel_crtc_state
> *pipe_config);
>  static void chv_prepare_pll(struct intel_crtc *crtc,
>  			    const struct intel_crtc_state
> *pipe_config);
> -static void intel_crtc_init_scalers(struct intel_crtc_state
> *crtc_state);
>  static void skylake_pfit_enable(const struct intel_crtc_state
> *crtc_state);
>  static void ironlake_pfit_disable(const struct intel_crtc_state
> *old_crtc_state);
>  static void ironlake_pfit_enable(const struct intel_crtc_state
> *crtc_state);
> @@ -10655,8 +10654,6 @@ static bool haswell_get_pipe_config(struct
> intel_crtc *crtc,
>  	u64 power_domain_mask;
>  	bool active;
>  
> -	intel_crtc_init_scalers(pipe_config);
> -
>  	pipe_config->master_transcoder = INVALID_TRANSCODER;
>  
>  	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
> @@ -11704,6 +11701,20 @@ static void ironlake_pch_clock_get(struct
> intel_crtc *crtc,
>  					 &pipe_config->fdi_m_n);
>  }
>  
> +static void intel_crtc_state_reset(struct intel_crtc_state
> *crtc_state,
> +				   struct intel_crtc *crtc)
> +{
> +	memset(crtc_state, 0, sizeof(*crtc_state));
> +
> +	__drm_atomic_helper_crtc_state_reset(&crtc_state->uapi, &crtc-
> >base);
> +
> +	crtc_state->cpu_transcoder = INVALID_TRANSCODER;
> +	crtc_state->master_transcoder = INVALID_TRANSCODER;

At least master_transcoder is set to invalid again but we can remove
the redundant sets later

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> +	crtc_state->hsw_workaround_pipe = INVALID_PIPE;
> +	crtc_state->output_format = INTEL_OUTPUT_FORMAT_INVALID;
> +	crtc_state->scaler_state.scaler_id = -1;
> +}
> +
>  /* Returns the currently programmed mode of the given encoder. */
>  struct drm_display_mode *
>  intel_encoder_current_mode(struct intel_encoder *encoder)
> @@ -11729,7 +11740,7 @@ intel_encoder_current_mode(struct
> intel_encoder *encoder)
>  		return NULL;
>  	}
>  
> -	crtc_state->uapi.crtc = &crtc->base;
> +	intel_crtc_state_reset(crtc_state, crtc);
>  
>  	if (!dev_priv->display.get_pipe_config(crtc, crtc_state)) {
>  		kfree(crtc_state);
> @@ -13577,18 +13588,14 @@ verify_crtc_state(struct intel_crtc *crtc,
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_encoder *encoder;
> -	struct intel_crtc_state *pipe_config;
> -	struct drm_atomic_state *state;
> +	struct intel_crtc_state *pipe_config = old_crtc_state;
> +	struct drm_atomic_state *state = old_crtc_state->uapi.state;
>  	bool active;
>  
> -	state = old_crtc_state->uapi.state;
>  	__drm_atomic_helper_crtc_destroy_state(&old_crtc_state->uapi);
>  	intel_crtc_free_hw_state(old_crtc_state);
> -
> -	pipe_config = old_crtc_state;
> -	memset(pipe_config, 0, sizeof(*pipe_config));
> -	pipe_config->uapi.crtc = &crtc->base;
> -	pipe_config->uapi.state = state;
> +	intel_crtc_state_reset(old_crtc_state, crtc);
> +	old_crtc_state->uapi.state = state;
>  
>  	DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.base.id, crtc-
> >base.name);
>  
> @@ -15745,15 +15752,6 @@ intel_cursor_plane_create(struct
> drm_i915_private *dev_priv,
>  	return ERR_PTR(ret);
>  }
>  
> -static void intel_crtc_init_scalers(struct intel_crtc_state
> *crtc_state)
> -{
> -	struct intel_crtc_scaler_state *scaler_state =
> -		&crtc_state->scaler_state;
> -
> -	memset(scaler_state, 0, sizeof(*scaler_state));
> -	scaler_state->scaler_id = -1;
> -}
> -
>  #define INTEL_CRTC_FUNCS \
>  	.gamma_set = drm_atomic_helper_legacy_gamma_set, \
>  	.set_config = drm_atomic_helper_set_config, \
> @@ -15836,9 +15834,9 @@ static struct intel_crtc
> *intel_crtc_alloc(void)
>  		return ERR_PTR(-ENOMEM);
>  	}
>  
> -	__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->uapi);
> -	intel_crtc_init_scalers(crtc_state);
> +	intel_crtc_state_reset(crtc_state, crtc);
>  
> +	crtc->base.state = &crtc_state->uapi;
>  	crtc->config = crtc_state;
>  
>  	return crtc;
> @@ -17414,8 +17412,7 @@ static void
> intel_modeset_readout_hw_state(struct drm_device *dev)
>  
>  		__drm_atomic_helper_crtc_destroy_state(&crtc_state-
> >uapi);
>  		intel_crtc_free_hw_state(crtc_state);
> -		memset(crtc_state, 0, sizeof(*crtc_state));
> -		__drm_atomic_helper_crtc_reset(&crtc->base,
> &crtc_state->uapi);
> +		intel_crtc_state_reset(crtc_state, crtc);
>  
>  		crtc_state->hw.active = crtc_state->hw.enable =
>  			dev_priv->display.get_pipe_config(crtc,
> crtc_state);
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 5/5] drm/i915: Introduce intel_plane_state_reset()

[Souza, Jose]

On Thu, 2019-11-07 at 16:24 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> For the sake of symmetry with the crtc stuff let's add
> a helper to reset the plane state to sane default values.
> For the moment this only gets caller from the plane init.
> 

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_atomic_plane.c | 15
> +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index 42b3b3449d2e..9429b8e17270 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -41,6 +41,16 @@
>  #include "intel_pm.h"
>  #include "intel_sprite.h"
>  
> +static void intel_plane_state_reset(struct intel_plane_state
> *plane_state,
> +				    struct intel_plane *plane)
> +{
> +	memset(plane_state, 0, sizeof(*plane_state));
> +
> +	__drm_atomic_helper_plane_state_reset(&plane_state->uapi,
> &plane->base);
> +
> +	plane_state->scaler_id = -1;
> +}
> +
>  struct intel_plane *intel_plane_alloc(void)
>  {
>  	struct intel_plane_state *plane_state;
> @@ -56,8 +66,9 @@ struct intel_plane *intel_plane_alloc(void)
>  		return ERR_PTR(-ENOMEM);
>  	}
>  
> -	__drm_atomic_helper_plane_reset(&plane->base, &plane_state-
> >uapi);
> -	plane_state->scaler_id = -1;
> +	intel_plane_state_reset(plane_state, plane);
> +
> +	plane->base.state = &plane_state->uapi;
>  
>  	return plane;
>  }
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/5] drm: Add __drm_atomic_helper_crtc_state_reset() & co.

[Lucas De Marchi]

On Thu, Nov 07, 2019 at 04:24:13PM +0200, Ville Syrjälä wrote:
>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>Annoyingly __drm_atomic_helper_crtc_reset() does two
>totally separate things:
>a) reset the state to defaults values
>b) assign the crtc->state pointer
>
>I just want a) without the b) so let's split out part
>a) into __drm_atomic_helper_crtc_state_reset(). And
>of course we'll do the same thing for planes and connectors.
>
>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>---
> drivers/gpu/drm/drm_atomic_state_helper.c | 70 ++++++++++++++++++++---
> include/drm/drm_atomic_state_helper.h     |  6 ++
> 2 files changed, 67 insertions(+), 9 deletions(-)
>
>diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
>index d0a937fb0c56..a972068d58cf 100644
>--- a/drivers/gpu/drm/drm_atomic_state_helper.c
>+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
>@@ -57,6 +57,22 @@
>  * for these functions.
>  */
>
>+/**
>+ * __drm_atomic_helper_crtc_state_reset - reset the CRTC state
>+ * @crtc_state: atomic CRTC state, must not be NULL
>+ * @crtc: CRTC object, must not be NULL
>+ *
>+ * Initializes the newly allocated @crtc_state with default
>+ * values. This is useful for drivers that subclass the CRTC state.
>+ */
>+void
>+__drm_atomic_helper_crtc_state_reset(struct drm_crtc_state *crtc_state,
>+				     struct drm_crtc *crtc)
>+{
>+	crtc_state->crtc = crtc;
>+}
>+EXPORT_SYMBOL(__drm_atomic_helper_crtc_state_reset);
>+
> /**
>  * __drm_atomic_helper_crtc_reset - reset state on CRTC
>  * @crtc: drm CRTC
>@@ -74,7 +90,7 @@ __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
> 			       struct drm_crtc_state *crtc_state)
> {
> 	if (crtc_state)
>-		crtc_state->crtc = crtc;
>+		__drm_atomic_helper_crtc_state_reset(crtc_state, crtc);
>
> 	crtc->state = crtc_state;
> }
>@@ -212,23 +228,43 @@ void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
> EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
>
> /**
>- * __drm_atomic_helper_plane_reset - resets planes state to default values
>+ * __drm_atomic_helper_plane_state_reset - resets plane state to default values
>+ * @plane_state: atomic plane state, must not be NULL
>  * @plane: plane object, must not be NULL
>- * @state: atomic plane state, must not be NULL
>  *
>- * Initializes plane state to default. This is useful for drivers that subclass
>- * the plane state.
>+ * Initializes the newly allocated @plane_state with default
>+ * values. This is useful for drivers that subclass the CRTC state.
>  */
>-void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
>-				     struct drm_plane_state *state)
>+void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *state,
>+					   struct drm_plane *plane)
> {
> 	state->plane = plane;
> 	state->rotation = DRM_MODE_ROTATE_0;
>
> 	state->alpha = DRM_BLEND_ALPHA_OPAQUE;
> 	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
>+}
>+EXPORT_SYMBOL(__drm_atomic_helper_plane_state_reset);
>
>-	plane->state = state;
>+/**
>+ * __drm_atomic_helper_plane_reset - reset state on plane
>+ * @plane: drm plane
>+ * @plane_state: plane state to assign
>+ *
>+ * Initializes the newly allocated @plane_state and assigns it to
>+ * the &drm_crtc->state pointer of @plane, usually required when
>+ * initializing the drivers or when called from the &drm_plane_funcs.reset
>+ * hook.
>+ *
>+ * This is useful for drivers that subclass the plane state.
>+ */
>+void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
>+				     struct drm_plane_state *plane_state)
>+{
>+	if (plane_state)
>+		__drm_atomic_helper_plane_state_reset(plane_state, plane);
>+
>+	plane->state = plane_state;
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_reset);
>
>@@ -335,6 +371,22 @@ void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,
> }
> EXPORT_SYMBOL(drm_atomic_helper_plane_destroy_state);
>
>+/**
>+ * __drm_atomic_helper_connector_state_reset - reset the connector state
>+ * @conn__state: atomic connector state, must not be NULL

typo here, otherwise


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

>+ * @connector: connectotr object, must not be NULL
>+ *
>+ * Initializes the newly allocated @conn_state with default
>+ * values. This is useful for drivers that subclass the connector state.
>+ */
>+void
>+__drm_atomic_helper_connector_state_reset(struct drm_connector_state *conn_state,
>+					  struct drm_connector *connector)
>+{
>+	conn_state->connector = connector;
>+}
>+EXPORT_SYMBOL(__drm_atomic_helper_connector_state_reset);
>+
> /**
>  * __drm_atomic_helper_connector_reset - reset state on connector
>  * @connector: drm connector
>@@ -352,7 +404,7 @@ __drm_atomic_helper_connector_reset(struct drm_connector *connector,
> 				    struct drm_connector_state *conn_state)
> {
> 	if (conn_state)
>-		conn_state->connector = connector;
>+		__drm_atomic_helper_connector_state_reset(conn_state, connector);
>
> 	connector->state = conn_state;
> }
>diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
>index e4577cc11689..8171dea4cc22 100644
>--- a/include/drm/drm_atomic_state_helper.h
>+++ b/include/drm/drm_atomic_state_helper.h
>@@ -37,6 +37,8 @@ struct drm_private_state;
> struct drm_modeset_acquire_ctx;
> struct drm_device;
>
>+void __drm_atomic_helper_crtc_state_reset(struct drm_crtc_state *state,
>+					  struct drm_crtc *crtc);
> void __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
> 				    struct drm_crtc_state *state);
> void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc);
>@@ -48,6 +50,8 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state);
> void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
> 					  struct drm_crtc_state *state);
>
>+void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *state,
>+					   struct drm_plane *plane);
> void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> 				     struct drm_plane_state *state);
> void drm_atomic_helper_plane_reset(struct drm_plane *plane);
>@@ -59,6 +63,8 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state);
> void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,
> 					  struct drm_plane_state *state);
>
>+void __drm_atomic_helper_connector_state_reset(struct drm_connector_state *conn_state,
>+					       struct drm_connector *connector);
> void __drm_atomic_helper_connector_reset(struct drm_connector *connector,
> 					 struct drm_connector_state *conn_state);
> void drm_atomic_helper_connector_reset(struct drm_connector *connector);
>-- 
>2.23.0
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/5] drm: Add __drm_atomic_helper_crtc_state_reset() & co.

[Ville Syrjälä]

On Fri, Dec 13, 2019 at 03:38:53PM -0800, Lucas De Marchi wrote:
> On Thu, Nov 07, 2019 at 04:24:13PM +0200, Ville Syrjälä wrote:
> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >Annoyingly __drm_atomic_helper_crtc_reset() does two
> >totally separate things:
> >a) reset the state to defaults values
> >b) assign the crtc->state pointer
> >
> >I just want a) without the b) so let's split out part
> >a) into __drm_atomic_helper_crtc_state_reset(). And
> >of course we'll do the same thing for planes and connectors.
> >
> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >---
> > drivers/gpu/drm/drm_atomic_state_helper.c | 70 ++++++++++++++++++++---
> > include/drm/drm_atomic_state_helper.h     |  6 ++
> > 2 files changed, 67 insertions(+), 9 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> >index d0a937fb0c56..a972068d58cf 100644
> >--- a/drivers/gpu/drm/drm_atomic_state_helper.c
> >+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> >@@ -57,6 +57,22 @@
> >  * for these functions.
> >  */
> >
> >+/**
> >+ * __drm_atomic_helper_crtc_state_reset - reset the CRTC state
> >+ * @crtc_state: atomic CRTC state, must not be NULL
> >+ * @crtc: CRTC object, must not be NULL
> >+ *
> >+ * Initializes the newly allocated @crtc_state with default
> >+ * values. This is useful for drivers that subclass the CRTC state.
> >+ */
> >+void
> >+__drm_atomic_helper_crtc_state_reset(struct drm_crtc_state *crtc_state,
> >+				     struct drm_crtc *crtc)
> >+{
> >+	crtc_state->crtc = crtc;
> >+}
> >+EXPORT_SYMBOL(__drm_atomic_helper_crtc_state_reset);
> >+
> > /**
> >  * __drm_atomic_helper_crtc_reset - reset state on CRTC
> >  * @crtc: drm CRTC
> >@@ -74,7 +90,7 @@ __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
> > 			       struct drm_crtc_state *crtc_state)
> > {
> > 	if (crtc_state)
> >-		crtc_state->crtc = crtc;
> >+		__drm_atomic_helper_crtc_state_reset(crtc_state, crtc);
> >
> > 	crtc->state = crtc_state;
> > }
> >@@ -212,23 +228,43 @@ void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
> > EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
> >
> > /**
> >- * __drm_atomic_helper_plane_reset - resets planes state to default values
> >+ * __drm_atomic_helper_plane_state_reset - resets plane state to default values
> >+ * @plane_state: atomic plane state, must not be NULL
> >  * @plane: plane object, must not be NULL
> >- * @state: atomic plane state, must not be NULL
> >  *
> >- * Initializes plane state to default. This is useful for drivers that subclass
> >- * the plane state.
> >+ * Initializes the newly allocated @plane_state with default
> >+ * values. This is useful for drivers that subclass the CRTC state.
> >  */
> >-void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> >-				     struct drm_plane_state *state)
> >+void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *state,
> >+					   struct drm_plane *plane)
> > {
> > 	state->plane = plane;
> > 	state->rotation = DRM_MODE_ROTATE_0;
> >
> > 	state->alpha = DRM_BLEND_ALPHA_OPAQUE;
> > 	state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> >+}
> >+EXPORT_SYMBOL(__drm_atomic_helper_plane_state_reset);
> >
> >-	plane->state = state;
> >+/**
> >+ * __drm_atomic_helper_plane_reset - reset state on plane
> >+ * @plane: drm plane
> >+ * @plane_state: plane state to assign
> >+ *
> >+ * Initializes the newly allocated @plane_state and assigns it to
> >+ * the &drm_crtc->state pointer of @plane, usually required when
> >+ * initializing the drivers or when called from the &drm_plane_funcs.reset
> >+ * hook.
> >+ *
> >+ * This is useful for drivers that subclass the plane state.
> >+ */
> >+void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> >+				     struct drm_plane_state *plane_state)
> >+{
> >+	if (plane_state)
> >+		__drm_atomic_helper_plane_state_reset(plane_state, plane);
> >+
> >+	plane->state = plane_state;
> > }
> > EXPORT_SYMBOL(__drm_atomic_helper_plane_reset);
> >
> >@@ -335,6 +371,22 @@ void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,
> > }
> > EXPORT_SYMBOL(drm_atomic_helper_plane_destroy_state);
> >
> >+/**
> >+ * __drm_atomic_helper_connector_state_reset - reset the connector state
> >+ * @conn__state: atomic connector state, must not be NULL
> 
> typo here, otherwise

Thanks for catching that. Made me run a doc build that found a mismatch
between kerneldoc vs. code for the plane function, so I fixed that up
while pushing.

Entire series pushed to dinq with Daniel's ack for the first patch.
Though in hindsight I could have just pushed that one to drm-misc eons
ago. Oh well.

Thanks for the reviews.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel