On Thu, 15 Nov 2018 12:39:42 -0800 Eric Anholt <[email protected]> wrote:
> Boris Brezillon <[email protected]> writes: > > > We are about to use vc4_plane_mode_set() in the async check path, and > > async updates require that LBM size stay the same since they reuse the > > LBM from the previous state. So we definitely don't want to allocate a > > new LBM region that we know for sure will be free right away. > > > > Move the LBM allocation out of vc4_plane_mode_set() and call the new > > function (vc4_plane_update_lbm()) from vc4_plane_atomic_check(). > > > > Signed-off-by: Boris Brezillon <[email protected]> > > --- > > drivers/gpu/drm/vc4/vc4_drv.h | 1 + > > drivers/gpu/drm/vc4/vc4_plane.c | 82 ++++++++++++++++++++++----------- > > 2 files changed, 55 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h > > index bd6ef1f31822..9ed05fb61eb6 100644 > > --- a/drivers/gpu/drm/vc4/vc4_drv.h > > +++ b/drivers/gpu/drm/vc4/vc4_drv.h > > @@ -338,6 +338,7 @@ struct vc4_plane_state { > > u32 pos0_offset; > > u32 pos2_offset; > > u32 ptr0_offset; > > + u32 lbm_offset; > > > > /* Offset where the plane's dlist was last stored in the > > * hardware at vc4_crtc_atomic_flush() time. > > diff --git a/drivers/gpu/drm/vc4/vc4_plane.c > > b/drivers/gpu/drm/vc4/vc4_plane.c > > index f6e3e8d33115..392a51f2bf8f 100644 > > --- a/drivers/gpu/drm/vc4/vc4_plane.c > > +++ b/drivers/gpu/drm/vc4/vc4_plane.c > > @@ -452,6 +452,43 @@ static void vc4_write_scaling_parameters(struct > > drm_plane_state *state, > > } > > } > > > > +static int vc4_plane_update_lbm(struct drm_plane_state *state) > > +{ > > + struct vc4_dev *vc4 = to_vc4_dev(state->plane->dev); > > + struct vc4_plane_state *vc4_state = to_vc4_plane_state(state); > > + unsigned long irqflags; > > + u32 lbm_size; > > + > > + lbm_size = vc4_lbm_size(state); > > + if (!lbm_size) > > + return 0; > > + > > + if (WARN_ON(!vc4_state->lbm_offset)) > > + return -EINVAL; > > + > > + /* Allocate the LBM memory that the HVS will use for temporary > > + * storage due to our scaling/format conversion. > > + */ > > + if (!vc4_state->lbm.allocated) { > > + int ret; > > + > > + spin_lock_irqsave(&vc4->hvs->mm_lock, irqflags); > > + ret = drm_mm_insert_node_generic(&vc4->hvs->lbm_mm, > > + &vc4_state->lbm, > > + lbm_size, 32, 0, 0); > > + spin_unlock_irqrestore(&vc4->hvs->mm_lock, irqflags); > > + > > + if (ret) > > + return ret; > > + } else { > > + WARN_ON_ONCE(lbm_size != vc4_state->lbm.size); > > + } > > + > > + vc4_state->dlist[vc4_state->lbm_offset] = vc4_state->lbm.start; > > + > > + return 0; > > +} > > + > > /* Writes out a full display list for an active plane to the plane's > > * private dlist state. > > */ > > @@ -469,31 +506,12 @@ static int vc4_plane_mode_set(struct drm_plane *plane, > > bool mix_plane_alpha; > > bool covers_screen; > > u32 scl0, scl1, pitch0; > > - u32 lbm_size, tiling; > > - unsigned long irqflags; > > + u32 tiling; > > u32 hvs_format = format->hvs; > > int ret, i; > > > > - ret = vc4_plane_setup_clipping_and_scaling(state); > > - if (ret) > > - return ret; > > - > > - /* Allocate the LBM memory that the HVS will use for temporary > > - * storage due to our scaling/format conversion. > > - */ > > - lbm_size = vc4_lbm_size(state); > > - if (lbm_size) { > > - if (!vc4_state->lbm.allocated) { > > - spin_lock_irqsave(&vc4->hvs->mm_lock, irqflags); > > - ret = drm_mm_insert_node_generic(&vc4->hvs->lbm_mm, > > - &vc4_state->lbm, > > - lbm_size, 32, 0, 0); > > - spin_unlock_irqrestore(&vc4->hvs->mm_lock, irqflags); > > - } else { > > - WARN_ON_ONCE(lbm_size != vc4_state->lbm.size); > > - } > > - } > > > > + ret = vc4_plane_setup_clipping_and_scaling(state); > > if (ret) > > return ret; > > > > @@ -717,15 +735,18 @@ static int vc4_plane_mode_set(struct drm_plane *plane, > > vc4_dlist_write(vc4_state, SCALER_CSC2_ITR_R_601_5); > > } > > > > + vc4_state->lbm_offset = 0; > > + > > if (vc4_state->x_scaling[0] != VC4_SCALING_NONE || > > vc4_state->x_scaling[1] != VC4_SCALING_NONE || > > vc4_state->y_scaling[0] != VC4_SCALING_NONE || > > vc4_state->y_scaling[1] != VC4_SCALING_NONE) { > > - /* LBM Base Address. */ > > + /* Reserve a slot for the LBM Base Address. The real value will > > + * be set when calling vc4_plane_update_lbm(). > > + */ > > if (vc4_state->y_scaling[0] != VC4_SCALING_NONE || > > - vc4_state->y_scaling[1] != VC4_SCALING_NONE) { > > - vc4_dlist_write(vc4_state, vc4_state->lbm.start); > > - } > > + vc4_state->y_scaling[1] != VC4_SCALING_NONE) > > + vc4_state->lbm_offset = vc4_state->dlist_count++; > > > > if (num_planes > 1) { > > /* Emit Cb/Cr as channel 0 and Y as channel > > @@ -785,13 +806,18 @@ static int vc4_plane_atomic_check(struct drm_plane > > *plane, > > struct drm_plane_state *state) > > { > > struct vc4_plane_state *vc4_state = to_vc4_plane_state(state); > > + int ret; > > > > vc4_state->dlist_count = 0; > > > > - if (plane_enabled(state)) > > - return vc4_plane_mode_set(plane, state); > > - else > > + if (!plane_enabled(state)) > > return 0; > > + > > + ret = vc4_plane_mode_set(plane, state); > > + if (ret) > > + return ret; > > + > > + return vc4_plane_update_lbm(state); > > } > > Couldn't you call update_lbm() (maybe called allocate_lbm()) first, and > then leave the dlist setup as is without needing lbm_offset? I'll anyway need lbm_offset for the dlist check I do in async_check(). _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
