[Nouveau] [PATCH 1/6] platform: specify the IOMMU physical translation bit

[Alexandre Courbot]

On Thu, Apr 16, 2015 at 8:06 PM, Vince Hsu <vinceh at nvidia.com> wrote:
> The IOMMU physical translation bit might vary with different SoCs. So add
> a variable to specify this bit for GK20A.
>
> Signed-off-by: Vince Hsu <vinceh at nvidia.com>
> ---
>  drm/nouveau/nouveau_platform.c | 19 +++++++++++++++++++
>  drm/nouveau/nouveau_platform.h |  1 +
>  2 files changed, 20 insertions(+)
>
> diff --git a/drm/nouveau/nouveau_platform.c b/drm/nouveau/nouveau_platform.c
> index 775277f1edb0..0d002f73e356 100644
> --- a/drm/nouveau/nouveau_platform.c
> +++ b/drm/nouveau/nouveau_platform.c
> @@ -25,6 +25,7 @@
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/reset.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/iommu.h>
> @@ -92,6 +93,22 @@ static int nouveau_platform_power_down(struct nouveau_platform_gpu *gpu)
>         return 0;
>  }
>
> +static unsigned long nouveau_platform_get_iommu_bit(struct device *dev)
> +{
> +       const struct of_device_id *match;
> +
> +       match = of_match_device(dev->driver->of_match_table, dev);
> +       if (!match) {
> +               dev_warn(dev, "cannot find OF match for device\n");
> +               return 0;
> +       }
> +
> +       if (!strcmp(match->compatible, "nvidia,gk20a"))
> +               return 34;
> +       else
> +               return 0;
> +}

Instead of this function, you should probably use the data field of
struct of_device_id. Define a local struct called, say,
nouveau_platform_params containing (for now) a single iommu_addr_bit
field and instanciate one for each entry of nouveau_platform_match.
Then you can cast match->data and retrieve the field directly instead
of using strcmp.

I'd say this is then simple enough to do directly in
nouveau_platform_probe_iommu() instead of having a function dedicated
to it. It is also safer because when we add a new chip, we have to
update nouveau_platform_match but might very well forget about your
function, and will end up with bit 0 being set on all our GPU
addresses and endless hours of fun figuring out how this happened. :)

While I am at it: how about defining this as a u64 mask to set/unset
on GPU addresses instead of just a bit? This is more flexible and
again safer in case someone "omits" to specify the correct bit for a
chip.