Re: [PATCH] drm_handle_t type

[Robert Noland <rnoland@2hip.net>]

Dave Airlie wrote:
> On Wed, Apr 14, 2010 at 1:32 PM, Robert Noland <rnoland@2hip.net> wrote:
>> On Wed, 2010-04-14 at 00:19 +0100, Matthew W. S. Bell wrote:
>>> On Sun, 2010-04-11 at 09:10 -0500, Robert Noland wrote:
>>>> On Sat, 2010-04-10 at 19:30 +0100, Matthew W. S. Bell wrote:
>>>>> On Mon, 2010-04-05 at 17:46 +1000, Dave Airlie wrote:
>>>>>> Its probably not documented well anywhere, though I think the handles are
>>>>>> 32-bit is written down somewhere.
>>>>> Ah sorry, I missed some.
>>>> drm_handle_t is correct here...
>>> No, drm_handle_t can be of a different size to void *; converting
>>> between integers and pointers of different sizes causes a warning. To
>>> eliminate the warning, the value first needs to be passed between
>>> uintptr_t and void *, which are of the same size, and then converted to
>>> drm_handle_t. The last part is implicit; the drm_handle_t casts are
>>> irrelevant/useless.
>> My point being that the casts to drm_handle_t are correct.  Feel free to
>> fix linux' define for drm_handle_t.
>>
> 
> Weird the drm is defined on Linux and is considered the standard, and
> uint32_t is considered correct handle sizing on Linux.
> 
> I'm not sure why FreeBSD didn't change to 32-bit handles at the same
> time, I think nobody wanted to break stuff by accident at the time,
> assuming FreeBSD maintainers would take care of it.

drm_handle_t is passed directly to mmap() and on FreeBSD that is 
expected to be an offset into the device memory.  If drm_handle_t were 
only a handle, this would all be fine, but due to the fact that it's 
meaning is overloaded and is supposed to represent the KVA as well, it 
either needs to vary with the architecture or be 64bits...  Last time I 
looked at just making it 64bits everywhere, some minor fixups and API 
breakage would have been required in the DDX drivers, which was too much 
trouble to argue about...

The define on FreeBSD is an unsigned long, which works fine on 32/64 bit 
platforms with current DDX drivers and doesn't produce warnings.

robert.

> Dave.