[PATCH xserver] modesetting: always set a hardware cursor when requested to load one.

[Hans de Goede]

Hi,

On 28-09-16 16:54, Michael Thayer wrote:
> Hello Hans,
>
> On 28.09.2016 15:37, Hans de Goede wrote:
>> Hi Michael,
>>
>> On 28-09-16 14:47, Michael Thayer wrote:
> [...]
>>>>>>>> On 09/16/2016 06:52 PM, Michael Thayer wrote:
>>>>>>>>> When the X server asks us to load a hardware cursor, that
>>>>>>>>> request is always followed up by a request to show it if we
>>>>>>>>> report success, or to hide it if we report failure.  Therefore
>>>>>>>>> it makes no sense to suppress the request if the cursor is not
>>>>>>>>> currently visible.
> [...]
>>> For the record, I looked at making show_cursor() return a boolean.  It
>>> seemed feasible, and would allow for removing the first time hack in
>>> modesetting too
>>
>> Making show_cursor return an error (adding a show_cursor_check()) seems
>> to be the best solution to me, we actually had a patch submitted for
>> this, but it was deemed unnecessary. I guess it is necessary after all:
>>
>> https://patchwork.freedesktop.org/patch/94495/
>>
>>> but there was a catch in that the cursor could theoretically be
>>> visible but still hidden on all screens (with a strange screen layout),
>>
>> Yes that is possible.
>>
>>> in which case show_cursor() would not be called until the cursor moved
>> onto a screen.
>>
>> Ack.
>>
>>>  Making set_cursor_position() return a success value seemed a bit too
>>> invasive.
>>
>> But in the scenario you describe it is not the drivers
>> set_cursor_position()
>> which will get called (well not only) also the drivers' show_cursor will
>> get
>> called on the crtc where the cursor should now show, so just making
>> show_cursor
>> return an error should be enough.
> [...]
>
> Right, xf86_crtc_set_cursor_position() calls the driver's show_cursor().  What I meant was that that means that this function can now fail, and that failure needs to be propagated up and probably handled in xf86CursorMoveCursor() by reverting to a software cursor there.  The alternative is another first time-style hack (if we are asked to show the cursor and it is not in range of any of the crtc-s then flicker it).

Hmm, so 2 things:

1) We definitely do not want another first-time hack, so if we really need to lets do the propagate error thing
2) Thinking about holes in crtc layouts again, I think the xrandr cursor constraint code will warp the pointer to
the closest crtc then (pretty sure actually) so I think this is a non issue. So just rebasing the patch I linked
(and dropping the first time hack) should be enough. And since we're planning to do this for 1.20, I think we
should just try doing things that way, we've an entire cycle to catch any issues then (which I do not believe
there will be)

Regards,

Hans




>
> Regards,
>
> Michael