On 04/16/2015 11:26 PM, Alexandre Courbot wrote:
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.
Wouldn't u64 be as dangerous? We'd be just messing up with address in another way.

Another way of expressing this information would be defining number of physical(*) address bits. IOMMU bit is the bit after physical address bits.

Terje

* Physical here means after GPU MMU translation, i.e. either physical or IOMMU address. Somebody needs to come up with a term to cover that.
_______________________________________________
Nouveau mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/nouveau

Reply via email to