[DPU PATCH] drm/msm: Remove dpu_plane_state->defer_prepare_fb

[DPU PATCH] drm/msm: Remove dpu_plane_state->defer_prepare_fb

[Sean Paul]

It's never set to true, so none of this code is ever run.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 65 -----------------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h |  2 -
 2 files changed, 67 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 9b684e94e8aa..cbda7dbf8725 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -894,22 +894,6 @@ static inline void _dpu_plane_set_scanout(struct drm_plane *plane,
 		return;
 	}
 
-	/*
-	 * framebuffer prepare is deferred for prepare_fb calls that
-	 * happen during the transition from secure to non-secure.
-	 * Handle the prepare at this point for such cases. This can be
-	 * expected for one or two frames during the transition.
-	 */
-	if (aspace && pstate->defer_prepare_fb) {
-		ret = msm_framebuffer_prepare(fb, pstate->aspace);
-		if (ret) {
-			DPU_ERROR_PLANE(pdpu,
-				"failed to prepare framebuffer %d\n", ret);
-			return;
-		}
-		pstate->defer_prepare_fb = false;
-	}
-
 	ret = dpu_format_populate_layout(aspace, fb, &pipe_cfg->layout);
 	if (ret == -EAGAIN)
 		DPU_DEBUG_PLANE(pdpu, "not updating same src addrs\n");
@@ -2103,13 +2087,6 @@ static int dpu_plane_rot_prepare_fb(struct drm_plane *plane,
 		}
 	}
 
-	if (new_pstate->defer_prepare_fb) {
-		DPU_DEBUG(
-		    "plane%d, domain not attached, prepare fb handled later\n",
-		    plane->base.id);
-		return 0;
-	}
-
 	/* prepare rotator input buffer */
 	ret = msm_framebuffer_prepare(new_state->fb, new_pstate->aspace);
 	if (ret) {
@@ -2401,34 +2378,6 @@ static void dpu_plane_rot_atomic_update(struct drm_plane *plane,
 	if (!rstate->out_sbuf || !rstate->rot_hw)
 		return;
 
-	/*
-	 * framebuffer prepare is deferred for prepare_fb calls that
-	 * happen during the transition from secure to non-secure.
-	 * Handle the prepare at this point for rotator in such cases.
-	 * This can be expected for one or two frames during the transition.
-	 */
-	if (pstate->aspace && pstate->defer_prepare_fb) {
-		/* prepare rotator input buffer */
-		ret = msm_framebuffer_prepare(state->fb, pstate->aspace);
-		if (ret) {
-			DPU_ERROR("p%d failed to prepare input fb %d\n",
-							plane->base.id, ret);
-			return;
-		}
-
-		/* prepare rotator output buffer */
-		if (dpu_plane_enabled(state) && rstate->out_fb) {
-			ret = msm_framebuffer_prepare(rstate->out_fb,
-						pstate->aspace);
-			if (ret) {
-				DPU_ERROR(
-				  "p%d failed to prepare inline fb %d\n",
-				  plane->base.id, ret);
-				goto error_prepare_output_buffer;
-			}
-		}
-	}
-
 	dpu_plane_rot_submit_command(plane, state, DPU_HW_ROT_CMD_COMMIT);
 
 	return;
@@ -2796,26 +2745,12 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
 	/* cache aspace */
 	pstate->aspace = aspace;
 
-	/*
-	 * when transitioning from secure to non-secure,
-	 * plane->prepare_fb happens before the commit. In such case,
-	 * defer the prepare_fb and handled it late, during the commit
-	 * after attaching the domains as part of the transition
-	 */
-	pstate->defer_prepare_fb = false;
-
 	ret = dpu_plane_rot_prepare_fb(plane, new_state);
 	if (ret) {
 		DPU_ERROR("failed to prepare rot framebuffer\n");
 		return ret;
 	}
 
-	if (pstate->defer_prepare_fb) {
-		DPU_DEBUG_PLANE(pdpu,
-		    "domain not attached, prepare_fb handled later\n");
-		return 0;
-	}
-
 	new_rstate = &to_dpu_plane_state(new_state)->rot;
 
 	if (pstate->aspace) {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index 8c49412d460d..717dfe05c6ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -129,7 +129,6 @@ enum dpu_plane_sclcheck_state {
  * @multirect_index: index of the rectangle of SSPP
  * @multirect_mode: parallel or time multiplex multirect mode
  * @pending:	whether the current update is still pending
- * @defer_prepare_fb:	indicate if prepare_fb call was deferred
  * @scaler3_cfg: configuration data for scaler3
  * @pixel_ext: configuration data for pixel extensions
  * @scaler_check_state: indicates status of user provided pixel extension data
@@ -147,7 +146,6 @@ struct dpu_plane_state {
 	uint32_t multirect_index;
 	uint32_t multirect_mode;
 	bool pending;
-	bool defer_prepare_fb;
 
 	/* scaler configuration */
 	struct dpu_hw_scaler3_cfg scaler3_cfg;
-- 
2.16.1.291.g4437f3f132-goog

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [DPU PATCH] drm/msm: Remove dpu_plane_state->defer_prepare_fb

[Rob Clark]

On Wed, Feb 21, 2018 at 2:43 PM, Sean Paul <seanpaul@chromium.org> wrote:
> It's never set to true, so none of this code is ever run.
>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

I guess if it was ever set to true, that got ripped out with the
secure buffer support.  And when that ever gets re-introduced, I hope
we can avoid the need for transitions, since it does seem to introduce
a lot more complexity.

Reviewed-by: Rob Clark <robdclark@gmail.com>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 65 -----------------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h |  2 -
>  2 files changed, 67 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 9b684e94e8aa..cbda7dbf8725 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -894,22 +894,6 @@ static inline void _dpu_plane_set_scanout(struct drm_plane *plane,
>                 return;
>         }
>
> -       /*
> -        * framebuffer prepare is deferred for prepare_fb calls that
> -        * happen during the transition from secure to non-secure.
> -        * Handle the prepare at this point for such cases. This can be
> -        * expected for one or two frames during the transition.
> -        */
> -       if (aspace && pstate->defer_prepare_fb) {
> -               ret = msm_framebuffer_prepare(fb, pstate->aspace);
> -               if (ret) {
> -                       DPU_ERROR_PLANE(pdpu,
> -                               "failed to prepare framebuffer %d\n", ret);
> -                       return;
> -               }
> -               pstate->defer_prepare_fb = false;
> -       }
> -
>         ret = dpu_format_populate_layout(aspace, fb, &pipe_cfg->layout);
>         if (ret == -EAGAIN)
>                 DPU_DEBUG_PLANE(pdpu, "not updating same src addrs\n");
> @@ -2103,13 +2087,6 @@ static int dpu_plane_rot_prepare_fb(struct drm_plane *plane,
>                 }
>         }
>
> -       if (new_pstate->defer_prepare_fb) {
> -               DPU_DEBUG(
> -                   "plane%d, domain not attached, prepare fb handled later\n",
> -                   plane->base.id);
> -               return 0;
> -       }
> -
>         /* prepare rotator input buffer */
>         ret = msm_framebuffer_prepare(new_state->fb, new_pstate->aspace);
>         if (ret) {
> @@ -2401,34 +2378,6 @@ static void dpu_plane_rot_atomic_update(struct drm_plane *plane,
>         if (!rstate->out_sbuf || !rstate->rot_hw)
>                 return;
>
> -       /*
> -        * framebuffer prepare is deferred for prepare_fb calls that
> -        * happen during the transition from secure to non-secure.
> -        * Handle the prepare at this point for rotator in such cases.
> -        * This can be expected for one or two frames during the transition.
> -        */
> -       if (pstate->aspace && pstate->defer_prepare_fb) {
> -               /* prepare rotator input buffer */
> -               ret = msm_framebuffer_prepare(state->fb, pstate->aspace);
> -               if (ret) {
> -                       DPU_ERROR("p%d failed to prepare input fb %d\n",
> -                                                       plane->base.id, ret);
> -                       return;
> -               }
> -
> -               /* prepare rotator output buffer */
> -               if (dpu_plane_enabled(state) && rstate->out_fb) {
> -                       ret = msm_framebuffer_prepare(rstate->out_fb,
> -                                               pstate->aspace);
> -                       if (ret) {
> -                               DPU_ERROR(
> -                                 "p%d failed to prepare inline fb %d\n",
> -                                 plane->base.id, ret);
> -                               goto error_prepare_output_buffer;
> -                       }
> -               }
> -       }
> -
>         dpu_plane_rot_submit_command(plane, state, DPU_HW_ROT_CMD_COMMIT);
>
>         return;
> @@ -2796,26 +2745,12 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
>         /* cache aspace */
>         pstate->aspace = aspace;
>
> -       /*
> -        * when transitioning from secure to non-secure,
> -        * plane->prepare_fb happens before the commit. In such case,
> -        * defer the prepare_fb and handled it late, during the commit
> -        * after attaching the domains as part of the transition
> -        */
> -       pstate->defer_prepare_fb = false;
> -
>         ret = dpu_plane_rot_prepare_fb(plane, new_state);
>         if (ret) {
>                 DPU_ERROR("failed to prepare rot framebuffer\n");
>                 return ret;
>         }
>
> -       if (pstate->defer_prepare_fb) {
> -               DPU_DEBUG_PLANE(pdpu,
> -                   "domain not attached, prepare_fb handled later\n");
> -               return 0;
> -       }
> -
>         new_rstate = &to_dpu_plane_state(new_state)->rot;
>
>         if (pstate->aspace) {
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> index 8c49412d460d..717dfe05c6ee 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> @@ -129,7 +129,6 @@ enum dpu_plane_sclcheck_state {
>   * @multirect_index: index of the rectangle of SSPP
>   * @multirect_mode: parallel or time multiplex multirect mode
>   * @pending:   whether the current update is still pending
> - * @defer_prepare_fb:  indicate if prepare_fb call was deferred
>   * @scaler3_cfg: configuration data for scaler3
>   * @pixel_ext: configuration data for pixel extensions
>   * @scaler_check_state: indicates status of user provided pixel extension data
> @@ -147,7 +146,6 @@ struct dpu_plane_state {
>         uint32_t multirect_index;
>         uint32_t multirect_mode;
>         bool pending;
> -       bool defer_prepare_fb;
>
>         /* scaler configuration */
>         struct dpu_hw_scaler3_cfg scaler3_cfg;
> --
> 2.16.1.291.g4437f3f132-goog
>
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [DPU PATCH] drm/msm: Remove dpu_plane_state->defer_prepare_fb

[Sean Paul]

On Thu, Feb 22, 2018 at 03:02:44PM -0500, Rob Clark wrote:
> On Wed, Feb 21, 2018 at 2:43 PM, Sean Paul <seanpaul@chromium.org> wrote:
> > It's never set to true, so none of this code is ever run.
> >
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> 
> I guess if it was ever set to true, that got ripped out with the
> secure buffer support.  And when that ever gets re-introduced, I hope
> we can avoid the need for transitions, since it does seem to introduce
> a lot more complexity.

Yeah, I think we'd want to re-evaluate anyways since it seemed like a lot of
copypasta sprinkled around. At the _very least_ we'd want to add a helper to
handle the prepare and state tracking.

> 
> Reviewed-by: Rob Clark <robdclark@gmail.com>

Thanks!

Sean

> 
> > ---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 65 -----------------------
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h |  2 -
> >  2 files changed, 67 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > index 9b684e94e8aa..cbda7dbf8725 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > @@ -894,22 +894,6 @@ static inline void _dpu_plane_set_scanout(struct drm_plane *plane,
> >                 return;
> >         }
> >
> > -       /*
> > -        * framebuffer prepare is deferred for prepare_fb calls that
> > -        * happen during the transition from secure to non-secure.
> > -        * Handle the prepare at this point for such cases. This can be
> > -        * expected for one or two frames during the transition.
> > -        */
> > -       if (aspace && pstate->defer_prepare_fb) {
> > -               ret = msm_framebuffer_prepare(fb, pstate->aspace);
> > -               if (ret) {
> > -                       DPU_ERROR_PLANE(pdpu,
> > -                               "failed to prepare framebuffer %d\n", ret);
> > -                       return;
> > -               }
> > -               pstate->defer_prepare_fb = false;
> > -       }
> > -
> >         ret = dpu_format_populate_layout(aspace, fb, &pipe_cfg->layout);
> >         if (ret == -EAGAIN)
> >                 DPU_DEBUG_PLANE(pdpu, "not updating same src addrs\n");
> > @@ -2103,13 +2087,6 @@ static int dpu_plane_rot_prepare_fb(struct drm_plane *plane,
> >                 }
> >         }
> >
> > -       if (new_pstate->defer_prepare_fb) {
> > -               DPU_DEBUG(
> > -                   "plane%d, domain not attached, prepare fb handled later\n",
> > -                   plane->base.id);
> > -               return 0;
> > -       }
> > -
> >         /* prepare rotator input buffer */
> >         ret = msm_framebuffer_prepare(new_state->fb, new_pstate->aspace);
> >         if (ret) {
> > @@ -2401,34 +2378,6 @@ static void dpu_plane_rot_atomic_update(struct drm_plane *plane,
> >         if (!rstate->out_sbuf || !rstate->rot_hw)
> >                 return;
> >
> > -       /*
> > -        * framebuffer prepare is deferred for prepare_fb calls that
> > -        * happen during the transition from secure to non-secure.
> > -        * Handle the prepare at this point for rotator in such cases.
> > -        * This can be expected for one or two frames during the transition.
> > -        */
> > -       if (pstate->aspace && pstate->defer_prepare_fb) {
> > -               /* prepare rotator input buffer */
> > -               ret = msm_framebuffer_prepare(state->fb, pstate->aspace);
> > -               if (ret) {
> > -                       DPU_ERROR("p%d failed to prepare input fb %d\n",
> > -                                                       plane->base.id, ret);
> > -                       return;
> > -               }
> > -
> > -               /* prepare rotator output buffer */
> > -               if (dpu_plane_enabled(state) && rstate->out_fb) {
> > -                       ret = msm_framebuffer_prepare(rstate->out_fb,
> > -                                               pstate->aspace);
> > -                       if (ret) {
> > -                               DPU_ERROR(
> > -                                 "p%d failed to prepare inline fb %d\n",
> > -                                 plane->base.id, ret);
> > -                               goto error_prepare_output_buffer;
> > -                       }
> > -               }
> > -       }
> > -
> >         dpu_plane_rot_submit_command(plane, state, DPU_HW_ROT_CMD_COMMIT);
> >
> >         return;
> > @@ -2796,26 +2745,12 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
> >         /* cache aspace */
> >         pstate->aspace = aspace;
> >
> > -       /*
> > -        * when transitioning from secure to non-secure,
> > -        * plane->prepare_fb happens before the commit. In such case,
> > -        * defer the prepare_fb and handled it late, during the commit
> > -        * after attaching the domains as part of the transition
> > -        */
> > -       pstate->defer_prepare_fb = false;
> > -
> >         ret = dpu_plane_rot_prepare_fb(plane, new_state);
> >         if (ret) {
> >                 DPU_ERROR("failed to prepare rot framebuffer\n");
> >                 return ret;
> >         }
> >
> > -       if (pstate->defer_prepare_fb) {
> > -               DPU_DEBUG_PLANE(pdpu,
> > -                   "domain not attached, prepare_fb handled later\n");
> > -               return 0;
> > -       }
> > -
> >         new_rstate = &to_dpu_plane_state(new_state)->rot;
> >
> >         if (pstate->aspace) {
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> > index 8c49412d460d..717dfe05c6ee 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> > @@ -129,7 +129,6 @@ enum dpu_plane_sclcheck_state {
> >   * @multirect_index: index of the rectangle of SSPP
> >   * @multirect_mode: parallel or time multiplex multirect mode
> >   * @pending:   whether the current update is still pending
> > - * @defer_prepare_fb:  indicate if prepare_fb call was deferred
> >   * @scaler3_cfg: configuration data for scaler3
> >   * @pixel_ext: configuration data for pixel extensions
> >   * @scaler_check_state: indicates status of user provided pixel extension data
> > @@ -147,7 +146,6 @@ struct dpu_plane_state {
> >         uint32_t multirect_index;
> >         uint32_t multirect_mode;
> >         bool pending;
> > -       bool defer_prepare_fb;
> >
> >         /* scaler configuration */
> >         struct dpu_hw_scaler3_cfg scaler3_cfg;
> > --
> > 2.16.1.291.g4437f3f132-goog
> >

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