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

2016-03-12 Thread Michael Thayer

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
 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 
---
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;
 

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

2016-03-12 Thread Jasper St. Pierre
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
 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 
>> ---
>> 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 = 

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

2016-03-12 Thread Michael Thayer

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 
---
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@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: 

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

2016-03-10 Thread Olivier Fourdan
Hi,

- Original Message -
> > [...]
> Right, I am still not too familiar with the patch process for X.Org.
> How do I mark the re-sent one, which as you said does not apply cleanly,
> as superseded?

You need to create an account in patchwork https://patchwork.freedesktop.org/ 
and link that account with your email address that you used to submit the 
patches, so you can change the status of those patches you submitted.

You may also install pwclient and the relevant .pwclientrc so you can use 
patchwork from the command line as well, very handy.

See Damien Lespiau's blog post here: 
http://damien.lespiau.name/2016/02/augmenting-mailing-lists-with-patchwork.html

Cheers,
Olivier
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

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

2016-03-10 Thread Michael Thayer

Hello Olivier,

On 10.03.2016 09:11, Olivier Fourdan wrote:

Hi Michael,

[...]

Re-sending as this did not seem to get noticed much the first
time.


I am not doing a formal review of your patch as it's outside of my
area of competences, just a few comments, trying to help.


Thank you, help welcome.


I am not sure re-sending the patch as-is is the best course of action
here, usually simply replying the original message with a gentle
reminder suffices.

Reason being that the original patch (sent only 4 days ago) is still
accessible in patchwork and still marked as "New":

https://patchwork.freedesktop.org/patch/75985/

And the new here is also available:

https://patchwork.freedesktop.org/patch/76437/

So you should mark on of the two as superseded in patchwork. But,
yes, patches do take some time for review.


Right, I am still not too familiar with the patch process for X.Org. 
How do I mark the re-sent one, which as you said does not apply cleanly, 
as superseded?



Another problem is this patch here won't apply here due to formatting
issues apparently:

$ pwc git-am 76437 Applying patch #76437 using 'git am' Description:
Re-send: [PATCH] modesetting: allow switching from software to
hardware cursors. Applying: Re-send: [PATCH] modesetting: allow
switching from software to hardware cursors. fatal: corrupt patch at
line 8 Patch failed at 0001 Re-send: [PATCH] modesetting: allow
switching from software to hardware cursors.

See below. But the original patch (the first one) doesn't seem to
suffer from the same formatting issues though.


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) {


That won't apply. There are several occurrences of the same issue in
the patch being resent, I think best would be to use git-send-email
to send patches instead of Thunderbird -as seen in the message
source- (or any other MUA for that matter) that can break the patch
formatting and confuse git.


Indeed, I sent the original with g-s-e and the re-send with Thunderbird.

Thanks again.

Regards,

Michael


HTH, Cheers, Olivier

--
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@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

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

2016-03-10 Thread Olivier Fourdan
Hi Michael,

[...]
> Re-sending as this did not seem to get noticed much the first time.

I am not doing a formal review of your patch as it's outside of my area of 
competences, just a few comments, trying to help.

I am not sure re-sending the patch as-is is the best course of action here, 
usually simply replying the original message with a gentle reminder suffices.

Reason being that the original patch (sent only 4 days ago) is still accessible 
in patchwork and still marked as "New":

https://patchwork.freedesktop.org/patch/75985/

And the new here is also available:

https://patchwork.freedesktop.org/patch/76437/

So you should mark on of the two as superseded in patchwork. But, yes, patches 
do take some time for review.

Another problem is this patch here won't apply here due to formatting issues 
apparently:

  $ pwc git-am 76437
  Applying patch #76437 using 'git am'
  Description: Re-send: [PATCH] modesetting: allow switching from software to 
hardware cursors.
  Applying: Re-send: [PATCH] modesetting: allow switching from software to 
hardware cursors.
  fatal: corrupt patch at line 8
  Patch failed at 0001 Re-send: [PATCH] modesetting: allow switching from 
software to hardware cursors.

See below. But the original patch (the first one) doesn't seem to suffer from 
the same formatting issues though.

>   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) {

That won't apply. There are several occurrences of the same issue in the patch 
being resent, I think best would be to use git-send-email to send patches 
instead of Thunderbird -as seen in the message source- (or any other MUA for 
that matter) that can break the patch formatting and confuse git.

HTH,
Cheers,
Olivier
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

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

2016-03-09 Thread Michael Thayer

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 
---
Re-sending as this did not seem to get noticed much the first time.

 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 */
--
2.5.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

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

2016-03-06 Thread Michael Thayer
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 
---
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 */
-- 
2.5.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

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

2016-03-02 Thread Michael Thayer
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.

Signed-off-by: Michael Thayer 
---
 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 */
-- 
2.5.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel