Hi Lina,
On Tue, May 08, 2018 at 10:14:33AM -0600, ilina@xxxxxxxxxxxxxx wrote:
> On 2018-05-03 16:06, Matthias Kaehlcke wrote:
> Hi Matthias,
>
> > Hi Lina,
> >
> > On Wed, May 02, 2018 at 01:37:45PM -0600, Lina Iyer wrote:
> > > Allow sleep and wake commands to be cleared from the respective TCSes,
> > > so that they can be re-populated.
> > >
> > > Signed-off-by: Lina Iyer <ilina@xxxxxxxxxxxxxx>
> > > ---
> > >
> > > Changes in v7:
> > > - Move bitmap_zero() outside the loop
> > >
> > > Changes in v6:
> > > - remove unnecessary locks around __tcs_invalidate
> > > - rename function to tcs_invaldiate
> > >
> > > Changes in v4:
> > > - refactored the rphm_rsc_invalidate()
> > > ---
> > > drivers/soc/qcom/rpmh-rsc.c | 45
> > > +++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 45 insertions(+)
> > >
> > > diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> > > index 4e2144a14c31..42aedf2d80fe 100644
> > > --- a/drivers/soc/qcom/rpmh-rsc.c
> > > +++ b/drivers/soc/qcom/rpmh-rsc.c
> > > @@ -104,6 +104,51 @@ static struct tcs_group *get_tcs_of_type(struct
> > > rsc_drv *drv, int type)
> > > return &drv->tcs[type];
> > > }
> > >
> > > +static int tcs_invalidate(struct rsc_drv *drv, int type)
> > > +{
> > > + int m;
> >
> > nit: use tcs_id?
> >
> It's an iterator. Hence didn't change this.
>
> > > + struct tcs_group *tcs;
> > > +
> > > + tcs = get_tcs_of_type(drv, type);
> > > + if (IS_ERR(tcs))
> > > + return PTR_ERR(tcs);
> > > +
> > > + spin_lock(&tcs->lock);
> > > + if (bitmap_empty(tcs->slots, MAX_TCS_SLOTS)) {
> > > + spin_unlock(&tcs->lock);
> > > + return 0;
> > > + }
> > > +
> > > + for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
> > > + if (!tcs_is_free(drv, m)) {
> > > + spin_unlock(&tcs->lock);
> > > + return -EAGAIN;
> > > + }
> > > + write_tcs_reg_sync(drv, RSC_DRV_CMD_ENABLE, m, 0);
> > > + }
> > > + bitmap_zero(tcs->slots, MAX_TCS_SLOTS);
> >
> > You didn't reply to (or address) my comment on v6:
> >
> > It could occur that one or more TCSes are disabled, then tcs_is_free()
> > returns false for the next TCS and the function returns without having
> > updated tcs->slots for the TCSes that have been disabled.
> How do you mean TCS are disabled? I think I asked that question in my mail.
> Sorry, if I forgot to ask.
>
> TCSes are either available for sending requests or they are busy sending
> requests. They cannot be disabled if they are present.
> Individual commands, however are enabled or disabled based on whether they
> have active requests or not.
Sorry for conflating terminologies, I don't have a firm grasp on that
yet, what I meant is a TCS being busy sending requests.
> What we are trying to do here is to cleanup the TCSes of their existing
> requests. Generally, sleep and wake TCSes are not used to send active state
> requests, they are sent through AMC/Active TCS. So they will be free.
> However, in the case of the Display RSC, there is no explicit TCS available
> for sending active state requests. So we overload the wake TCS to send the
> active state requests. Even in that case, TCS would have finished and should
> be free when this function is called. The tcs_is_free() check is addition to
> make sure the requirement is not violated.
If you are sure that tcs_is_free(tcs) basically should never return
false when tcs_invalidate() is called I won't insist. If it is a
requirement violation should the code path include a WARN?
--
To unsubscribe from this list: send the line "unsubscribe linux-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
