diff mbox series

[v3,07/13] drm/msm/dpu: avoid querying for hw intf before assignment

Message ID 1533697956-29686-8-git-send-email-jsanka@codeaurora.org (mailing list archive)
State New, archived
Headers show
Series Atomic resource management | expand

Commit Message

Jeykumar Sankaran Aug. 8, 2018, 3:12 a.m. UTC 
hw intf blocks are needed only during encoder enable to program
timing engines(for video panels). encoder->enable is triggered
only after atomic_modeset at which point we assign the
resources for the display pipeline. This patch defers the
hw_intf look-up until encoder enable.

changes in v2:
	- none
changes in v3:
	- none

Change-Id: Ib0a2253431468151355e50cbad7b91e2b77b6e54
Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   | 53 +++++++---------------
 1 file changed, 16 insertions(+), 37 deletions(-)

Comments

Sean Paul Aug. 14, 2018, 7:37 p.m. UTC | #1 
On Tue, Aug 07, 2018 at 08:12:34PM -0700, Jeykumar Sankaran wrote:
> hw intf blocks are needed only during encoder enable to program
> timing engines(for video panels). encoder->enable is triggered
> only after atomic_modeset at which point we assign the
> resources for the display pipeline. This patch defers the
> hw_intf look-up until encoder enable.

I'm sure there's a good reason for this, but you haven't stated it. As a casual
reader of the patch, I'd be inclined to prefer doing work only once at init vs
everytime we enable.

> 
> changes in v2:
> 	- none
> changes in v3:
> 	- none
> 
> Change-Id: Ib0a2253431468151355e50cbad7b91e2b77b6e54
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> ---
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   | 53 +++++++---------------
>  1 file changed, 16 insertions(+), 37 deletions(-)
> 
> 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 c0221cc..a0b3744 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
> @@ -462,7 +462,7 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
>  {
>  	struct msm_drm_private *priv;
>  	struct dpu_encoder_phys_vid *vid_enc;
> -	struct dpu_hw_intf *intf;
> +	struct dpu_rm_hw_iter iter;
>  	struct dpu_hw_ctl *ctl;
>  	u32 flush_mask = 0;
>  
> @@ -474,11 +474,20 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
>  	priv = phys_enc->parent->dev->dev_private;
>  
>  	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
> -	intf = vid_enc->hw_intf;
>  	ctl = phys_enc->hw_ctl;
> -	if (!vid_enc->hw_intf || !phys_enc->hw_ctl) {
> -		DPU_ERROR("invalid hw_intf %d hw_ctl %d\n",
> -				vid_enc->hw_intf != 0, phys_enc->hw_ctl != 0);
> +
> +	dpu_rm_init_hw_iter(&iter, phys_enc->parent->base.id, DPU_HW_BLK_INTF);
> +	while (dpu_rm_get_hw(&phys_enc->dpu_kms->rm, &iter)) {
> +		struct dpu_hw_intf *hw_intf = (struct dpu_hw_intf *)iter.hw;
> +
> +		if (hw_intf->idx == phys_enc->intf_idx) {
> +			vid_enc->hw_intf = hw_intf;
> +			break;
> +		}
> +	}
> +
> +	if (!vid_enc->hw_intf) {
> +		DPU_ERROR("hw_intf not assigned\n");
>  		return;
>  	}
>  
> @@ -500,7 +509,7 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
>  		!dpu_encoder_phys_vid_is_master(phys_enc))
>  		goto skip_flush;
>  
> -	ctl->ops.get_bitmask_intf(ctl, &flush_mask, intf->idx);
> +	ctl->ops.get_bitmask_intf(ctl, &flush_mask, vid_enc->hw_intf->idx);
>  	ctl->ops.update_pending_flush(ctl, flush_mask);
>  
>  skip_flush:
> @@ -531,22 +540,13 @@ static void dpu_encoder_phys_vid_get_hw_resources(
>  		struct dpu_encoder_hw_resources *hw_res,
>  		struct drm_connector_state *conn_state)
>  {
> -	struct dpu_encoder_phys_vid *vid_enc;
> -
>  	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);
>  		return;
>  	}
>  
> -	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
> -	if (!vid_enc->hw_intf) {
> -		DPU_ERROR("invalid arg(s), hw_intf\n");
> -		return;
> -	}
> -
> -	DPU_DEBUG_VIDENC(vid_enc, "\n");
> -	hw_res->intfs[vid_enc->hw_intf->idx - INTF_0] = INTF_MODE_VIDEO;
> +	hw_res->intfs[phys_enc->intf_idx - INTF_0] = INTF_MODE_VIDEO;
>  }
>  
>  static int _dpu_encoder_phys_vid_wait_for_vblank(
> @@ -809,7 +809,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
>  {
>  	struct dpu_encoder_phys *phys_enc = NULL;
>  	struct dpu_encoder_phys_vid *vid_enc = NULL;
> -	struct dpu_rm_hw_iter iter;
>  	struct dpu_encoder_irq *irq;
>  	int i, ret = 0;
>  
> @@ -829,26 +828,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
>  	phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
>  	phys_enc->intf_idx = p->intf_idx;
>  
> -	/**
> -	 * hw_intf resource permanently assigned to this encoder
> -	 * Other resources allocated at atomic commit time by use case
> -	 */
> -	dpu_rm_init_hw_iter(&iter, 0, DPU_HW_BLK_INTF);
> -	while (dpu_rm_get_hw(&p->dpu_kms->rm, &iter)) {
> -		struct dpu_hw_intf *hw_intf = (struct dpu_hw_intf *)iter.hw;
> -
> -		if (hw_intf->idx == p->intf_idx) {
> -			vid_enc->hw_intf = hw_intf;
> -			break;
> -		}
> -	}
> -
> -	if (!vid_enc->hw_intf) {
> -		ret = -EINVAL;
> -		DPU_ERROR("failed to get hw_intf\n");
> -		goto fail;
> -	}
> -
>  	DPU_DEBUG_VIDENC(vid_enc, "\n");
>  
>  	dpu_encoder_phys_vid_init_ops(&phys_enc->ops);
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
diff mbox series

Patch

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 c0221cc..a0b3744 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
@@ -462,7 +462,7 @@  static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
 {
 	struct msm_drm_private *priv;
 	struct dpu_encoder_phys_vid *vid_enc;
-	struct dpu_hw_intf *intf;
+	struct dpu_rm_hw_iter iter;
 	struct dpu_hw_ctl *ctl;
 	u32 flush_mask = 0;
 
@@ -474,11 +474,20 @@  static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
 	priv = phys_enc->parent->dev->dev_private;
 
 	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
-	intf = vid_enc->hw_intf;
 	ctl = phys_enc->hw_ctl;
-	if (!vid_enc->hw_intf || !phys_enc->hw_ctl) {
-		DPU_ERROR("invalid hw_intf %d hw_ctl %d\n",
-				vid_enc->hw_intf != 0, phys_enc->hw_ctl != 0);
+
+	dpu_rm_init_hw_iter(&iter, phys_enc->parent->base.id, DPU_HW_BLK_INTF);
+	while (dpu_rm_get_hw(&phys_enc->dpu_kms->rm, &iter)) {
+		struct dpu_hw_intf *hw_intf = (struct dpu_hw_intf *)iter.hw;
+
+		if (hw_intf->idx == phys_enc->intf_idx) {
+			vid_enc->hw_intf = hw_intf;
+			break;
+		}
+	}
+
+	if (!vid_enc->hw_intf) {
+		DPU_ERROR("hw_intf not assigned\n");
 		return;
 	}
 
@@ -500,7 +509,7 @@  static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
 		!dpu_encoder_phys_vid_is_master(phys_enc))
 		goto skip_flush;
 
-	ctl->ops.get_bitmask_intf(ctl, &flush_mask, intf->idx);
+	ctl->ops.get_bitmask_intf(ctl, &flush_mask, vid_enc->hw_intf->idx);
 	ctl->ops.update_pending_flush(ctl, flush_mask);
 
 skip_flush:
@@ -531,22 +540,13 @@  static void dpu_encoder_phys_vid_get_hw_resources(
 		struct dpu_encoder_hw_resources *hw_res,
 		struct drm_connector_state *conn_state)
 {
-	struct dpu_encoder_phys_vid *vid_enc;
-
 	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);
 		return;
 	}
 
-	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
-	if (!vid_enc->hw_intf) {
-		DPU_ERROR("invalid arg(s), hw_intf\n");
-		return;
-	}
-
-	DPU_DEBUG_VIDENC(vid_enc, "\n");
-	hw_res->intfs[vid_enc->hw_intf->idx - INTF_0] = INTF_MODE_VIDEO;
+	hw_res->intfs[phys_enc->intf_idx - INTF_0] = INTF_MODE_VIDEO;
 }
 
 static int _dpu_encoder_phys_vid_wait_for_vblank(
@@ -809,7 +809,6 @@  struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
 {
 	struct dpu_encoder_phys *phys_enc = NULL;
 	struct dpu_encoder_phys_vid *vid_enc = NULL;
-	struct dpu_rm_hw_iter iter;
 	struct dpu_encoder_irq *irq;
 	int i, ret = 0;
 
@@ -829,26 +828,6 @@  struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
 	phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
 	phys_enc->intf_idx = p->intf_idx;
 
-	/**
-	 * hw_intf resource permanently assigned to this encoder
-	 * Other resources allocated at atomic commit time by use case
-	 */
-	dpu_rm_init_hw_iter(&iter, 0, DPU_HW_BLK_INTF);
-	while (dpu_rm_get_hw(&p->dpu_kms->rm, &iter)) {
-		struct dpu_hw_intf *hw_intf = (struct dpu_hw_intf *)iter.hw;
-
-		if (hw_intf->idx == p->intf_idx) {
-			vid_enc->hw_intf = hw_intf;
-			break;
-		}
-	}
-
-	if (!vid_enc->hw_intf) {
-		ret = -EINVAL;
-		DPU_ERROR("failed to get hw_intf\n");
-		goto fail;
-	}
-
 	DPU_DEBUG_VIDENC(vid_enc, "\n");
 
 	dpu_encoder_phys_vid_init_ops(&phys_enc->ops);