[PATCH xserver] modesetting: Fix msSharePixmapBacking returning a non-linear bo

[Hans de Goede]

Hi,

On 17-08-16 16:44, Keith Packard wrote:
> Hans de Goede <hdegoede at redhat.com> writes:
>
>> Some code paths end up in msSharePixmapBacking with a pixmap
>> which does not have its usage_hint set to sharable.
>>
>> This causes glamor_fd_from_pixmap() to create a non-linear bo,
>> which is wrong for a shared pixmap.
>>
>> This commits sets the pixmap usage hint to shared before
>> calling glamor_fd_from_pixmap(), fixing this.
>>
>> Specifically this fixes mis-rendering when running the mode
>> setting driver on the master gpu in a dual-gpu setup and
>> running an opengl app with DRI_PRIME=1.
>>
>> One could argue that this is a problem of the caller, but the
>> usage_hint is as the name implies just a hint. I've tried
>> tracing the DRI_PRIME case from above, but the pixmap
>> ends up coming from a drawable passed into the server by
>> a client.
>>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>>  hw/xfree86/drivers/modesetting/driver.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/xfree86/drivers/modesetting/driver.c b/hw/xfree86/drivers/modesetting/driver.c
>> index 5ebb394..023d491 100644
>> --- a/hw/xfree86/drivers/modesetting/driver.c
>> +++ b/hw/xfree86/drivers/modesetting/driver.c
>> @@ -1382,7 +1382,12 @@ msSharePixmapBacking(PixmapPtr ppix, ScreenPtr screen, void **handle)
>>      int ret;
>>      CARD16 stride;
>>      CARD32 size;
>> +    unsigned orig_usage_hint = ppix->usage_hint;
>> +
>> +    /* Ensure that glamor_fd_from_pixmap creates a sharable (linear) bo */
>> +    ppix->usage_hint = CREATE_PIXMAP_USAGE_SHARED;
>>      ret = glamor_fd_from_pixmap(ppix->drawable.pScreen, ppix, &stride, &size);
>> +    ppix->usage_hint = orig_usage_hint;
>
> That just can't be the right way to fix this...

That is what I thought, see the commit message. Trying to come up
with a better fix did not get me anywhere.

An alternative solution would be to add a flag argument to
glamor_fd_from_pixmap, so that callers like this one which
know for sure they want a linear bo can specify that through
a flag, and other callers can just pass in:

ppix->usage_hint == CREATE_PIXMAP_USAGE_SHARED

For that flag, the reason I did not do that is because
glamor_fd_from_pixmap is part of the xserver ABI AFAICT.

 > Not that I have a better idea at this point.

Is that a convoluted way of acking the patch ?   :)

Regards,

Hans