[Xf86-video-armsoc] [PATCH 1/1] Added HWCURSOR_API_NONE to the platform specific hwcursor api.

[Daniel Kurtz]

On Fri, Feb 21, 2014 at 1:54 AM,  <armsoc-bugs at arm.com> wrote:
> From: Dave Barnish <dave.barnish at arm.com>
>

Hi Dave,

So, below please find some review comments for this patch... but it
looks like it already made it into the repository.
I think we need to work on the process a bit so patches are sent to
the list first, and only later committed.
At least, that is what I'm used to for other open source projects.

> New option allows a software-only cursor to be specified
>
> Change-Id: Ice4bd122ab6b24146d50dcd497a9a70831fc1274
> ---
>  src/drmmode_display.c |   16 +++++++++++-----
>  src/drmmode_driver.h  |   18 ++++++++++++------
>  2 files changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> index f7b0dc6..b9e59f1 100644
> --- a/src/drmmode_display.c
> +++ b/src/drmmode_display.c
> @@ -424,7 +424,7 @@ drmmode_hide_cursor(xf86CrtcPtr crtc)
>  /*
>   * The argument "update_image" controls whether the cursor image needs
>   * to be updated by the HW or not. This is ignored by HWCURSOR_API_PLANE
> - * which doesn't allow changing the cursor possition without updating
> + * which doesn't allow changing the cursor position without updating
>   * the image too.
>   */
>  static void
> @@ -512,7 +512,7 @@ drmmode_set_cursor_position(xf86CrtcPtr crtc, int x, int y)
>         cursor->y = y;
>
>         /*
> -        * Show the cursor at a different possition without updating the image
> +        * Show the cursor at a different position without updating the image
>          * when possible (HWCURSOR_API_PLANE doesn't have a way to update
>          * cursor position without updating the image too).
>          */
> @@ -769,10 +769,16 @@ Bool drmmode_cursor_init(ScreenPtr pScreen)
>
>         INFO_MSG("HW cursor init()");
>
> -       if (pARMSOC->drmmode_interface->cursor_api == HWCURSOR_API_PLANE)
> +       switch (pARMSOC->drmmode_interface->cursor_api) {
> +       case HWCURSOR_API_PLANE:
>                 return drmmode_cursor_init_plane(pScreen);
> -       else /* HWCURSOR_API_STANDARD */
> +       case HWCURSOR_API_STANDARD:
>                 return drmmode_cursor_init_standard(pScreen);
> +       case HWCURSOR_API_NONE:
> +               return FALSE;
> +       default:
> +               assert(0);

Print error message, set pARMSOC->drmmode_interface->cursor_api =
HWCURSOR_API_NONE, and return FALSE to fall back to SW cursor.

> +       }
>  }
>
>  void drmmode_cursor_fini(ScreenPtr pScreen)
> @@ -1573,7 +1579,7 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, int fd, int cpp)
>
>         TRACE_ENTER();
>
> -       drmmode = calloc(1, sizeof *drmmode);
> +       drmmode = calloc(1, sizeof(*drmmode));

This was correct as it was.

>         if (!drmmode)
>                 return FALSE;
>
> diff --git a/src/drmmode_driver.h b/src/drmmode_driver.h
> index 0e802b9..a46042c 100644
> --- a/src/drmmode_driver.h
> +++ b/src/drmmode_driver.h
> @@ -34,6 +34,7 @@
>  enum hwcursor_api {
>         HWCURSOR_API_PLANE = 0,
>         HWCURSOR_API_STANDARD = 1,
> +       HWCURSOR_API_NONE = 2

Any reason this enum needs explicit values?

>  };
>
>  struct drmmode_interface {
> @@ -53,16 +54,21 @@ struct drmmode_interface {
>          */
>         int cursor_padding;
>
> -       /* This specifies whether the DRM implements HW cursor support
> -        * using planes or the standard HW cursor API using drmModeSetCursor()
> -        * and drmModeMoveCursor().
> +       /* Specifies the hardware cursor api used by the DRM :
> +        *   HWCURSOR_API_PLANE    - Use planes.
> +        *   HWCURSOR_API_STANDARD - Use the standard API : drmModeSetCursor() & drmModeMoveCursor().
> +        *   HWCURSOR_API_NONE     - No hardware cursor - use a software cursor.
>          */
>         enum hwcursor_api cursor_api;
>
> -       /* (Optional) Initialize the given plane for use as a hardware cursor.
> +       /* (Optional) Initialize the hardware cursor plane.
>          *
> -        * This function should do any initialization necessary, for example
> -        * setting the z-order on the plane to appear above all other layers.
> +        * When cursor_api is HWCURSOR_API_PLANE, this function should do any
> +        * plane initialization necessary, for example setting the z-order on the
> +        * plane to appear above all other layers. If this function fails the driver
> +        * falls back to using a software cursor.
> +        *
> +        * If cursor_api is not HWCURSOR_API_PLANE this function should be omitted.
>          *
>          * @param drm_fd   The DRM device file
>          * @param plane_id The plane to initialize
> --
> 1.7.9.5
>
> _______________________________________________
> Xf86-video-armsoc mailing list
> Xf86-video-armsoc at lists.freedesktop.org
> http://lists.x.org/mailman/listinfo/xf86-video-armsoc