Re: [PATCH v2] drm: Compare only lower 32 bits of framebuffer map offsets

[Tormod Volden <lists.tormod@gmail.com>]

On Mon, May 30, 2011 at 9:45 PM, Tormod Volden wrote:
> From: Tormod Volden <debian.tormod@gmail.com>
>
> Drivers using multiple framebuffers got broken by commit
> 41c2e75e60200a860a74b7c84a6375c105e7437f which ignored the framebuffer
> (or register) map offset when looking for existing maps. The rationale
> was that the kernel-userspace ABI is fixed at a 32-bit offset, so the
> real offsets could not always be handed over for comparison.
>
> Instead of ignoring the offset we will compare the lower 32 bit. Drivers
> using multiple framebuffers should just make sure that the lower 32 bit
> are different. The existing drivers in question are practically limited
> to 32-bit systems so that should be fine for them.
>
> It is assumed that current drivers always specify a correct framebuffer
> map offset, even if this offset was ignored since above commit. So this
> patch should not change anything for drivers using only one framebuffer.
>
> Drivers needing multiple framebuffers with 64-bit map offsets will need
> to cook up something, for instance keeping an ID in the lower bit which
> is to be aligned away when it comes to using the offset.
>
> All of above applies to _DRM_REGISTERS as well.
>
> Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
> ---
>
>
> On Mon, May 30, 2011 at 1:29 AM, Dave Airlie wrote:
>>
>> If you test it and it works I like it best. Simple and clear, and pretty
>> close to what I was thinking was a good idea.
>>
>> As you say if someone needs this functionality in a new driver they can
>> fix it, but really new drivers shouldn't be doing anything in this area.
>>
>> Dave.
>
>
> Whoops, there was a less obvious fallthrough from the _DRM_SHM case above,
> where we do not want to compare offsets at all if it contains a lock(*).
>
> This patch has been tested on savage, and for verification also on radeon
> with DRI1 and DRI2.
>
> Tormod

Hi Dave,

Is this good to go now and I can expect to see it in e.g. drm-next or
are there any issues or doubts?

Cheers,
Tormod