[PATCH] modesetting: allow switching from software to hardware cursors.

Michael Thayer michael.thayer at oracle.com
Sat Mar 12 19:53:16 UTC 2016


Hello Jasper,

My case is virtual machine cursor pass-through.  VirtualBox has a 
setting which lets the user enable or disable guest-host cursor 
integration.  When it is enabled, the guest pointer position tracks the 
host one, and the guest cursor is rendered by the host.  When it is 
disabled, the guest and host pointer positions are independent, and in 
this case the guest renders its cursor itself in software.  The user can 
switch between the two at any time, though of course in practice 
switches do not occur in close succession.  But if integration was 
previously enabled, then the user disables it and the guest re-loads the 
previous cursor, it will be told that hardware rendering is not 
possible, which it was previously (and vice-versa).

Sorry that was so long - hope it was clear enough.

Regards,

Michael

On 12.03.2016 20:19, Jasper St. Pierre wrote:
> Can you give an example of when drmSetCursor2 would fail sometimes but
> not always, enough to switch back and forth actively?
>
> I'm not a huge fan of the patch because now errors could cause really
> bizarre double-cursor, or no-cursor flickering, since there is no
> guarantee that the swapover happens in the same frame.
>
> We sort-of tried this in Weston for something else (we rendered
> certain windows in overlays) and had to turn it off and go to always
> SW because of the obnoxious flickering, at least until atomic landed.
>
> On Sat, Mar 12, 2016 at 3:54 AM, Michael Thayer
> <michael.thayer at oracle.com> wrote:
>> Hello,
>>
>> I would just like to politely draw attention to this again.  I would assume
>> from the silence that the people who might review it don't think it is a
>> completely wrong thing (that would not take long to say) but don't have the
>> spare cycles to take a closer look.  Any idea when someone might have twenty
>> minutes to review it?
>>
>> Regards,
>>
>> Michael
>>
>>
>> On 06.03.2016 12:56, Michael Thayer wrote:
>>>
>>> Currently if modesetting ever fails to set a hardware cursor it will
>>> switch
>>> to using a software cursor and never go back.  Change this to try a
>>> hardware
>>> cursor first every time a new one is set.  This is needed because hardware
>>> may be able to handle some cursors in harware and others not, or virtual
>>> hardware may be able to handle hardware cursors at some times and not
>>> others.
>>>
>>> Signed-off-by: Michael Thayer <michael.thayer at oracle.com>
>>> ---
>>> Made the commit message more verbose on Emil's request.
>>>
>>>    hw/xfree86/drivers/modesetting/drmmode_display.c | 47
>>> ++++++++++--------------
>>>    1 file changed, 20 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c
>>> b/hw/xfree86/drivers/modesetting/drmmode_display.c
>>> index 0d34ca1..36c3093 100644
>>> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
>>> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
>>> @@ -485,44 +485,36 @@ drmmode_set_cursor_position(xf86CrtcPtr crtc, int x,
>>> int y)
>>>        drmModeMoveCursor(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id, x,
>>> y);
>>>    }
>>>
>>> -static void
>>> +static Bool
>>>    drmmode_set_cursor(xf86CrtcPtr crtc)
>>>    {
>>>        drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
>>>        drmmode_ptr drmmode = drmmode_crtc->drmmode;
>>>        uint32_t handle = drmmode_crtc->cursor_bo->handle;
>>>        modesettingPtr ms = modesettingPTR(crtc->scrn);
>>> -    static Bool use_set_cursor2 = TRUE;
>>>        int ret;
>>>
>>> -    if (use_set_cursor2) {
>>> -        xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(crtc->scrn);
>>> -        CursorPtr cursor = xf86_config->cursor;
>>> -
>>> -        ret =
>>> -            drmModeSetCursor2(drmmode->fd,
>>> drmmode_crtc->mode_crtc->crtc_id,
>>> -                              handle, ms->cursor_width,
>>> ms->cursor_height,
>>> -                              cursor->bits->xhot, cursor->bits->yhot);
>>> -        if (!ret)
>>> -            return;
>>> -        if (ret == -EINVAL)
>>> -            use_set_cursor2 = FALSE;
>>> -    }
>>> +    xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(crtc->scrn);
>>> +    CursorPtr cursor = xf86_config->cursor;
>>>
>>> -    ret = drmModeSetCursor(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
>>> handle,
>>> -                           ms->cursor_width, ms->cursor_height);
>>> +    ret =
>>> +        drmModeSetCursor2(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
>>> +                          handle, ms->cursor_width, ms->cursor_height,
>>> +                          cursor->bits->xhot, cursor->bits->yhot);
>>> +    if (!ret)
>>> +        return TRUE;
>>>
>>> -    if (ret) {
>>> -        xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(crtc->scrn);
>>> -        xf86CursorInfoPtr cursor_info = xf86_config->cursor_info;
>>> +    /* -EINVAL can mean bad cursor parameters or Cursor2 API not
>>> supported. */
>>> +    if (ret == -EINVAL)
>>> +        ret = drmModeSetCursor(drmmode->fd,
>>> drmmode_crtc->mode_crtc->crtc_id,
>>> +                               handle, ms->cursor_width,
>>> ms->cursor_height);
>>>
>>> -        cursor_info->MaxWidth = cursor_info->MaxHeight = 0;
>>> -        drmmode_crtc->drmmode->sw_cursor = TRUE;
>>> -        /* fallback to swcursor */
>>> -    }
>>> +    if (ret)
>>> +        return FALSE; /* fallback to swcursor */
>>> +    return TRUE;
>>>    }
>>>
>>> -static void
>>> +static Bool
>>>    drmmode_load_cursor_argb(xf86CrtcPtr crtc, CARD32 *image)
>>>    {
>>>        modesettingPtr ms = modesettingPTR(crtc->scrn);
>>> @@ -537,7 +529,8 @@ drmmode_load_cursor_argb(xf86CrtcPtr crtc, CARD32
>>> *image)
>>>            ptr[i] = image[i];      // cpu_to_le32(image[i]);
>>>
>>>        if (drmmode_crtc->cursor_up)
>>> -        drmmode_set_cursor(crtc);
>>> +        return drmmode_set_cursor(crtc);
>>> +    return TRUE;
>>>    }
>>>
>>>    static void
>>> @@ -799,7 +792,7 @@ static const xf86CrtcFuncsRec drmmode_crtc_funcs = {
>>>        .set_cursor_position = drmmode_set_cursor_position,
>>>        .show_cursor = drmmode_show_cursor,
>>>        .hide_cursor = drmmode_hide_cursor,
>>> -    .load_cursor_argb = drmmode_load_cursor_argb,
>>> +    .load_cursor_argb_check = drmmode_load_cursor_argb,
>>>
>>>        .gamma_set = drmmode_crtc_gamma_set,
>>>        .destroy = NULL,            /* XXX */
>>>
>>
>> --
>> Michael Thayer | VirtualBox engineer
>> ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt
>>
>> ORACLE Deutschland B.V. & Co. KG
>> Hauptverwaltung: Riesstraße 25, D-80992 München
>> Registergericht: Amtsgericht München, HRA 95603
>>
>> Komplementärin: ORACLE Deutschland Verwaltung B.V.
>> Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
>> Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697
>> Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
>>
>> _______________________________________________
>> xorg-devel at lists.x.org: X.Org development
>> Archives: http://lists.x.org/archives/xorg-devel
>> Info: https://lists.x.org/mailman/listinfo/xorg-devel
>
>
>

-- 
Michael Thayer | VirtualBox engineer
ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt

ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstraße 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher


More information about the xorg-devel mailing list