Re: [PATCH 08/25] drm/msm/dpu: release reservation using crtc state

Sean Paul
2018-10-10T14:50:27Z

On Mon, Oct 08, 2018 at 09:27:25PM -0700, Jeykumar Sankaran wrote:
> Use the hw block pointers stored in crtc state to
> release them back to RM resource pool. This change
> is made to uncouple RM reservation from encoder_id.
> Separate change is submitted to clean up RM of
> encoder id tagging.
>
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 69 +++++++++++++++++++++++------
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 6 +--
> 3 files changed, 60 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 17dbbc3..a8fd14e 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1223,7 +1223,7 @@ static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc)
>
> DPU_DEBUG_ENC(dpu_enc, "encoder disabled\n");
>
> - dpu_rm_release(&dpu_kms->rm, drm_enc);
> + dpu_rm_release(&dpu_kms->rm, drm_enc->crtc->state);

From drm_encoder.h:

* @crtc: Currently bound CRTC, only really meaningful for non-atomic
* drivers. Atomic drivers should instead check
* &drm_connector_state.crtc.

> }
>
> static enum dpu_intf dpu_encoder_get_intf(struct dpu_mdss_cfg *catalog,
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index 5703b11..619b596 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -625,27 +625,70 @@ static int _dpu_rm_populate_requirements(
> return 0;
> }
>
> -static void _dpu_rm_release_reservation(struct dpu_rm *rm, uint32_t enc_id)
> +static int _dpu_rm_release_hw(struct dpu_rm *rm, enum dpu_hw_blk_type type,
> + int id)
> {
> struct dpu_rm_hw_blk *blk;
> - enum dpu_hw_blk_type type;
>
> - for (type = 0; type < DPU_HW_BLK_MAX; type++) {
> - list_for_each_entry(blk, &rm->hw_blks[type], list) {
> - if (blk->enc_id == enc_id) {
> - blk->enc_id = 0;
> - DPU_DEBUG("rel enc %d %d %d\n", enc_id,
> - blk->type, blk->id);
> - }
> + list_for_each_entry(blk, &rm->hw_blks[type], list) {
> + if (blk->hw->id == id) {
> + blk->enc_id = 0;
> + return 0;
> }
> }
> +
> + DRM_DEBUG_KMS("failed to find hw id(%d) of type(%d) for releasing\n",
> + id, type);
> +
> + return -EINVAL;
> }
>
> -void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc)
> +static void _dpu_rm_release_reservation(struct dpu_rm *rm,
> + struct dpu_crtc_state *dpu_cstate)
> {
> + int i;
> +
> + for (i = 0; i < dpu_cstate->num_mixers; i++) {
> + struct dpu_crtc_mixer *mixer = &dpu_cstate->mixers[i];
> +
> + if (!mixer->hw_lm)
> + continue;
> +
> + if (!_dpu_rm_release_hw(rm, DPU_HW_BLK_LM,
> + mixer->hw_lm->base.id))
> + mixer->hw_lm = NULL;
> +
> + if (!_dpu_rm_release_hw(rm, DPU_HW_BLK_PINGPONG,
> + mixer->hw_pp->base.id))
> + mixer->hw_pp = NULL;
> + }
> +
> + for (i = 0; i < dpu_cstate->num_ctls; i++) {
> + if (!dpu_cstate->hw_ctls[i])
> + continue;
> +
> + if (!_dpu_rm_release_hw(rm, DPU_HW_BLK_CTL,
> + dpu_cstate->hw_ctls[i]->base.id))
> + dpu_cstate->hw_ctls[i] = NULL;
> + }
> +
> + for (i = 0; i < dpu_cstate->num_intfs; i++) {
> + if (!dpu_cstate->hw_intfs[i])
> + continue;
> +
> + if (!_dpu_rm_release_hw(rm, DPU_HW_BLK_INTF,
> + dpu_cstate->hw_intfs[i]->base.id))
> + dpu_cstate->hw_intfs[i] = NULL;
> + }
> +}
> +
> +void dpu_rm_release(struct dpu_rm *rm, struct drm_crtc_state *crtc_state)
> +{
> + struct dpu_crtc_state *dpu_cstate = to_dpu_crtc_state(crtc_state);
> +
> mutex_lock(&rm->rm_lock);
>
> - _dpu_rm_release_reservation(rm, enc->base.id);
> + _dpu_rm_release_reservation(rm, dpu_cstate);
>
> mutex_unlock(&rm->rm_lock);
> }
> @@ -679,12 +722,12 @@ int dpu_rm_reserve(
> ret = _dpu_rm_make_reservation(rm, enc, dpu_cstate, &reqs);
> if (ret) {
> DPU_ERROR("failed to reserve hw resources: %d\n", ret);
> - _dpu_rm_release_reservation(rm, enc->base.id);
> + _dpu_rm_release_reservation(rm, dpu_cstate);
> } else if (test_only) {
> /* test_only: test the reservation and then undo */
> DPU_DEBUG("test_only: discard test [enc: %d]\n",
> enc->base.id);
> - _dpu_rm_release_reservation(rm, enc->base.id);
> + _dpu_rm_release_reservation(rm, dpu_cstate);
> }
>
> end:
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> index eb6a6ac..e48e8f2 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> @@ -95,13 +95,13 @@ int dpu_rm_reserve(struct dpu_rm *rm,
> bool test_only);
>
> /**
> - * dpu_rm_reserve - Given the encoder for the display chain, release any
> + * dpu_rm_release - Given the encoder for the display chain, release any
> * HW blocks previously reserved for that use case.
> * @rm: DPU Resource Manager handle
> - * @enc: DRM Encoder handle
> + * @crtc_state: atomic DRM state handle
> * @Return: 0 on Success otherwise -ERROR
> */
> -void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc);
> +void dpu_rm_release(struct dpu_rm *rm, struct drm_crtc_state *crtc_state);
>
> /**
> * dpu_rm_get_mdp - Retrieve HW block for MDP TOP.
> --
> 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