On Mon, Mar 24, 2014 at 05:42:26PM +0900, Alexandre Courbot wrote:
[...]
> diff --git a/drivers/gpu/drm/nouveau/core/subdev/bar/nvc0.c b/drivers/gpu/drm/nouveau/core/subdev/bar/nvc0.c
[...]
> static int
> -nvc0_bar_ctor(struct nouveau_object *parent, struct nouveau_object *engine,
> - struct nouveau_oclass *oclass, void *data, u32 size,
> - struct nouveau_object **pobject)
> +nvc0_bar_init_vm(struct nvc0_bar_priv *priv, int nr, int bar)
> {
[...]
> - /* BAR3 */
> ret = nouveau_gpuobj_new(nv_object(priv), NULL, 0x1000, 0, 0,
> - &priv->bar[0].mem);
> - mem = priv->bar[0].mem;
> + &priv->bar[nr].mem);
> + mem = priv->bar[nr].mem;
> if (ret)
> return ret;
>
> ret = nouveau_gpuobj_new(nv_object(priv), NULL, 0x8000, 0, 0,
> - &priv->bar[0].pgd);
> + &priv->bar[nr].pgd);
> if (ret)
> return ret;
[...]
> +static int
> +nvc0_bar_ctor(struct nouveau_object *parent, struct nouveau_object *engine,
> + struct nouveau_oclass *oclass, void *data, u32 size,
> + struct nouveau_object **pobject)
> +{
[...]
> + /* BAR3 */
> + if (has_bar3) {
> + ret = nvc0_bar_init_vm(priv, 0, 3);
[...]
> + /* BAR1 */
> + ret = nvc0_bar_init_vm(priv, 1, 1);
> if (ret)
> return ret;
The calls to nvc0_bar_init_vm() are somewhat confusing in my opinion. It
is hard to see from the invocation what these numbers mean and therefore
distinguish which parameter is which.
Perhaps a slightly more readable way would be to pass in a pointer to a
structure as second parameter instead of the index into an array. So
it'd look somewhat like this:
if (has_bar3) {
ret = nvc0_bar_init_vm(priv, &priv->bar[0], 3);
...
}
...
ret = nvc0_bar_init_vm(priv, &priv->bar[1], 1);
...
Unfortunately that would require a new type to be created for the bar[]
structures, so it'd be slightly more intrusive.
Thierry
Attachment:
pgpWiCMTFPWQq.pgp
Description: PGP signature
