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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx>
---
 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

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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux