[v5,17/19] drm/msm/dpu: remove RM dependency on connector state

Submitted by Jeykumar Sankaran on Sept. 6, 2018, 2:08 a.m.

Details

Message ID 1536199708-23664-18-git-send-email-jsanka@codeaurora.org
State New
Headers show
Series "clean up DPU for RM refactor" ( rev: 2 ) in DRI devel

Not browsing as part of any series.

Commit Message

Jeykumar Sankaran Sept. 6, 2018, 2:08 a.m.
Connector states were passed around RM to update the custom
topology connector property with chosen topology data. Now that
we got rid of both custom properties and topology names, this
change cleans up the mechanism to pass connector states across
RM helpers and encoder functions.

changes in v5:
	- Introduced in the series

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        | 15 +++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h        |  4 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h   |  3 +-
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c   |  3 +-
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   |  7 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c             | 60 +++++-----------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h             |  2 -
 7 files changed, 27 insertions(+), 67 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0d43525..18f5d1d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -436,15 +436,14 @@  int dpu_encoder_helper_unregister_irq(struct dpu_encoder_phys *phys_enc,
 }
 
 void dpu_encoder_get_hw_resources(struct drm_encoder *drm_enc,
-		struct dpu_encoder_hw_resources *hw_res,
-		struct drm_connector_state *conn_state)
+				  struct dpu_encoder_hw_resources *hw_res)
 {
 	struct dpu_encoder_virt *dpu_enc = NULL;
 	int i = 0;
 
-	if (!hw_res || !drm_enc || !conn_state) {
-		DPU_ERROR("invalid argument(s), drm_enc %d, res %d, state %d\n",
-				drm_enc != 0, hw_res != 0, conn_state != 0);
+	if (!hw_res || !drm_enc) {
+		DPU_ERROR("invalid argument(s), drm_enc %d, res %d\n",
+			  drm_enc != 0, hw_res != 0);
 		return;
 	}
 
@@ -458,7 +457,7 @@  void dpu_encoder_get_hw_resources(struct drm_encoder *drm_enc,
 		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
 
 		if (phys && phys->ops.get_hw_resources)
-			phys->ops.get_hw_resources(phys, hw_res, conn_state);
+			phys->ops.get_hw_resources(phys, hw_res);
 	}
 }
 
@@ -652,7 +651,7 @@  static int dpu_encoder_virt_atomic_check(
 		if (drm_atomic_crtc_needs_modeset(crtc_state)
 				&& dpu_enc->mode_set_complete) {
 			ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, crtc_state,
-				conn_state, topology, true);
+					     topology, true);
 			dpu_enc->mode_set_complete = false;
 		}
 	}
@@ -1044,7 +1043,7 @@  static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 
 	/* Reserve dynamic resources now. Indicating non-AtomicTest phase */
 	ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, drm_enc->crtc->state,
-			conn->state, topology, false);
+			     topology, false);
 	if (ret) {
 		DPU_ERROR_ENC(dpu_enc,
 				"failed to reserve hw resources, %d\n", ret);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index f109b4d..34ac5b6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -51,11 +51,9 @@  struct dpu_encoder_kickoff_params {
  * dpu_encoder_get_hw_resources - Populate table of required hardware resources
  * @encoder:	encoder pointer
  * @hw_res:	resource table to populate with encoder required resources
- * @conn_state:	report hw reqs based on this proposed connector state
  */
 void dpu_encoder_get_hw_resources(struct drm_encoder *encoder,
-		struct dpu_encoder_hw_resources *hw_res,
-		struct drm_connector_state *conn_state);
+				  struct dpu_encoder_hw_resources *hw_res);
 
 /**
  * dpu_encoder_register_vblank_callback - provide callback to encoder that
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index a00b222..3fe4ed9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -140,8 +140,7 @@  struct dpu_encoder_phys_ops {
 			    struct drm_connector_state *conn_state);
 	void (*destroy)(struct dpu_encoder_phys *encoder);
 	void (*get_hw_resources)(struct dpu_encoder_phys *encoder,
-			struct dpu_encoder_hw_resources *hw_res,
-			struct drm_connector_state *conn_state);
+				 struct dpu_encoder_hw_resources *hw_res);
 	int (*control_vblank_irq)(struct dpu_encoder_phys *enc, bool enable);
 	int (*wait_for_commit_done)(struct dpu_encoder_phys *phys_enc);
 	int (*wait_for_tx_complete)(struct dpu_encoder_phys *phys_enc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index 5c89868..f277a69 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -599,8 +599,7 @@  static void dpu_encoder_phys_cmd_destroy(struct dpu_encoder_phys *phys_enc)
 
 static void dpu_encoder_phys_cmd_get_hw_resources(
 		struct dpu_encoder_phys *phys_enc,
-		struct dpu_encoder_hw_resources *hw_res,
-		struct drm_connector_state *conn_state)
+		struct dpu_encoder_hw_resources *hw_res)
 {
 	struct dpu_encoder_phys_cmd *cmd_enc =
 		to_dpu_encoder_phys_cmd(phys_enc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index 6de13f4..fd51fe6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -537,12 +537,11 @@  static void dpu_encoder_phys_vid_destroy(struct dpu_encoder_phys *phys_enc)
 
 static void dpu_encoder_phys_vid_get_hw_resources(
 		struct dpu_encoder_phys *phys_enc,
-		struct dpu_encoder_hw_resources *hw_res,
-		struct drm_connector_state *conn_state)
+		struct dpu_encoder_hw_resources *hw_res)
 {
 	if (!phys_enc || !hw_res) {
-		DPU_ERROR("invalid arg(s), enc %d hw_res %d conn_state %d\n",
-				phys_enc != 0, hw_res != 0, conn_state != 0);
+		DPU_ERROR("invalid arg(s), enc %d hw_res %d\n",
+			  phys_enc != 0, hw_res != 0);
 		return;
 	}
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 86466f0..1164561 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -682,7 +682,6 @@  static int _dpu_rm_make_next_rsvp(
 		struct dpu_rm *rm,
 		struct drm_encoder *enc,
 		struct drm_crtc_state *crtc_state,
-		struct drm_connector_state *conn_state,
 		struct dpu_rm_rsvp *rsvp,
 		struct dpu_rm_requirements *reqs)
 {
@@ -728,7 +727,6 @@  static int _dpu_rm_populate_requirements(
 		struct dpu_rm *rm,
 		struct drm_encoder *enc,
 		struct drm_crtc_state *crtc_state,
-		struct drm_connector_state *conn_state,
 		struct dpu_rm_requirements *reqs,
 		struct msm_display_topology req_topology)
 {
@@ -736,7 +734,7 @@  static int _dpu_rm_populate_requirements(
 
 	memset(reqs, 0, sizeof(*reqs));
 
-	dpu_encoder_get_hw_resources(enc, &reqs->hw_res, conn_state);
+	dpu_encoder_get_hw_resources(enc, &reqs->hw_res);
 
 	for (i = 0; i < DPU_RM_TOPOLOGY_MAX; i++) {
 		if (RM_IS_TOPOLOGY_MATCH(g_top_table[i],
@@ -780,29 +778,12 @@  static struct dpu_rm_rsvp *_dpu_rm_get_rsvp(
 	return NULL;
 }
 
-static struct drm_connector *_dpu_rm_get_connector(
-		struct drm_encoder *enc)
-{
-	struct drm_connector *conn = NULL;
-	struct list_head *connector_list =
-			&enc->dev->mode_config.connector_list;
-
-	list_for_each_entry(conn, connector_list, head)
-		if (conn->encoder == enc)
-			return conn;
-
-	return NULL;
-}
-
 /**
  * _dpu_rm_release_rsvp - release resources and release a reservation
  * @rm:	KMS handle
  * @rsvp:	RSVP pointer to release and release resources for
  */
-static void _dpu_rm_release_rsvp(
-		struct dpu_rm *rm,
-		struct dpu_rm_rsvp *rsvp,
-		struct drm_connector *conn)
+static void _dpu_rm_release_rsvp(struct dpu_rm *rm, struct dpu_rm_rsvp *rsvp)
 {
 	struct dpu_rm_rsvp *rsvp_c, *rsvp_n;
 	struct dpu_rm_hw_blk *blk;
@@ -843,7 +824,6 @@  static void _dpu_rm_release_rsvp(
 void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc)
 {
 	struct dpu_rm_rsvp *rsvp;
-	struct drm_connector *conn;
 
 	if (!rm || !enc) {
 		DPU_ERROR("invalid params\n");
@@ -858,21 +838,12 @@  void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc)
 		goto end;
 	}
 
-	conn = _dpu_rm_get_connector(enc);
-	if (!conn) {
-		DPU_ERROR("failed to get connector for enc %d\n", enc->base.id);
-		goto end;
-	}
-
-	_dpu_rm_release_rsvp(rm, rsvp, conn);
+	_dpu_rm_release_rsvp(rm, rsvp);
 end:
 	mutex_unlock(&rm->rm_lock);
 }
 
-static int _dpu_rm_commit_rsvp(
-		struct dpu_rm *rm,
-		struct dpu_rm_rsvp *rsvp,
-		struct drm_connector_state *conn_state)
+static int _dpu_rm_commit_rsvp(struct dpu_rm *rm, struct dpu_rm_rsvp *rsvp)
 {
 	struct dpu_rm_hw_blk *blk;
 	enum dpu_hw_blk_type type;
@@ -899,7 +870,6 @@  int dpu_rm_reserve(
 		struct dpu_rm *rm,
 		struct drm_encoder *enc,
 		struct drm_crtc_state *crtc_state,
-		struct drm_connector_state *conn_state,
 		struct msm_display_topology topology,
 		bool test_only)
 {
@@ -907,7 +877,7 @@  int dpu_rm_reserve(
 	struct dpu_rm_requirements reqs;
 	int ret;
 
-	if (!rm || !enc || !crtc_state || !conn_state) {
+	if (!rm || !enc || !crtc_state) {
 		DPU_ERROR("invalid arguments\n");
 		return -EINVAL;
 	}
@@ -916,16 +886,15 @@  int dpu_rm_reserve(
 	if (!drm_atomic_crtc_needs_modeset(crtc_state))
 		return 0;
 
-	DRM_DEBUG_KMS("reserving hw for conn %d enc %d crtc %d test_only %d\n",
-		      conn_state->connector->base.id, enc->base.id,
-		      crtc_state->crtc->base.id, test_only);
+	DRM_DEBUG_KMS("reserving hw for enc %d crtc %d test_only %d\n",
+		      enc->base.id, crtc_state->crtc->base.id, test_only);
 
 	mutex_lock(&rm->rm_lock);
 
 	_dpu_rm_print_rsvps(rm, DPU_RM_STAGE_BEGIN);
 
-	ret = _dpu_rm_populate_requirements(rm, enc, crtc_state,
-			conn_state, &reqs, topology);
+	ret = _dpu_rm_populate_requirements(rm, enc, crtc_state, &reqs,
+					    topology);
 	if (ret) {
 		DPU_ERROR("failed to populate hw requirements\n");
 		goto end;
@@ -951,14 +920,13 @@  int dpu_rm_reserve(
 	rsvp_cur = _dpu_rm_get_rsvp(rm, enc);
 
 	/* Check the proposed reservation, store it in hw's "next" field */
-	ret = _dpu_rm_make_next_rsvp(rm, enc, crtc_state, conn_state,
-			rsvp_nxt, &reqs);
+	ret = _dpu_rm_make_next_rsvp(rm, enc, crtc_state, rsvp_nxt, &reqs);
 
 	_dpu_rm_print_rsvps(rm, DPU_RM_STAGE_AFTER_RSVPNEXT);
 
 	if (ret) {
 		DPU_ERROR("failed to reserve hw resources: %d\n", ret);
-		_dpu_rm_release_rsvp(rm, rsvp_nxt, conn_state->connector);
+		_dpu_rm_release_rsvp(rm, rsvp_nxt);
 	} else if (test_only) {
 		/*
 		 * Normally, if test_only, test the reservation and then undo
@@ -967,11 +935,11 @@  int dpu_rm_reserve(
 		 */
 		DPU_DEBUG("test_only: discard test rsvp[s%de%d]\n",
 				rsvp_nxt->seq, rsvp_nxt->enc_id);
-		_dpu_rm_release_rsvp(rm, rsvp_nxt, conn_state->connector);
+		_dpu_rm_release_rsvp(rm, rsvp_nxt);
 	} else {
-		_dpu_rm_release_rsvp(rm, rsvp_cur, conn_state->connector);
+		_dpu_rm_release_rsvp(rm, rsvp_cur);
 
-		ret = _dpu_rm_commit_rsvp(rm, rsvp_nxt, conn_state);
+		_dpu_rm_commit_rsvp(rm, rsvp_nxt);
 	}
 
 	_dpu_rm_print_rsvps(rm, DPU_RM_STAGE_FINAL);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index 3a6a5546..28481a1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -107,7 +107,6 @@  int dpu_rm_init(struct dpu_rm *rm,
  * @rm: DPU Resource Manager handle
  * @drm_enc: DRM Encoder handle
  * @crtc_state: Proposed Atomic DRM CRTC State handle
- * @conn_state: Proposed Atomic DRM Connector State handle
  * @topology: Pointer to topology info for the display
  * @test_only: Atomic-Test phase, discard results (unless property overrides)
  * @Return: 0 on Success otherwise -ERROR
@@ -115,7 +114,6 @@  int dpu_rm_init(struct dpu_rm *rm,
 int dpu_rm_reserve(struct dpu_rm *rm,
 		struct drm_encoder *drm_enc,
 		struct drm_crtc_state *crtc_state,
-		struct drm_connector_state *conn_state,
 		struct msm_display_topology topology,
 		bool test_only);
 

Comments

On Wed, Sep 05, 2018 at 07:08:26PM -0700, Jeykumar Sankaran wrote:
> Connector states were passed around RM to update the custom
> topology connector property with chosen topology data. Now that
> we got rid of both custom properties and topology names, this
> change cleans up the mechanism to pass connector states across
> RM helpers and encoder functions.
> 
> changes in v5:
> 	- Introduced in the series
> 
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        | 15 +++---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h        |  4 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h   |  3 +-
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c   |  3 +-
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   |  7 ++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c             | 60 +++++-----------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h             |  2 -
>  7 files changed, 27 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 0d43525..18f5d1d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -436,15 +436,14 @@ int dpu_encoder_helper_unregister_irq(struct dpu_encoder_phys *phys_enc,
>  }
>  
>  void dpu_encoder_get_hw_resources(struct drm_encoder *drm_enc,
> -		struct dpu_encoder_hw_resources *hw_res,
> -		struct drm_connector_state *conn_state)
> +				  struct dpu_encoder_hw_resources *hw_res)
>  {
>  	struct dpu_encoder_virt *dpu_enc = NULL;
>  	int i = 0;
>  
> -	if (!hw_res || !drm_enc || !conn_state) {
> -		DPU_ERROR("invalid argument(s), drm_enc %d, res %d, state %d\n",
> -				drm_enc != 0, hw_res != 0, conn_state != 0);
> +	if (!hw_res || !drm_enc) {
> +		DPU_ERROR("invalid argument(s), drm_enc %d, res %d\n",
> +			  drm_enc != 0, hw_res != 0);

As far as I can tell both hw_res and drm_enc will be set for all the callers so
this check shouldn't be needed. If you are paranoid you can switch to a
WARN_ON() and get rid of the custom error message but I don't think you need to.

>  		return;
>  	}
>  
> @@ -458,7 +457,7 @@ void dpu_encoder_get_hw_resources(struct drm_encoder *drm_enc,
>  		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
>  
>  		if (phys && phys->ops.get_hw_resources)
> -			phys->ops.get_hw_resources(phys, hw_res, conn_state);
> +			phys->ops.get_hw_resources(phys, hw_res);
>  	}
>  }
>  
> @@ -652,7 +651,7 @@ static int dpu_encoder_virt_atomic_check(
>  		if (drm_atomic_crtc_needs_modeset(crtc_state)
>  				&& dpu_enc->mode_set_complete) {
>  			ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, crtc_state,
> -				conn_state, topology, true);
> +					     topology, true);
>  			dpu_enc->mode_set_complete = false;
>  		}
>  	}
> @@ -1044,7 +1043,7 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>  
>  	/* Reserve dynamic resources now. Indicating non-AtomicTest phase */
>  	ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, drm_enc->crtc->state,
> -			conn->state, topology, false);
> +			     topology, false);
>  	if (ret) {
>  		DPU_ERROR_ENC(dpu_enc,
>  				"failed to reserve hw resources, %d\n", ret);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> index f109b4d..34ac5b6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> @@ -51,11 +51,9 @@ struct dpu_encoder_kickoff_params {
>   * dpu_encoder_get_hw_resources - Populate table of required hardware resources
>   * @encoder:	encoder pointer
>   * @hw_res:	resource table to populate with encoder required resources
> - * @conn_state:	report hw reqs based on this proposed connector state
>   */
>  void dpu_encoder_get_hw_resources(struct drm_encoder *encoder,
> -		struct dpu_encoder_hw_resources *hw_res,
> -		struct drm_connector_state *conn_state);
> +				  struct dpu_encoder_hw_resources *hw_res);
>  
>  /**
>   * dpu_encoder_register_vblank_callback - provide callback to encoder that
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> index a00b222..3fe4ed9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -140,8 +140,7 @@ struct dpu_encoder_phys_ops {
>  			    struct drm_connector_state *conn_state);
>  	void (*destroy)(struct dpu_encoder_phys *encoder);
>  	void (*get_hw_resources)(struct dpu_encoder_phys *encoder,
> -			struct dpu_encoder_hw_resources *hw_res,
> -			struct drm_connector_state *conn_state);
> +				 struct dpu_encoder_hw_resources *hw_res);
>  	int (*control_vblank_irq)(struct dpu_encoder_phys *enc, bool enable);
>  	int (*wait_for_commit_done)(struct dpu_encoder_phys *phys_enc);
>  	int (*wait_for_tx_complete)(struct dpu_encoder_phys *phys_enc);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> index 5c89868..f277a69 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> @@ -599,8 +599,7 @@ static void dpu_encoder_phys_cmd_destroy(struct dpu_encoder_phys *phys_enc)
>  
>  static void dpu_encoder_phys_cmd_get_hw_resources(
>  		struct dpu_encoder_phys *phys_enc,
> -		struct dpu_encoder_hw_resources *hw_res,
> -		struct drm_connector_state *conn_state)
> +		struct dpu_encoder_hw_resources *hw_res)
>  {
>  	struct dpu_encoder_phys_cmd *cmd_enc =
>  		to_dpu_encoder_phys_cmd(phys_enc);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index 6de13f4..fd51fe6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -537,12 +537,11 @@ static void dpu_encoder_phys_vid_destroy(struct dpu_encoder_phys *phys_enc)
>  
>  static void dpu_encoder_phys_vid_get_hw_resources(
>  		struct dpu_encoder_phys *phys_enc,
> -		struct dpu_encoder_hw_resources *hw_res,
> -		struct drm_connector_state *conn_state)
> +		struct dpu_encoder_hw_resources *hw_res)
>  {
>  	if (!phys_enc || !hw_res) {
> -		DPU_ERROR("invalid arg(s), enc %d hw_res %d conn_state %d\n",
> -				phys_enc != 0, hw_res != 0, conn_state != 0);
> +		DPU_ERROR("invalid arg(s), enc %d hw_res %d\n",
> +			  phys_enc != 0, hw_res != 0);

I don't think this is possible either.

>  		return;
>  	}
>  
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index 86466f0..1164561 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -682,7 +682,6 @@ static int _dpu_rm_make_next_rsvp(
>  		struct dpu_rm *rm,
>  		struct drm_encoder *enc,
>  		struct drm_crtc_state *crtc_state,
> -		struct drm_connector_state *conn_state,
>  		struct dpu_rm_rsvp *rsvp,
>  		struct dpu_rm_requirements *reqs)
>  {
> @@ -728,7 +727,6 @@ static int _dpu_rm_populate_requirements(
>  		struct dpu_rm *rm,
>  		struct drm_encoder *enc,
>  		struct drm_crtc_state *crtc_state,
> -		struct drm_connector_state *conn_state,
>  		struct dpu_rm_requirements *reqs,
>  		struct msm_display_topology req_topology)
>  {
> @@ -736,7 +734,7 @@ static int _dpu_rm_populate_requirements(
>  
>  	memset(reqs, 0, sizeof(*reqs));
>  
> -	dpu_encoder_get_hw_resources(enc, &reqs->hw_res, conn_state);
> +	dpu_encoder_get_hw_resources(enc, &reqs->hw_res);
>  
>  	for (i = 0; i < DPU_RM_TOPOLOGY_MAX; i++) {
>  		if (RM_IS_TOPOLOGY_MATCH(g_top_table[i],
> @@ -780,29 +778,12 @@ static struct dpu_rm_rsvp *_dpu_rm_get_rsvp(
>  	return NULL;
>  }
>  
> -static struct drm_connector *_dpu_rm_get_connector(
> -		struct drm_encoder *enc)
> -{
> -	struct drm_connector *conn = NULL;
> -	struct list_head *connector_list =
> -			&enc->dev->mode_config.connector_list;
> -
> -	list_for_each_entry(conn, connector_list, head)
> -		if (conn->encoder == enc)
> -			return conn;
> -
> -	return NULL;
> -}
> -
>  /**
>   * _dpu_rm_release_rsvp - release resources and release a reservation
>   * @rm:	KMS handle
>   * @rsvp:	RSVP pointer to release and release resources for
>   */
> -static void _dpu_rm_release_rsvp(
> -		struct dpu_rm *rm,
> -		struct dpu_rm_rsvp *rsvp,
> -		struct drm_connector *conn)
> +static void _dpu_rm_release_rsvp(struct dpu_rm *rm, struct dpu_rm_rsvp *rsvp)
>  {
>  	struct dpu_rm_rsvp *rsvp_c, *rsvp_n;
>  	struct dpu_rm_hw_blk *blk;
> @@ -843,7 +824,6 @@ static void _dpu_rm_release_rsvp(
>  void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc)
>  {
>  	struct dpu_rm_rsvp *rsvp;
> -	struct drm_connector *conn;
>  
>  	if (!rm || !enc) {
>  		DPU_ERROR("invalid params\n");
> @@ -858,21 +838,12 @@ void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc)
>  		goto end;
>  	}
>  
> -	conn = _dpu_rm_get_connector(enc);
> -	if (!conn) {
> -		DPU_ERROR("failed to get connector for enc %d\n", enc->base.id);
> -		goto end;
> -	}
> -
> -	_dpu_rm_release_rsvp(rm, rsvp, conn);
> +	_dpu_rm_release_rsvp(rm, rsvp);
>  end:
>  	mutex_unlock(&rm->rm_lock);
>  }
>  
> -static int _dpu_rm_commit_rsvp(
> -		struct dpu_rm *rm,
> -		struct dpu_rm_rsvp *rsvp,
> -		struct drm_connector_state *conn_state)
> +static int _dpu_rm_commit_rsvp(struct dpu_rm *rm, struct dpu_rm_rsvp *rsvp)
>  {
>  	struct dpu_rm_hw_blk *blk;
>  	enum dpu_hw_blk_type type;
> @@ -899,7 +870,6 @@ int dpu_rm_reserve(
>  		struct dpu_rm *rm,
>  		struct drm_encoder *enc,
>  		struct drm_crtc_state *crtc_state,
> -		struct drm_connector_state *conn_state,
>  		struct msm_display_topology topology,
>  		bool test_only)
>  {
> @@ -907,7 +877,7 @@ int dpu_rm_reserve(
>  	struct dpu_rm_requirements reqs;
>  	int ret;
>  
> -	if (!rm || !enc || !crtc_state || !conn_state) {
> +	if (!rm || !enc || !crtc_state) {

I also don't think this is possible.

>  		DPU_ERROR("invalid arguments\n");
>  		return -EINVAL;
>  	}

<snip>

Jordan
On Wed, Sep 05, 2018 at 07:08:26PM -0700, Jeykumar Sankaran wrote:
> Connector states were passed around RM to update the custom
> topology connector property with chosen topology data. Now that
> we got rid of both custom properties and topology names, this
> change cleans up the mechanism to pass connector states across
> RM helpers and encoder functions.
> 
> changes in v5:
> 	- Introduced in the series
> 
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>

With Jordan's nits addressed,

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        | 15 +++---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h        |  4 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h   |  3 +-
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c   |  3 +-
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   |  7 ++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c             | 60 +++++-----------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h             |  2 -
>  7 files changed, 27 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 0d43525..18f5d1d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -436,15 +436,14 @@ int dpu_encoder_helper_unregister_irq(struct dpu_encoder_phys *phys_enc,
>  }
>  
>  void dpu_encoder_get_hw_resources(struct drm_encoder *drm_enc,
> -		struct dpu_encoder_hw_resources *hw_res,
> -		struct drm_connector_state *conn_state)
> +				  struct dpu_encoder_hw_resources *hw_res)
>  {
>  	struct dpu_encoder_virt *dpu_enc = NULL;
>  	int i = 0;
>  
> -	if (!hw_res || !drm_enc || !conn_state) {
> -		DPU_ERROR("invalid argument(s), drm_enc %d, res %d, state %d\n",
> -				drm_enc != 0, hw_res != 0, conn_state != 0);
> +	if (!hw_res || !drm_enc) {
> +		DPU_ERROR("invalid argument(s), drm_enc %d, res %d\n",
> +			  drm_enc != 0, hw_res != 0);
>  		return;
>  	}
>  
> @@ -458,7 +457,7 @@ void dpu_encoder_get_hw_resources(struct drm_encoder *drm_enc,
>  		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
>  
>  		if (phys && phys->ops.get_hw_resources)
> -			phys->ops.get_hw_resources(phys, hw_res, conn_state);
> +			phys->ops.get_hw_resources(phys, hw_res);
>  	}
>  }
>  
> @@ -652,7 +651,7 @@ static int dpu_encoder_virt_atomic_check(
>  		if (drm_atomic_crtc_needs_modeset(crtc_state)
>  				&& dpu_enc->mode_set_complete) {
>  			ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, crtc_state,
> -				conn_state, topology, true);
> +					     topology, true);
>  			dpu_enc->mode_set_complete = false;
>  		}
>  	}
> @@ -1044,7 +1043,7 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>  
>  	/* Reserve dynamic resources now. Indicating non-AtomicTest phase */
>  	ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, drm_enc->crtc->state,
> -			conn->state, topology, false);
> +			     topology, false);
>  	if (ret) {
>  		DPU_ERROR_ENC(dpu_enc,
>  				"failed to reserve hw resources, %d\n", ret);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> index f109b4d..34ac5b6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> @@ -51,11 +51,9 @@ struct dpu_encoder_kickoff_params {
>   * dpu_encoder_get_hw_resources - Populate table of required hardware resources
>   * @encoder:	encoder pointer
>   * @hw_res:	resource table to populate with encoder required resources
> - * @conn_state:	report hw reqs based on this proposed connector state
>   */
>  void dpu_encoder_get_hw_resources(struct drm_encoder *encoder,
> -		struct dpu_encoder_hw_resources *hw_res,
> -		struct drm_connector_state *conn_state);
> +				  struct dpu_encoder_hw_resources *hw_res);
>  
>  /**
>   * dpu_encoder_register_vblank_callback - provide callback to encoder that
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> index a00b222..3fe4ed9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -140,8 +140,7 @@ struct dpu_encoder_phys_ops {
>  			    struct drm_connector_state *conn_state);
>  	void (*destroy)(struct dpu_encoder_phys *encoder);
>  	void (*get_hw_resources)(struct dpu_encoder_phys *encoder,
> -			struct dpu_encoder_hw_resources *hw_res,
> -			struct drm_connector_state *conn_state);
> +				 struct dpu_encoder_hw_resources *hw_res);
>  	int (*control_vblank_irq)(struct dpu_encoder_phys *enc, bool enable);
>  	int (*wait_for_commit_done)(struct dpu_encoder_phys *phys_enc);
>  	int (*wait_for_tx_complete)(struct dpu_encoder_phys *phys_enc);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> index 5c89868..f277a69 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> @@ -599,8 +599,7 @@ static void dpu_encoder_phys_cmd_destroy(struct dpu_encoder_phys *phys_enc)
>  
>  static void dpu_encoder_phys_cmd_get_hw_resources(
>  		struct dpu_encoder_phys *phys_enc,
> -		struct dpu_encoder_hw_resources *hw_res,
> -		struct drm_connector_state *conn_state)
> +		struct dpu_encoder_hw_resources *hw_res)
>  {
>  	struct dpu_encoder_phys_cmd *cmd_enc =
>  		to_dpu_encoder_phys_cmd(phys_enc);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index 6de13f4..fd51fe6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -537,12 +537,11 @@ static void dpu_encoder_phys_vid_destroy(struct dpu_encoder_phys *phys_enc)
>  
>  static void dpu_encoder_phys_vid_get_hw_resources(
>  		struct dpu_encoder_phys *phys_enc,
> -		struct dpu_encoder_hw_resources *hw_res,
> -		struct drm_connector_state *conn_state)
> +		struct dpu_encoder_hw_resources *hw_res)
>  {
>  	if (!phys_enc || !hw_res) {
> -		DPU_ERROR("invalid arg(s), enc %d hw_res %d conn_state %d\n",
> -				phys_enc != 0, hw_res != 0, conn_state != 0);
> +		DPU_ERROR("invalid arg(s), enc %d hw_res %d\n",
> +			  phys_enc != 0, hw_res != 0);
>  		return;
>  	}
>  
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index 86466f0..1164561 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -682,7 +682,6 @@ static int _dpu_rm_make_next_rsvp(
>  		struct dpu_rm *rm,
>  		struct drm_encoder *enc,
>  		struct drm_crtc_state *crtc_state,
> -		struct drm_connector_state *conn_state,
>  		struct dpu_rm_rsvp *rsvp,
>  		struct dpu_rm_requirements *reqs)
>  {
> @@ -728,7 +727,6 @@ static int _dpu_rm_populate_requirements(
>  		struct dpu_rm *rm,
>  		struct drm_encoder *enc,
>  		struct drm_crtc_state *crtc_state,
> -		struct drm_connector_state *conn_state,
>  		struct dpu_rm_requirements *reqs,
>  		struct msm_display_topology req_topology)
>  {
> @@ -736,7 +734,7 @@ static int _dpu_rm_populate_requirements(
>  
>  	memset(reqs, 0, sizeof(*reqs));
>  
> -	dpu_encoder_get_hw_resources(enc, &reqs->hw_res, conn_state);
> +	dpu_encoder_get_hw_resources(enc, &reqs->hw_res);
>  
>  	for (i = 0; i < DPU_RM_TOPOLOGY_MAX; i++) {
>  		if (RM_IS_TOPOLOGY_MATCH(g_top_table[i],
> @@ -780,29 +778,12 @@ static struct dpu_rm_rsvp *_dpu_rm_get_rsvp(
>  	return NULL;
>  }
>  
> -static struct drm_connector *_dpu_rm_get_connector(
> -		struct drm_encoder *enc)
> -{
> -	struct drm_connector *conn = NULL;
> -	struct list_head *connector_list =
> -			&enc->dev->mode_config.connector_list;
> -
> -	list_for_each_entry(conn, connector_list, head)
> -		if (conn->encoder == enc)
> -			return conn;
> -
> -	return NULL;
> -}
> -
>  /**
>   * _dpu_rm_release_rsvp - release resources and release a reservation
>   * @rm:	KMS handle
>   * @rsvp:	RSVP pointer to release and release resources for
>   */
> -static void _dpu_rm_release_rsvp(
> -		struct dpu_rm *rm,
> -		struct dpu_rm_rsvp *rsvp,
> -		struct drm_connector *conn)
> +static void _dpu_rm_release_rsvp(struct dpu_rm *rm, struct dpu_rm_rsvp *rsvp)
>  {
>  	struct dpu_rm_rsvp *rsvp_c, *rsvp_n;
>  	struct dpu_rm_hw_blk *blk;
> @@ -843,7 +824,6 @@ static void _dpu_rm_release_rsvp(
>  void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc)
>  {
>  	struct dpu_rm_rsvp *rsvp;
> -	struct drm_connector *conn;
>  
>  	if (!rm || !enc) {
>  		DPU_ERROR("invalid params\n");
> @@ -858,21 +838,12 @@ void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc)
>  		goto end;
>  	}
>  
> -	conn = _dpu_rm_get_connector(enc);
> -	if (!conn) {
> -		DPU_ERROR("failed to get connector for enc %d\n", enc->base.id);
> -		goto end;
> -	}
> -
> -	_dpu_rm_release_rsvp(rm, rsvp, conn);
> +	_dpu_rm_release_rsvp(rm, rsvp);
>  end:
>  	mutex_unlock(&rm->rm_lock);
>  }
>  
> -static int _dpu_rm_commit_rsvp(
> -		struct dpu_rm *rm,
> -		struct dpu_rm_rsvp *rsvp,
> -		struct drm_connector_state *conn_state)
> +static int _dpu_rm_commit_rsvp(struct dpu_rm *rm, struct dpu_rm_rsvp *rsvp)
>  {
>  	struct dpu_rm_hw_blk *blk;
>  	enum dpu_hw_blk_type type;
> @@ -899,7 +870,6 @@ int dpu_rm_reserve(
>  		struct dpu_rm *rm,
>  		struct drm_encoder *enc,
>  		struct drm_crtc_state *crtc_state,
> -		struct drm_connector_state *conn_state,
>  		struct msm_display_topology topology,
>  		bool test_only)
>  {
> @@ -907,7 +877,7 @@ int dpu_rm_reserve(
>  	struct dpu_rm_requirements reqs;
>  	int ret;
>  
> -	if (!rm || !enc || !crtc_state || !conn_state) {
> +	if (!rm || !enc || !crtc_state) {
>  		DPU_ERROR("invalid arguments\n");
>  		return -EINVAL;
>  	}
> @@ -916,16 +886,15 @@ int dpu_rm_reserve(
>  	if (!drm_atomic_crtc_needs_modeset(crtc_state))
>  		return 0;
>  
> -	DRM_DEBUG_KMS("reserving hw for conn %d enc %d crtc %d test_only %d\n",
> -		      conn_state->connector->base.id, enc->base.id,
> -		      crtc_state->crtc->base.id, test_only);
> +	DRM_DEBUG_KMS("reserving hw for enc %d crtc %d test_only %d\n",
> +		      enc->base.id, crtc_state->crtc->base.id, test_only);
>  
>  	mutex_lock(&rm->rm_lock);
>  
>  	_dpu_rm_print_rsvps(rm, DPU_RM_STAGE_BEGIN);
>  
> -	ret = _dpu_rm_populate_requirements(rm, enc, crtc_state,
> -			conn_state, &reqs, topology);
> +	ret = _dpu_rm_populate_requirements(rm, enc, crtc_state, &reqs,
> +					    topology);
>  	if (ret) {
>  		DPU_ERROR("failed to populate hw requirements\n");
>  		goto end;
> @@ -951,14 +920,13 @@ int dpu_rm_reserve(
>  	rsvp_cur = _dpu_rm_get_rsvp(rm, enc);
>  
>  	/* Check the proposed reservation, store it in hw's "next" field */
> -	ret = _dpu_rm_make_next_rsvp(rm, enc, crtc_state, conn_state,
> -			rsvp_nxt, &reqs);
> +	ret = _dpu_rm_make_next_rsvp(rm, enc, crtc_state, rsvp_nxt, &reqs);
>  
>  	_dpu_rm_print_rsvps(rm, DPU_RM_STAGE_AFTER_RSVPNEXT);
>  
>  	if (ret) {
>  		DPU_ERROR("failed to reserve hw resources: %d\n", ret);
> -		_dpu_rm_release_rsvp(rm, rsvp_nxt, conn_state->connector);
> +		_dpu_rm_release_rsvp(rm, rsvp_nxt);
>  	} else if (test_only) {
>  		/*
>  		 * Normally, if test_only, test the reservation and then undo
> @@ -967,11 +935,11 @@ int dpu_rm_reserve(
>  		 */
>  		DPU_DEBUG("test_only: discard test rsvp[s%de%d]\n",
>  				rsvp_nxt->seq, rsvp_nxt->enc_id);
> -		_dpu_rm_release_rsvp(rm, rsvp_nxt, conn_state->connector);
> +		_dpu_rm_release_rsvp(rm, rsvp_nxt);
>  	} else {
> -		_dpu_rm_release_rsvp(rm, rsvp_cur, conn_state->connector);
> +		_dpu_rm_release_rsvp(rm, rsvp_cur);
>  
> -		ret = _dpu_rm_commit_rsvp(rm, rsvp_nxt, conn_state);
> +		_dpu_rm_commit_rsvp(rm, rsvp_nxt);
>  	}
>  
>  	_dpu_rm_print_rsvps(rm, DPU_RM_STAGE_FINAL);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> index 3a6a5546..28481a1 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> @@ -107,7 +107,6 @@ int dpu_rm_init(struct dpu_rm *rm,
>   * @rm: DPU Resource Manager handle
>   * @drm_enc: DRM Encoder handle
>   * @crtc_state: Proposed Atomic DRM CRTC State handle
> - * @conn_state: Proposed Atomic DRM Connector State handle
>   * @topology: Pointer to topology info for the display
>   * @test_only: Atomic-Test phase, discard results (unless property overrides)
>   * @Return: 0 on Success otherwise -ERROR
> @@ -115,7 +114,6 @@ int dpu_rm_init(struct dpu_rm *rm,
>  int dpu_rm_reserve(struct dpu_rm *rm,
>  		struct drm_encoder *drm_enc,
>  		struct drm_crtc_state *crtc_state,
> -		struct drm_connector_state *conn_state,
>  		struct msm_display_topology topology,
>  		bool test_only);
>  
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
On 2018-09-06 09:14, Jordan Crouse wrote:
> On Wed, Sep 05, 2018 at 07:08:26PM -0700, Jeykumar Sankaran wrote:
>> Connector states were passed around RM to update the custom
>> topology connector property with chosen topology data. Now that
>> we got rid of both custom properties and topology names, this
>> change cleans up the mechanism to pass connector states across
>> RM helpers and encoder functions.
>> 
>> changes in v5:
>> 	- Introduced in the series
>> 
>> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
>> ---
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        | 15 +++---
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h        |  4 +-
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h   |  3 +-
>>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c   |  3 +-
>>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   |  7 ++-
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c             | 60
> +++++-----------------
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h             |  2 -
>>  7 files changed, 27 insertions(+), 67 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index 0d43525..18f5d1d 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> @@ -436,15 +436,14 @@ int dpu_encoder_helper_unregister_irq(struct
> dpu_encoder_phys *phys_enc,
>>  }
>> 
>>  void dpu_encoder_get_hw_resources(struct drm_encoder *drm_enc,
>> -		struct dpu_encoder_hw_resources *hw_res,
>> -		struct drm_connector_state *conn_state)
>> +				  struct dpu_encoder_hw_resources *hw_res)
>>  {
>>  	struct dpu_encoder_virt *dpu_enc = NULL;
>>  	int i = 0;
>> 
>> -	if (!hw_res || !drm_enc || !conn_state) {
>> -		DPU_ERROR("invalid argument(s), drm_enc %d, res %d, state
> %d\n",
>> -				drm_enc != 0, hw_res != 0, conn_state !=
> 0);
>> +	if (!hw_res || !drm_enc) {
>> +		DPU_ERROR("invalid argument(s), drm_enc %d, res %d\n",
>> +			  drm_enc != 0, hw_res != 0);
> 
> As far as I can tell both hw_res and drm_enc will be set for all the
> callers so
> this check shouldn't be needed. If you are paranoid you can switch to a
> WARN_ON() and get rid of the custom error message but I don't think you
> need to.
> 
>>  		return;
>>  	}
>> 
>> @@ -458,7 +457,7 @@ void dpu_encoder_get_hw_resources(struct 
>> drm_encoder
> *drm_enc,
>>  		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
>> 
>>  		if (phys && phys->ops.get_hw_resources)
>> -			phys->ops.get_hw_resources(phys, hw_res,
> conn_state);
>> +			phys->ops.get_hw_resources(phys, hw_res);
>>  	}
>>  }
>> 
>> @@ -652,7 +651,7 @@ static int dpu_encoder_virt_atomic_check(
>>  		if (drm_atomic_crtc_needs_modeset(crtc_state)
>>  				&& dpu_enc->mode_set_complete) {
>>  			ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc,
> crtc_state,
>> -				conn_state, topology, true);
>> +					     topology, true);
>>  			dpu_enc->mode_set_complete = false;
>>  		}
>>  	}
>> @@ -1044,7 +1043,7 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
>> 
>>  	/* Reserve dynamic resources now. Indicating non-AtomicTest phase
> */
>>  	ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, drm_enc->crtc->state,
>> -			conn->state, topology, false);
>> +			     topology, false);
>>  	if (ret) {
>>  		DPU_ERROR_ENC(dpu_enc,
>>  				"failed to reserve hw resources, %d\n",
> ret);
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
>> index f109b4d..34ac5b6 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
>> @@ -51,11 +51,9 @@ struct dpu_encoder_kickoff_params {
>>   * dpu_encoder_get_hw_resources - Populate table of required hardware
> resources
>>   * @encoder:	encoder pointer
>>   * @hw_res:	resource table to populate with encoder required 
>> resources
>> - * @conn_state:	report hw reqs based on this proposed connector
> state
>>   */
>>  void dpu_encoder_get_hw_resources(struct drm_encoder *encoder,
>> -		struct dpu_encoder_hw_resources *hw_res,
>> -		struct drm_connector_state *conn_state);
>> +				  struct dpu_encoder_hw_resources
> *hw_res);
>> 
>>  /**
>>   * dpu_encoder_register_vblank_callback - provide callback to encoder
> that
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
>> index a00b222..3fe4ed9 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
>> @@ -140,8 +140,7 @@ struct dpu_encoder_phys_ops {
>>  			    struct drm_connector_state *conn_state);
>>  	void (*destroy)(struct dpu_encoder_phys *encoder);
>>  	void (*get_hw_resources)(struct dpu_encoder_phys *encoder,
>> -			struct dpu_encoder_hw_resources *hw_res,
>> -			struct drm_connector_state *conn_state);
>> +				 struct dpu_encoder_hw_resources *hw_res);
>>  	int (*control_vblank_irq)(struct dpu_encoder_phys *enc, bool
> enable);
>>  	int (*wait_for_commit_done)(struct dpu_encoder_phys *phys_enc);
>>  	int (*wait_for_tx_complete)(struct dpu_encoder_phys *phys_enc);
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
>> index 5c89868..f277a69 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
>> @@ -599,8 +599,7 @@ static void dpu_encoder_phys_cmd_destroy(struct
> dpu_encoder_phys *phys_enc)
>> 
>>  static void dpu_encoder_phys_cmd_get_hw_resources(
>>  		struct dpu_encoder_phys *phys_enc,
>> -		struct dpu_encoder_hw_resources *hw_res,
>> -		struct drm_connector_state *conn_state)
>> +		struct dpu_encoder_hw_resources *hw_res)
>>  {
>>  	struct dpu_encoder_phys_cmd *cmd_enc =
>>  		to_dpu_encoder_phys_cmd(phys_enc);
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
>> index 6de13f4..fd51fe6 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
>> @@ -537,12 +537,11 @@ static void dpu_encoder_phys_vid_destroy(struct
> dpu_encoder_phys *phys_enc)
>> 
>>  static void dpu_encoder_phys_vid_get_hw_resources(
>>  		struct dpu_encoder_phys *phys_enc,
>> -		struct dpu_encoder_hw_resources *hw_res,
>> -		struct drm_connector_state *conn_state)
>> +		struct dpu_encoder_hw_resources *hw_res)
>>  {
>>  	if (!phys_enc || !hw_res) {
>> -		DPU_ERROR("invalid arg(s), enc %d hw_res %d conn_state
> %d\n",
>> -				phys_enc != 0, hw_res != 0, conn_state !=
> 0);
>> +		DPU_ERROR("invalid arg(s), enc %d hw_res %d\n",
>> +			  phys_enc != 0, hw_res != 0);
> 
> I don't think this is possible either.
The next patch in the series take care of these checks in encoder.
https://patchwork.freedesktop.org/patch/247509/

> 
>>  		return;
>>  	}
>> 
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>> index 86466f0..1164561 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>> @@ -682,7 +682,6 @@ static int _dpu_rm_make_next_rsvp(
>>  		struct dpu_rm *rm,
>>  		struct drm_encoder *enc,
>>  		struct drm_crtc_state *crtc_state,
>> -		struct drm_connector_state *conn_state,
>>  		struct dpu_rm_rsvp *rsvp,
>>  		struct dpu_rm_requirements *reqs)
>>  {
>> @@ -728,7 +727,6 @@ static int _dpu_rm_populate_requirements(
>>  		struct dpu_rm *rm,
>>  		struct drm_encoder *enc,
>>  		struct drm_crtc_state *crtc_state,
>> -		struct drm_connector_state *conn_state,
>>  		struct dpu_rm_requirements *reqs,
>>  		struct msm_display_topology req_topology)
>>  {
>> @@ -736,7 +734,7 @@ static int _dpu_rm_populate_requirements(
>> 
>>  	memset(reqs, 0, sizeof(*reqs));
>> 
>> -	dpu_encoder_get_hw_resources(enc, &reqs->hw_res, conn_state);
>> +	dpu_encoder_get_hw_resources(enc, &reqs->hw_res);
>> 
>>  	for (i = 0; i < DPU_RM_TOPOLOGY_MAX; i++) {
>>  		if (RM_IS_TOPOLOGY_MATCH(g_top_table[i],
>> @@ -780,29 +778,12 @@ static struct dpu_rm_rsvp *_dpu_rm_get_rsvp(
>>  	return NULL;
>>  }
>> 
>> -static struct drm_connector *_dpu_rm_get_connector(
>> -		struct drm_encoder *enc)
>> -{
>> -	struct drm_connector *conn = NULL;
>> -	struct list_head *connector_list =
>> -			&enc->dev->mode_config.connector_list;
>> -
>> -	list_for_each_entry(conn, connector_list, head)
>> -		if (conn->encoder == enc)
>> -			return conn;
>> -
>> -	return NULL;
>> -}
>> -
>>  /**
>>   * _dpu_rm_release_rsvp - release resources and release a reservation
>>   * @rm:	KMS handle
>>   * @rsvp:	RSVP pointer to release and release resources for
>>   */
>> -static void _dpu_rm_release_rsvp(
>> -		struct dpu_rm *rm,
>> -		struct dpu_rm_rsvp *rsvp,
>> -		struct drm_connector *conn)
>> +static void _dpu_rm_release_rsvp(struct dpu_rm *rm, struct 
>> dpu_rm_rsvp
> *rsvp)
>>  {
>>  	struct dpu_rm_rsvp *rsvp_c, *rsvp_n;
>>  	struct dpu_rm_hw_blk *blk;
>> @@ -843,7 +824,6 @@ static void _dpu_rm_release_rsvp(
>>  void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc)
>>  {
>>  	struct dpu_rm_rsvp *rsvp;
>> -	struct drm_connector *conn;
>> 
>>  	if (!rm || !enc) {
>>  		DPU_ERROR("invalid params\n");
>> @@ -858,21 +838,12 @@ void dpu_rm_release(struct dpu_rm *rm, struct
> drm_encoder *enc)
>>  		goto end;
>>  	}
>> 
>> -	conn = _dpu_rm_get_connector(enc);
>> -	if (!conn) {
>> -		DPU_ERROR("failed to get connector for enc %d\n",
> enc->base.id);
>> -		goto end;
>> -	}
>> -
>> -	_dpu_rm_release_rsvp(rm, rsvp, conn);
>> +	_dpu_rm_release_rsvp(rm, rsvp);
>>  end:
>>  	mutex_unlock(&rm->rm_lock);
>>  }
>> 
>> -static int _dpu_rm_commit_rsvp(
>> -		struct dpu_rm *rm,
>> -		struct dpu_rm_rsvp *rsvp,
>> -		struct drm_connector_state *conn_state)
>> +static int _dpu_rm_commit_rsvp(struct dpu_rm *rm, struct dpu_rm_rsvp
> *rsvp)
>>  {
>>  	struct dpu_rm_hw_blk *blk;
>>  	enum dpu_hw_blk_type type;
>> @@ -899,7 +870,6 @@ int dpu_rm_reserve(
>>  		struct dpu_rm *rm,
>>  		struct drm_encoder *enc,
>>  		struct drm_crtc_state *crtc_state,
>> -		struct drm_connector_state *conn_state,
>>  		struct msm_display_topology topology,
>>  		bool test_only)
>>  {
>> @@ -907,7 +877,7 @@ int dpu_rm_reserve(
>>  	struct dpu_rm_requirements reqs;
>>  	int ret;
>> 
>> -	if (!rm || !enc || !crtc_state || !conn_state) {
>> +	if (!rm || !enc || !crtc_state) {
> 
> I also don't think this is possible.
Will remove the checks!
> 
>>  		DPU_ERROR("invalid arguments\n");
>>  		return -EINVAL;
>>  	}
> 
> <snip>
> 
> Jordan