linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Paul <sean@poorly.run>
To: Jeykumar Sankaran <jsanka@codeaurora.org>
Cc: linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	seanpaul@chromium.org, hoegsberg@google.com,
	freedreno@lists.freedesktop.org
Subject: Re: [PATCH v5 17/19] drm/msm/dpu: remove RM dependency on connector state
Date: Fri, 7 Sep 2018 09:30:28 -0400	[thread overview]
Message-ID: <20180907133028.GE188300@art_vandelay> (raw)
In-Reply-To: <1536199708-23664-18-git-send-email-jsanka@codeaurora.org>

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
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2018-09-07 13:30 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-06  2:08 [PATCH v5 00/19] clean up DPU for RM refactor Jeykumar Sankaran
     [not found] ` <1536199708-23664-1-git-send-email-jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-09-06  2:08   ` [PATCH v5 01/19] drm/msm/dpu: remove debugfs support for misr Jeykumar Sankaran
     [not found]     ` <1536199708-23664-2-git-send-email-jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-09-07 12:58       ` Sean Paul
2018-09-06  2:08   ` [PATCH v5 02/19] drm/msm/dpu: squash power handle event types Jeykumar Sankaran
     [not found]     ` <1536199708-23664-3-git-send-email-jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-09-07 13:07       ` Sean Paul
2018-09-06  2:08   ` [PATCH v5 03/19] drm/msm/dpu: remove scalar config definitions Jeykumar Sankaran
2018-09-06  2:08   ` [PATCH v5 04/19] drm/msm/dpu: remove resource pool manager Jeykumar Sankaran
2018-09-06  2:08   ` [PATCH v5 05/19] drm/msm/dpu: remove ping pong split topology variables Jeykumar Sankaran
2018-09-06  2:08   ` [PATCH v5 06/19] drm/msm/dpu: enable master-slave encoders explicitly Jeykumar Sankaran
2018-09-06  2:08   ` [PATCH v5 07/19] drm/msm/dpu: use kms stored hw mdp block Jeykumar Sankaran
2018-09-06  2:08   ` [PATCH v5 08/19] drm/msm/dpu: iterate for assigned hw ctl in virtual encoder Jeykumar Sankaran
2018-09-06  2:08   ` [PATCH v5 09/19] drm/msm/dpu: avoid querying for hw intf before assignment Jeykumar Sankaran
2018-09-06  2:08   ` [PATCH v5 10/19] drm/msm/dpu: make crtc get_mixer_width helper static Jeykumar Sankaran
2018-09-06  2:08   ` [PATCH v5 11/19] drm/msm/dpu: move hw resource tracking to crtc state Jeykumar Sankaran
2018-09-06  2:08   ` [PATCH v5 12/19] drm/msm/dpu: rename hw_ctl to lm_ctl Jeykumar Sankaran
2018-09-06  2:08   ` [PATCH v5 13/19] drm/msm/dpu: clean up destination scaler residue Jeykumar Sankaran
2018-09-06  2:08   ` [PATCH v5 14/19] drm/msm/dpu: remove cdm block support from resource manager Jeykumar Sankaran
2018-09-06  2:08   ` [PATCH v5 15/19] drm/msm/dpu: remove LOCK/CLEAR support in RM Jeykumar Sankaran
2018-09-06  2:08   ` [PATCH v5 16/19] drm/msm/dpu: remove display H_TILE from encoder Jeykumar Sankaran
2018-09-06  2:08   ` [PATCH v5 17/19] drm/msm/dpu: remove RM dependency on connector state Jeykumar Sankaran
     [not found]     ` <1536199708-23664-18-git-send-email-jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-09-06 16:14       ` Jordan Crouse
     [not found]         ` <20180906161428.GC13368-9PYrDHPZ2Orvke4nUoYGnHL1okKdlPRT@public.gmane.org>
2018-09-07 21:02           ` Jeykumar Sankaran
2018-09-07 13:30     ` Sean Paul [this message]
2018-09-06  2:08   ` [PATCH v5 18/19] drm/msm/dpu: relax parameter validation in encoders Jeykumar Sankaran
     [not found]     ` <1536199708-23664-19-git-send-email-jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-09-07 13:30       ` Sean Paul
2018-09-06  2:08   ` [PATCH v5 19/19] drm/msm/dpu: remove RM topology definition Jeykumar Sankaran
     [not found]     ` <1536199708-23664-20-git-send-email-jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-09-07 13:29       ` Sean Paul
2018-09-07 13:41   ` [PATCH v5 00/19] clean up DPU for RM refactor Sean Paul

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180907133028.GE188300@art_vandelay \
    --to=sean@poorly.run \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=hoegsberg@google.com \
    --cc=jsanka@codeaurora.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=seanpaul@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).