Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933236AbbFVKep (ORCPT ); Mon, 22 Jun 2015 06:34:45 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:51897 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756008AbbFVKei (ORCPT ); Mon, 22 Jun 2015 06:34:38 -0400 Message-ID: <5587E4B7.9080004@codeaurora.org> Date: Mon, 22 Jun 2015 16:04:31 +0530 From: Archit Taneja User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Wentao Xu , dri-devel@lists.freedesktop.org CC: linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm/msm/mdp5: release SMB(shared memory blocks) in various cases References: <1434737022-23591-1-git-send-email-wentaox@codeaurora.org> In-Reply-To: <1434737022-23591-1-git-send-email-wentaox@codeaurora.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 11028 Lines: 298 On 06/19/2015 11:33 PM, Wentao Xu wrote: > Release all blocks after the pipe is disabled, even when vsync > didn't happen in some error cases. Allow requesting SMB multiple > times before configuring to hardware, by releasing blocks not > programmed to hardware yet for shrinking case. Tested-by: Archit Taneja > > Signed-off-by: Wentao Xu > --- > drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 13 +++++ > drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 2 + > drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 33 +++++------- > drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c | 87 ++++++++++++++++++++++++++----- > drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h | 1 + > 5 files changed, 104 insertions(+), 32 deletions(-) > > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c > index 97226a1..db49ee8 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c > @@ -78,7 +78,20 @@ static void mdp5_prepare_commit(struct msm_kms *kms, struct drm_atomic_state *st > > static void mdp5_complete_commit(struct msm_kms *kms, struct drm_atomic_state *state) > { > + int i; > struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms)); > + int nplanes = mdp5_kms->dev->mode_config.num_total_plane; > + > + for (i = 0; i < nplanes; i++) { > + struct drm_plane *plane = state->planes[i]; > + struct drm_plane_state *plane_state = state->plane_states[i]; > + > + if (!plane) > + continue; > + > + mdp5_plane_complete_commit(plane, plane_state); > + } > + > mdp5_disable(mdp5_kms); > } > > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > index 2c0de17..42f270b 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > @@ -227,6 +227,8 @@ void mdp5_plane_install_properties(struct drm_plane *plane, > struct drm_mode_object *obj); > uint32_t mdp5_plane_get_flush(struct drm_plane *plane); > void mdp5_plane_complete_flip(struct drm_plane *plane); > +void mdp5_plane_complete_commit(struct drm_plane *plane, > + struct drm_plane_state *state); > enum mdp5_pipe mdp5_plane_pipe(struct drm_plane *plane); > struct drm_plane *mdp5_plane_init(struct drm_device *dev, > enum mdp5_pipe pipe, bool private_plane, uint32_t reg_offset); > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > index 18a3d20..05b2634 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > @@ -31,8 +31,6 @@ struct mdp5_plane { > > uint32_t nformats; > uint32_t formats[32]; > - > - bool enabled; > }; > #define to_mdp5_plane(x) container_of(x, struct mdp5_plane, base) > > @@ -56,22 +54,6 @@ static bool plane_enabled(struct drm_plane_state *state) > return state->fb && state->crtc; > } > > -static int mdp5_plane_disable(struct drm_plane *plane) > -{ > - struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane); > - struct mdp5_kms *mdp5_kms = get_kms(plane); > - enum mdp5_pipe pipe = mdp5_plane->pipe; > - > - DBG("%s: disable", mdp5_plane->name); > - > - if (mdp5_kms) { > - /* Release the memory we requested earlier from the SMP: */ > - mdp5_smp_release(mdp5_kms->smp, pipe); > - } > - > - return 0; > -} > - > static void mdp5_plane_destroy(struct drm_plane *plane) > { > struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane); > @@ -224,7 +206,6 @@ static void mdp5_plane_atomic_update(struct drm_plane *plane, > > if (!plane_enabled(state)) { > to_mdp5_plane_state(state)->pending = true; > - mdp5_plane_disable(plane); > } else if (to_mdp5_plane_state(state)->mode_changed) { > int ret; > to_mdp5_plane_state(state)->pending = true; > @@ -602,6 +583,20 @@ uint32_t mdp5_plane_get_flush(struct drm_plane *plane) > return mdp5_plane->flush_mask; > } > > +/* called after vsync in thread context */ > +void mdp5_plane_complete_commit(struct drm_plane *plane, > + struct drm_plane_state *state) > +{ > + struct mdp5_kms *mdp5_kms = get_kms(plane); > + struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane); > + enum mdp5_pipe pipe = mdp5_plane->pipe; > + > + if (!plane_enabled(plane->state)) { > + DBG("%s: free SMP", mdp5_plane->name); > + mdp5_smp_release(mdp5_kms->smp, pipe); > + } > +} > + > /* initialize plane */ > struct drm_plane *mdp5_plane_init(struct drm_device *dev, > enum mdp5_pipe pipe, bool private_plane, uint32_t reg_offset) > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c > index 16702ae..64a27d8 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c > @@ -34,22 +34,44 @@ > * and CANNOT be re-allocated (eg: MMB0 and MMB1 both tied to RGB0). > * > * For each block that can be dynamically allocated, it can be either > - * free, or pending/in-use by a client. The updates happen in three steps: > + * free: > + * The block is free. > + * > + * pending: > + * The block is allocated to some client and not free. > + * > + * configured: > + * The block is allocated to some client, and assigned to that > + * client in MDP5_MDP_SMP_ALLOC registers. > + * > + * inuse: > + * The block is being actively used by a client. > + * > + * The updates happen in the following steps: > * > * 1) mdp5_smp_request(): > * When plane scanout is setup, calculate required number of > - * blocks needed per client, and request. Blocks not inuse or > - * pending by any other client are added to client's pending > - * set. > + * blocks needed per client, and request. Blocks neither inuse nor > + * configured nor pending by any other client are added to client's > + * pending set. > + * For shrinking, blocks in pending but not in configured can be freed > + * directly, but those already in configured will be freed later by > + * mdp5_smp_commit. > * > * 2) mdp5_smp_configure(): > * As hw is programmed, before FLUSH, MDP5_MDP_SMP_ALLOC registers > * are configured for the union(pending, inuse) > + * Current pending is copied to configured. > + * It is assumed that mdp5_smp_request and mdp5_smp_configure not run > + * concurrently for the same pipe. > * > * 3) mdp5_smp_commit(): > - * After next vblank, copy pending -> inuse. Optionally update > + * After next vblank, copy configured -> inuse. Optionally update > * MDP5_SMP_ALLOC registers if there are newly unused blocks > * > + * 4) mdp5_smp_release(): > + * Must be called after the pipe is disabled and no longer uses any SMB > + * > * On the next vblank after changes have been committed to hw, the > * client's pending blocks become it's in-use blocks (and no-longer > * in-use blocks become available to other clients). > @@ -77,6 +99,9 @@ struct mdp5_smp { > struct mdp5_client_smp_state client_state[MAX_CLIENTS]; > }; > > +static void update_smp_state(struct mdp5_smp *smp, > + u32 cid, mdp5_smp_state_t *assigned); > + > static inline > struct mdp5_kms *get_kms(struct mdp5_smp *smp) > { > @@ -149,7 +174,12 @@ static int smp_request_block(struct mdp5_smp *smp, > for (i = cur_nblks; i > nblks; i--) { > int blk = find_first_bit(ps->pending, cnt); > clear_bit(blk, ps->pending); > - /* don't clear in global smp_state until _commit() */ > + > + /* clear in global smp_state if not in configured > + * otherwise until _commit() > + */ > + if (!test_bit(blk, ps->configured)) > + clear_bit(blk, smp->state); > } > } > > @@ -223,10 +253,33 @@ int mdp5_smp_request(struct mdp5_smp *smp, enum mdp5_pipe pipe, u32 fmt, u32 wid > /* Release SMP blocks for all clients of the pipe */ > void mdp5_smp_release(struct mdp5_smp *smp, enum mdp5_pipe pipe) > { > - int i, nblks; > + int i; > + unsigned long flags; > + int cnt = smp->blk_cnt; > + > + for (i = 0; i < pipe2nclients(pipe); i++) { > + mdp5_smp_state_t assigned; > + u32 cid = pipe2client(pipe, i); > + struct mdp5_client_smp_state *ps = &smp->client_state[cid]; > + > + spin_lock_irqsave(&smp->state_lock, flags); > + > + /* clear hw assignment */ > + bitmap_or(assigned, ps->inuse, ps->configured, cnt); > + update_smp_state(smp, CID_UNUSED, &assigned); > + > + /* free to global pool */ > + bitmap_andnot(smp->state, smp->state, ps->pending, cnt); > + bitmap_andnot(smp->state, smp->state, assigned, cnt); > + > + /* clear client's infor */ > + bitmap_zero(ps->pending, cnt); > + bitmap_zero(ps->configured, cnt); > + bitmap_zero(ps->inuse, cnt); > + > + spin_unlock_irqrestore(&smp->state_lock, flags); > + } > > - for (i = 0, nblks = 0; i < pipe2nclients(pipe); i++) > - smp_request_block(smp, pipe2client(pipe, i), 0); > set_fifo_thresholds(smp, pipe, 0); > } > > @@ -274,12 +327,20 @@ void mdp5_smp_configure(struct mdp5_smp *smp, enum mdp5_pipe pipe) > u32 cid = pipe2client(pipe, i); > struct mdp5_client_smp_state *ps = &smp->client_state[cid]; > > - bitmap_or(assigned, ps->inuse, ps->pending, cnt); > + /* > + * if vblank has not happened since last smp_configure > + * skip the configure for now > + */ > + if (!bitmap_equal(ps->inuse, ps->configured, cnt)) > + continue; > + > + bitmap_copy(ps->configured, ps->pending, cnt); > + bitmap_or(assigned, ps->inuse, ps->configured, cnt); > update_smp_state(smp, cid, &assigned); > } > } > > -/* step #3: after vblank, copy pending -> inuse: */ > +/* step #3: after vblank, copy configured -> inuse: */ > void mdp5_smp_commit(struct mdp5_smp *smp, enum mdp5_pipe pipe) > { > int cnt = smp->blk_cnt; > @@ -295,7 +356,7 @@ void mdp5_smp_commit(struct mdp5_smp *smp, enum mdp5_pipe pipe) > * using, which can be released and made available to other > * clients: > */ > - if (bitmap_andnot(released, ps->inuse, ps->pending, cnt)) { > + if (bitmap_andnot(released, ps->inuse, ps->configured, cnt)) { > unsigned long flags; > > spin_lock_irqsave(&smp->state_lock, flags); > @@ -306,7 +367,7 @@ void mdp5_smp_commit(struct mdp5_smp *smp, enum mdp5_pipe pipe) > update_smp_state(smp, CID_UNUSED, &released); > } > > - bitmap_copy(ps->inuse, ps->pending, cnt); > + bitmap_copy(ps->inuse, ps->configured, cnt); > } > } > > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h > index e47179f..5b6c236 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h > @@ -23,6 +23,7 @@ > > struct mdp5_client_smp_state { > mdp5_smp_state_t inuse; > + mdp5_smp_state_t configured; > mdp5_smp_state_t pending; > }; > > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/