[Freedreno] [PATCH v2 6/7] drm/msm/dpu: get rid of dpu_encoder_helper_(un)register_irq

abhinavk at codeaurora.org abhinavk at codeaurora.org
Wed Aug 18 03:35:09 UTC 2021


On 2021-06-17 15:20, Dmitry Baryshkov wrote:
> Get rid of dpu_encoder_helper_register_irq/unregister_irq helpers, call
> dpu_core_register/unregister_callback directly, without surrounding 
> them
> with helpers.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
Reviewed-by: Abhinav Kumar <abhinavk at codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 64 -------------------
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  | 18 ------
>  .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c  | 39 +++++++----
>  .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  | 21 ++++--
>  .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c |  4 ++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h     | 29 +++------
>  6 files changed, 56 insertions(+), 119 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index d3557b0f4db9..3d8864df8605 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -333,70 +333,6 @@ int dpu_encoder_helper_wait_for_irq(struct
> dpu_encoder_phys *phys_enc,
>  	return ret;
>  }
> 
> -int dpu_encoder_helper_register_irq(struct dpu_encoder_phys *phys_enc,
> -		enum dpu_intr_idx intr_idx)
> -{
> -	struct dpu_encoder_irq *irq;
> -	int ret = 0;
> -
> -	if (intr_idx >= INTR_IDX_MAX) {
> -		DPU_ERROR("invalid params\n");
> -		return -EINVAL;
> -	}
> -	irq = &phys_enc->irq[intr_idx];
> -
> -	if (irq->irq_idx < 0) {
> -		DPU_ERROR_PHYS(phys_enc,
> -			"invalid IRQ index:%d\n", irq->irq_idx);
> -		return -EINVAL;
> -	}
> -
> -	ret = dpu_core_irq_register_callback(phys_enc->dpu_kms, irq->irq_idx,
> -			irq->func, phys_enc);
> -	if (ret) {
> -		DPU_ERROR_PHYS(phys_enc,
> -			"failed to register IRQ callback for %s\n",
> -			irq->name);
> -		irq->irq_idx = -EINVAL;
> -		return ret;
> -	}
> -
> -	trace_dpu_enc_irq_register_success(DRMID(phys_enc->parent), intr_idx,
> -				irq->irq_idx);
> -
> -	return ret;
> -}
> -
> -int dpu_encoder_helper_unregister_irq(struct dpu_encoder_phys 
> *phys_enc,
> -		enum dpu_intr_idx intr_idx)
> -{
> -	struct dpu_encoder_irq *irq;
> -	int ret;
> -
> -	irq = &phys_enc->irq[intr_idx];
> -
> -	/* silently skip irqs that weren't registered */
> -	if (irq->irq_idx < 0) {
> -		DRM_ERROR("duplicate unregister id=%u, intr=%d, irq=%d",
> -			  DRMID(phys_enc->parent), intr_idx,
> -			  irq->irq_idx);
> -		return 0;
> -	}
> -
> -	ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms, 
> irq->irq_idx,
> -			irq->func, phys_enc);
> -	if (ret) {
> -		DRM_ERROR("unreg cb fail id=%u, intr=%d, irq=%d ret=%d",
> -			  DRMID(phys_enc->parent), intr_idx,
> -			  irq->irq_idx, ret);
> -	}
> -
> -	trace_dpu_enc_irq_unregister_success(DRMID(phys_enc->parent), 
> intr_idx,
> -					     irq->irq_idx);
> -
> -	return 0;
> -}
> -
>  int dpu_encoder_get_frame_count(struct drm_encoder *drm_enc)
>  {
>  	struct dpu_encoder_virt *dpu_enc;
> 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 80d87871fd94..ff2218155b44 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -364,22 +364,4 @@ int dpu_encoder_helper_wait_for_irq(struct
> dpu_encoder_phys *phys_enc,
>  		enum dpu_intr_idx intr_idx,
>  		struct dpu_encoder_wait_info *wait_info);
> 
> -/**
> - * dpu_encoder_helper_register_irq - register and enable an irq
> - * @phys_enc: Pointer to physical encoder structure
> - * @intr_idx: encoder interrupt index
> - * @Return: 0 or -ERROR
> - */
> -int dpu_encoder_helper_register_irq(struct dpu_encoder_phys *phys_enc,
> -		enum dpu_intr_idx intr_idx);
> -
> -/**
> - * dpu_encoder_helper_unregister_irq - unregister and disable an irq
> - * @phys_enc: Pointer to physical encoder structure
> - * @intr_idx: encoder interrupt index
> - * @Return: 0 or -ERROR
> - */
> -int dpu_encoder_helper_unregister_irq(struct dpu_encoder_phys 
> *phys_enc,
> -		enum dpu_intr_idx intr_idx);
> -
>  #endif /* __dpu_encoder_phys_H__ */
> 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 f921a5c99456..4bfeac821f51 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
> @@ -211,7 +211,9 @@ static int 
> _dpu_encoder_phys_cmd_handle_ppdone_timeout(
>  			  cmd_enc->pp_timeout_report_cnt,
>  			  atomic_read(&phys_enc->pending_kickoff_cnt));
>  		msm_disp_snapshot_state(drm_enc->dev);
> -		dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_RDPTR);
> +		dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
> +				phys_enc->irq[INTR_IDX_RDPTR].irq_idx,
> +				phys_enc->irq[INTR_IDX_RDPTR].func, phys_enc);
>  	}
> 
>  	atomic_add_unless(&phys_enc->pending_kickoff_cnt, -1, 0);
> @@ -277,10 +279,13 @@ static int 
> dpu_encoder_phys_cmd_control_vblank_irq(
>  		      enable ? "true" : "false", refcount);
> 
>  	if (enable && atomic_inc_return(&phys_enc->vblank_refcount) == 1)
> -		ret = dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_RDPTR);
> +		ret = dpu_core_irq_register_callback(phys_enc->dpu_kms,
> +				phys_enc->irq[INTR_IDX_RDPTR].irq_idx,
> +				phys_enc->irq[INTR_IDX_RDPTR].func, phys_enc);
>  	else if (!enable && atomic_dec_return(&phys_enc->vblank_refcount) == 
> 0)
> -		ret = dpu_encoder_helper_unregister_irq(phys_enc,
> -				INTR_IDX_RDPTR);
> +		ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
> +				phys_enc->irq[INTR_IDX_RDPTR].irq_idx,
> +				phys_enc->irq[INTR_IDX_RDPTR].func, phys_enc);
> 
>  end:
>  	if (ret) {
> @@ -301,21 +306,31 @@ static void
> dpu_encoder_phys_cmd_irq_control(struct dpu_encoder_phys *phys_enc,
>  			enable, atomic_read(&phys_enc->vblank_refcount));
> 
>  	if (enable) {
> -		dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_PINGPONG);
> -		dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_UNDERRUN);
> +		dpu_core_irq_register_callback(phys_enc->dpu_kms,
> +				phys_enc->irq[INTR_IDX_PINGPONG].irq_idx,
> +				phys_enc->irq[INTR_IDX_PINGPONG].func, phys_enc);
> +		dpu_core_irq_register_callback(phys_enc->dpu_kms,
> +				phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx,
> +				phys_enc->irq[INTR_IDX_UNDERRUN].func, phys_enc);
>  		dpu_encoder_phys_cmd_control_vblank_irq(phys_enc, true);
> 
>  		if (dpu_encoder_phys_cmd_is_master(phys_enc))
> -			dpu_encoder_helper_register_irq(phys_enc,
> -					INTR_IDX_CTL_START);
> +			dpu_core_irq_register_callback(phys_enc->dpu_kms,
> +					phys_enc->irq[INTR_IDX_CTL_START].irq_idx,
> +					phys_enc->irq[INTR_IDX_CTL_START].func, phys_enc);
>  	} else {
>  		if (dpu_encoder_phys_cmd_is_master(phys_enc))
> -			dpu_encoder_helper_unregister_irq(phys_enc,
> -					INTR_IDX_CTL_START);
> +			dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
> +					phys_enc->irq[INTR_IDX_CTL_START].irq_idx,
> +					phys_enc->irq[INTR_IDX_CTL_START].func, phys_enc);
> 
> -		dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_UNDERRUN);
> +		dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
> +				phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx,
> +				phys_enc->irq[INTR_IDX_UNDERRUN].func, phys_enc);
>  		dpu_encoder_phys_cmd_control_vblank_irq(phys_enc, false);
> -		dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_PINGPONG);
> +		dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
> +				phys_enc->irq[INTR_IDX_PINGPONG].irq_idx,
> +				phys_enc->irq[INTR_IDX_PINGPONG].func, 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 437af231d6a4..7f605287a377 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
> @@ -404,10 +404,13 @@ static int 
> dpu_encoder_phys_vid_control_vblank_irq(
>  		      atomic_read(&phys_enc->vblank_refcount));
> 
>  	if (enable && atomic_inc_return(&phys_enc->vblank_refcount) == 1)
> -		ret = dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_VSYNC);
> +		ret = dpu_core_irq_register_callback(phys_enc->dpu_kms,
> +				phys_enc->irq[INTR_IDX_VSYNC].irq_idx,
> +				phys_enc->irq[INTR_IDX_VSYNC].func, phys_enc);
>  	else if (!enable && atomic_dec_return(&phys_enc->vblank_refcount) == 
> 0)
> -		ret = dpu_encoder_helper_unregister_irq(phys_enc,
> -				INTR_IDX_VSYNC);
> +		ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
> +				phys_enc->irq[INTR_IDX_VSYNC].irq_idx,
> +				phys_enc->irq[INTR_IDX_VSYNC].func, phys_enc);
> 
>  end:
>  	if (ret) {
> @@ -539,7 +542,9 @@ static void 
> dpu_encoder_phys_vid_prepare_for_kickoff(
>  		DPU_ERROR_VIDENC(phys_enc, "ctl %d reset failure: %d\n",
>  				ctl->idx, rc);
>  		msm_disp_snapshot_state(drm_enc->dev);
> -		dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_VSYNC);
> +		dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
> +				phys_enc->irq[INTR_IDX_VSYNC].irq_idx,
> +				phys_enc->irq[INTR_IDX_VSYNC].func, phys_enc);
>  	}
>  }
> 
> @@ -628,10 +633,14 @@ static void
> dpu_encoder_phys_vid_irq_control(struct dpu_encoder_phys *phys_enc,
>  		if (WARN_ON(ret))
>  			return;
> 
> -		dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_UNDERRUN);
> +		dpu_core_irq_register_callback(phys_enc->dpu_kms,
> +				phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx,
> +				phys_enc->irq[INTR_IDX_UNDERRUN].func, phys_enc);
>  	} else {
>  		dpu_encoder_phys_vid_control_vblank_irq(phys_enc, false);
> -		dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_UNDERRUN);
> +		dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
> +				phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx,
> +				phys_enc->irq[INTR_IDX_UNDERRUN].func, phys_enc);
>  	}
>  }
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> index 9b74cfdf5355..557ad831e651 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> @@ -451,6 +451,8 @@ int dpu_core_irq_register_callback(struct dpu_kms
> *dpu_kms, int irq_idx,
>  					irq_idx);
>  	spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);
> 
> +	trace_dpu_irq_register_success(irq_idx);
> +
>  	return 0;
>  }
> 
> @@ -493,6 +495,8 @@ int dpu_core_irq_unregister_callback(struct
> dpu_kms *dpu_kms, int irq_idx,
> 
>  	spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);
> 
> +	trace_dpu_irq_unregister_success(irq_idx);
> +
>  	return 0;
>  }
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> index 1e2619556f93..58b7b8654543 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> @@ -167,33 +167,24 @@ TRACE_EVENT(dpu_perf_crtc_update,
>  			__entry->update_clk)
>  );
> 
> -DECLARE_EVENT_CLASS(dpu_enc_irq_template,
> -	TP_PROTO(uint32_t drm_id, enum dpu_intr_idx intr_idx,
> -		 int irq_idx),
> -	TP_ARGS(drm_id, intr_idx, irq_idx),
> +DECLARE_EVENT_CLASS(dpu_irq_template,
> +	TP_PROTO(int irq_idx),
> +	TP_ARGS(irq_idx),
>  	TP_STRUCT__entry(
> -		__field(	uint32_t,		drm_id		)
> -		__field(	enum dpu_intr_idx,	intr_idx	)
>  		__field(	int,			irq_idx		)
>  	),
>  	TP_fast_assign(
> -		__entry->drm_id = drm_id;
> -		__entry->intr_idx = intr_idx;
>  		__entry->irq_idx = irq_idx;
>  	),
> -	TP_printk("id=%u, intr=%d, irq=%d",
> -		  __entry->drm_id, __entry->intr_idx,
> -		  __entry->irq_idx)
> +	TP_printk("irq=%d", __entry->irq_idx)
>  );
> -DEFINE_EVENT(dpu_enc_irq_template, dpu_enc_irq_register_success,
> -	TP_PROTO(uint32_t drm_id, enum dpu_intr_idx intr_idx,
> -		 int irq_idx),
> -	TP_ARGS(drm_id, intr_idx, irq_idx)
> +DEFINE_EVENT(dpu_irq_template, dpu_irq_register_success,
> +	TP_PROTO(int irq_idx),
> +	TP_ARGS(irq_idx)
>  );
> -DEFINE_EVENT(dpu_enc_irq_template, dpu_enc_irq_unregister_success,
> -	TP_PROTO(uint32_t drm_id, enum dpu_intr_idx intr_idx,
> -		 int irq_idx),
> -	TP_ARGS(drm_id, intr_idx, irq_idx)
> +DEFINE_EVENT(dpu_irq_template, dpu_irq_unregister_success,
> +	TP_PROTO(int irq_idx),
> +	TP_ARGS(irq_idx)
>  );
> 
>  TRACE_EVENT(dpu_enc_irq_wait_success,


More information about the Freedreno mailing list