Quoting Jordan Crouse (2018-09-17 14:14:14)
> On Mon, Sep 17, 2018 at 01:40:07PM -0700, Stephen Boyd wrote:
> > + vals = cmd_db_read_aux_data(id, &len);
> > /*
> > * The data comes back as an array of unsigned shorts so adjust the
> > * count accordingly
> > */
> > - return len >> 1;
> > + len >>= 1;
> > +
> > + if (!len || WARN_ON(len > 16))
> > + len = 0;
>
> I don't think we care any more about the size of the array since there isn't any
> data being copied.
>
Ok, sounds good.
> > +
> > + *size = len;
> > + return vals;
> > }
>
> I'm not sure this function has any value any more. We could just call
> cmd_db_read_aux_data() directly for each id and adjust the size on the fly
> when we call a6xx_gmu_rpmh_arc_votes_init.
Ok. Let me attempt to do more code reduction!
>
> > /* Return the 'arc-level' for the given frequency */
> > @@ -907,8 +907,8 @@ static u32 a6xx_gmu_get_arc_level(struct device *dev, unsigned long freq)
> >
> > static int a6xx_gmu_rpmh_arc_votes_init(struct device *dev, u32 *votes,
> > unsigned long *freqs, int freqs_count,
> > - u16 *pri, int pri_count,
> > - u16 *sec, int sec_count)
> > + const u16 *pri, int pri_count,
> > + const u16 *sec, int sec_count)
> > {
> > int i, j;
> >
> > @@ -970,14 +970,14 @@ static int a6xx_gmu_rpmh_votes_init(struct a6xx_gmu *gmu)
> > struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
> > struct msm_gpu *gpu = &adreno_gpu->base;
> >
> > - u16 gx[16], cx[16], mx[16];
> > + const u16 *gx, *cx, *mx;
> > u32 gxcount, cxcount, mxcount;
> > int ret;
> >
> > /* Get the list of available voltage levels for each component */
> > - gxcount = a6xx_gmu_rpmh_arc_cmds("gfx.lvl", gx, sizeof(gx));
> > - cxcount = a6xx_gmu_rpmh_arc_cmds("cx.lvl", cx, sizeof(cx));
> > - mxcount = a6xx_gmu_rpmh_arc_cmds("mx.lvl", mx, sizeof(mx));
> > + gx = a6xx_gmu_rpmh_arc_cmds("gfx.lvl", &gxcount);
> > + cx = a6xx_gmu_rpmh_arc_cmds("cx.lvl", &cxcount);
> > + mx = a6xx_gmu_rpmh_arc_cmds("mx.lvl", &mxcount);
> >
> > /* Build the GX votes */
> > ret = a6xx_gmu_rpmh_arc_votes_init(&gpu->pdev->dev, gmu->gx_arc_votes,
> > diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
> > index 5c9cc6824891..c701b3b010f1 100644
> > --- a/drivers/soc/qcom/cmd-db.c
> > +++ b/drivers/soc/qcom/cmd-db.c
> > @@ -192,55 +192,28 @@ EXPORT_SYMBOL(cmd_db_read_addr);
> > /**
> > * cmd_db_read_aux_data() - Query command db for aux data.
> > *
> > - * @id: Resource to retrieve AUX Data on.
> > - * @data: Data buffer to copy returned aux data to. Returns size on NULL
> > - * @len: Caller provides size of data buffer passed in.
> > + * @id: Resource to retrieve AUX Data on
> > + * @len: size of data buffer returned
>
> Is the GPU the only consumer of this function? Would it make more sense to
> return the number of entries rather than the size? I would suppose that would
> imply that the caller knows the size of the entries but we have to know that
> anyway if we're going to walk the list.
I think the bus scaling driver also calls this function. I don't see
much advantage to doing that here when we can just consider the aux_data
as a big blob of data that is interpreted however the consumer cares.
