subject:"\[PATCH xserver v8 05\/14\] modesetting\: Use atomic modesetting API for pageflip if available"

Re: [PATCH xserver v8 05/14] modesetting: Use atomic modesetting API for pageflip if available

2018-10-01 Thread Adam Jackson

On Mon, 2018-10-01 at 11:31 -0400, Adam Jackson wrote:

> AFAICT the free(dst) is always wrong. The other places we use
> drmmode_prop_info_copy, we're initializing an array in the middle of a
> drmmode_{crtc,output}_private_rec.

https://gitlab.freedesktop.org/xorg/xserver/merge_requests/33

- ajax

___
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: [PATCH xserver v8 05/14] modesetting: Use atomic modesetting API for pageflip if available

2018-10-01 Thread Adam Jackson

On Wed, 2018-09-12 at 11:01 +1000, Dave Airlie wrote:
> On Wed, 28 Feb 2018 at 11:21, Daniel Stone  wrote:
> > 
> > +static Bool
> > +drmmode_prop_info_copy(drmmode_prop_info_ptr dst,
> > +  const drmmode_prop_info_rec *src,
> > +  unsigned int num_props,
> > +  Bool copy_prop_id)
> > +{
> > +unsigned int i;
> > +
> > +memcpy(dst, src, num_props * sizeof(*dst));
> > +
> > +for (i = 0; i < num_props; i++) {
> > +unsigned int j;
> > +
> > +if (copy_prop_id)
> > +dst[i].prop_id = src[i].prop_id;
> > +else
> > +dst[i].prop_id = 0;
> > +
> > +if (src[i].num_enum_values == 0)
> > +continue;
> > +
> > +dst[i].enum_values =
> > +malloc(src[i].num_enum_values *
> > +sizeof(*dst[i].enum_values));
> > +if (!dst[i].enum_values)
> > +goto err;
> > +
> > +memcpy(dst[i].enum_values, src[i].enum_values,
> > +src[i].num_enum_values * sizeof(*dst[i].enum_values));
> > +
> > +for (j = 0; j < dst[i].num_enum_values; j++)
> > +dst[i].enum_values[j].valid = FALSE;
> > +}
> > +
> > +return TRUE;
> > +
> > +err:
> > +while (i--)
> > +free(dst[i].enum_values);
> > +free(dst);
> > +return FALSE;
> > +}
> 
> On failure this frees dst, however...
> 
> > +drmmode_crtc_create_planes(xf86CrtcPtr crtc, int num)
> > +{
> > +drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> > +drmmode_ptr drmmode = drmmode_crtc->drmmode;
> > +drmModePlaneRes *kplane_res;
> > +drmModePlane *kplane;
> > +drmModeObjectProperties *props;
> > +uint32_t i, type;
> > +int current_crtc, best_plane = 0;
> > +
> > +static drmmode_prop_enum_info_rec plane_type_enums[] = {
> > +[DRMMODE_PLANE_TYPE_PRIMARY] = {
> > +.name = "Primary",
> > +},
> > +[DRMMODE_PLANE_TYPE_OVERLAY] = {
> > +.name = "Overlay",
> > +},
> > +[DRMMODE_PLANE_TYPE_CURSOR] = {
> > +.name = "Cursor",
> > +},
> > +};
> > +static const drmmode_prop_info_rec plane_props[] = {
> > +[DRMMODE_PLANE_TYPE] = {
> > +.name = "type",
> > +.enum_values = plane_type_enums,
> > +.num_enum_values = DRMMODE_PLANE_TYPE__COUNT,
> > +},
> > +[DRMMODE_PLANE_FB_ID] = { .name = "FB_ID", },
> > +[DRMMODE_PLANE_CRTC_ID] = { .name = "CRTC_ID", },
> > +[DRMMODE_PLANE_SRC_X] = { .name = "SRC_X", },
> > +[DRMMODE_PLANE_SRC_Y] = { .name = "SRC_Y", },
> > +};
> > +drmmode_prop_info_rec tmp_props[DRMMODE_PLANE__COUNT];
> 
> We use a stack allocated tmp_props here.

AFAICT the free(dst) is always wrong. The other places we use
drmmode_prop_info_copy, we're initializing an array in the middle of a
drmmode_{crtc,output}_private_rec.

- ajax

___
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: [PATCH xserver v8 05/14] modesetting: Use atomic modesetting API for pageflip if available

2018-09-11 Thread Dave Airlie

On Wed, 28 Feb 2018 at 11:21, Daniel Stone  wrote:
>
> From: Louis-Francis Ratté-Boulianne 
>
> In order to flip between compressed and uncompressed buffers -
> something drmModePageFlip explicitly bans us from doing - we need
> to port use the atomic modesetting API. It's only 'fake' atomic
> though given we still commit for each CRTC separately and
> CRTC and connector properties are not set with the atomic API.
>
> The helper functions to retrieve DRM properties have been borrowed
> from Weston.
>
> Signed-off-by: Louis-Francis Ratté-Boulianne 
> Reviewed-by: Daniel Stone 
> ---
>  configure.ac |   3 +
>  hw/xfree86/drivers/modesetting/driver.c  |   6 +
>  hw/xfree86/drivers/modesetting/driver.h  |   1 +
>  hw/xfree86/drivers/modesetting/drmmode_display.c | 374 
> ++-
>  hw/xfree86/drivers/modesetting/drmmode_display.h |  36 +++
>  hw/xfree86/drivers/modesetting/pageflip.c|  22 +-
>  include/dix-config.h.in  |   3 +
>  include/meson.build  |   2 +
>  8 files changed, 443 insertions(+), 4 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index cef9503d7..46662867f 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2106,6 +2106,9 @@ if test "x$GLAMOR" = xyes; then
> fi
> fi
>
> +   PKG_CHECK_EXISTS(libdrm >= 2.4.62,
> +[AC_DEFINE(GLAMOR_HAS_DRM_ATOMIC, 1, [libdrm 
> supports atomic API])],
> +[])
> PKG_CHECK_EXISTS(libdrm >= 2.4.74,
>  [AC_DEFINE(GLAMOR_HAS_DRM_NAME_FROM_FD_2, 1, [Have 
> GLAMOR_HAS_DRM_NAME_FROM_FD_2])],
>  [])
> diff --git a/hw/xfree86/drivers/modesetting/driver.c 
> b/hw/xfree86/drivers/modesetting/driver.c
> index ec2aa9a27..88a42257c 100644
> --- a/hw/xfree86/drivers/modesetting/driver.c
> +++ b/hw/xfree86/drivers/modesetting/driver.c
> @@ -1018,6 +1018,12 @@ PreInit(ScrnInfoPtr pScrn, int flags)
>  }
>  #endif
>
> +#ifdef GLAMOR_HAS_DRM_ATOMIC
> +ret = drmSetClientCap(ms->fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> +ret |= drmSetClientCap(ms->fd, DRM_CLIENT_CAP_ATOMIC, 1);
> +ms->atomic_modeset = (ret == 0);
> +#endif
> +
>  if (drmmode_pre_init(pScrn, >drmmode, pScrn->bitsPerPixel / 8) == 
> FALSE) {
>  xf86DrvMsg(pScrn->scrnIndex, X_ERROR, "KMS setup failed\n");
>  goto fail;
> diff --git a/hw/xfree86/drivers/modesetting/driver.h 
> b/hw/xfree86/drivers/modesetting/driver.h
> index fe835918b..ed32239db 100644
> --- a/hw/xfree86/drivers/modesetting/driver.h
> +++ b/hw/xfree86/drivers/modesetting/driver.h
> @@ -105,6 +105,7 @@ typedef struct _modesettingRec {
>   * Page flipping stuff.
>   *  @{
>   */
> +Bool atomic_modeset;
>  /** @} */
>
>  DamagePtr damage;
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
> b/hw/xfree86/drivers/modesetting/drmmode_display.c
> index bfc8c50db..8674c2c16 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> @@ -75,6 +75,233 @@ drmmode_zaphod_string_matches(ScrnInfoPtr scrn, const 
> char *s, char *output_name
>  return ret;
>  }
>
> +static uint64_t
> +drmmode_prop_get_value(drmmode_prop_info_ptr info,
> +   drmModeObjectPropertiesPtr props,
> +   uint64_t def)
> +{
> +unsigned int i;
> +
> +if (info->prop_id == 0)
> +return def;
> +
> +for (i = 0; i < props->count_props; i++) {
> +unsigned int j;
> +
> +if (props->props[i] != info->prop_id)
> +continue;
> +
> +/* Simple (non-enum) types can return the value directly */
> +if (info->num_enum_values == 0)
> +return props->prop_values[i];
> +
> +/* Map from raw value to enum value */
> +for (j = 0; j < info->num_enum_values; j++) {
> +if (!info->enum_values[j].valid)
> +continue;
> +if (info->enum_values[j].value != props->prop_values[i])
> +continue;
> +
> +return j;
> +}
> +}
> +
> +return def;
> +}
> +
> +static uint32_t
> +drmmode_prop_info_update(drmmode_ptr drmmode,
> + drmmode_prop_info_ptr info,
> + unsigned int num_infos,
> + drmModeObjectProperties *props)
> +{
> +drmModePropertyRes *prop;
> +uint32_t valid_mask = 0;
> +unsigned i, j;
> +
> +assert(num_infos <= 32 && "update return type");
> +
> +for (i = 0; i < props->count_props; i++) {
> +Bool props_incomplete = FALSE;
> +unsigned int k;
> +
> +for (j = 0; j < num_infos; j++) {
> +if (info[j].prop_id == props->props[i])
> +break;
> +if (!info[j].prop_id)
> +props_incomplete = TRUE;
> +}
> +
> +/* We've already discovered

[PATCH xserver v8 05/14] modesetting: Use atomic modesetting API for pageflip if available

2018-02-27 Thread Daniel Stone

From: Louis-Francis Ratté-Boulianne 

In order to flip between compressed and uncompressed buffers -
something drmModePageFlip explicitly bans us from doing - we need
to port use the atomic modesetting API. It's only 'fake' atomic
though given we still commit for each CRTC separately and
CRTC and connector properties are not set with the atomic API.

The helper functions to retrieve DRM properties have been borrowed
from Weston.

Signed-off-by: Louis-Francis Ratté-Boulianne 
Reviewed-by: Daniel Stone 
---
 configure.ac |   3 +
 hw/xfree86/drivers/modesetting/driver.c  |   6 +
 hw/xfree86/drivers/modesetting/driver.h  |   1 +
 hw/xfree86/drivers/modesetting/drmmode_display.c | 374 ++-
 hw/xfree86/drivers/modesetting/drmmode_display.h |  36 +++
 hw/xfree86/drivers/modesetting/pageflip.c|  22 +-
 include/dix-config.h.in  |   3 +
 include/meson.build  |   2 +
 8 files changed, 443 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index cef9503d7..46662867f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2106,6 +2106,9 @@ if test "x$GLAMOR" = xyes; then
fi
fi
 
+   PKG_CHECK_EXISTS(libdrm >= 2.4.62,
+[AC_DEFINE(GLAMOR_HAS_DRM_ATOMIC, 1, [libdrm supports 
atomic API])],
+[])
PKG_CHECK_EXISTS(libdrm >= 2.4.74,
 [AC_DEFINE(GLAMOR_HAS_DRM_NAME_FROM_FD_2, 1, [Have 
GLAMOR_HAS_DRM_NAME_FROM_FD_2])],
 [])
diff --git a/hw/xfree86/drivers/modesetting/driver.c 
b/hw/xfree86/drivers/modesetting/driver.c
index ec2aa9a27..88a42257c 100644
--- a/hw/xfree86/drivers/modesetting/driver.c
+++ b/hw/xfree86/drivers/modesetting/driver.c
@@ -1018,6 +1018,12 @@ PreInit(ScrnInfoPtr pScrn, int flags)
 }
 #endif
 
+#ifdef GLAMOR_HAS_DRM_ATOMIC
+ret = drmSetClientCap(ms->fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
+ret |= drmSetClientCap(ms->fd, DRM_CLIENT_CAP_ATOMIC, 1);
+ms->atomic_modeset = (ret == 0);
+#endif
+
 if (drmmode_pre_init(pScrn, >drmmode, pScrn->bitsPerPixel / 8) == 
FALSE) {
 xf86DrvMsg(pScrn->scrnIndex, X_ERROR, "KMS setup failed\n");
 goto fail;
diff --git a/hw/xfree86/drivers/modesetting/driver.h 
b/hw/xfree86/drivers/modesetting/driver.h
index fe835918b..ed32239db 100644
--- a/hw/xfree86/drivers/modesetting/driver.h
+++ b/hw/xfree86/drivers/modesetting/driver.h
@@ -105,6 +105,7 @@ typedef struct _modesettingRec {
  * Page flipping stuff.
  *  @{
  */
+Bool atomic_modeset;
 /** @} */
 
 DamagePtr damage;
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
b/hw/xfree86/drivers/modesetting/drmmode_display.c
index bfc8c50db..8674c2c16 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -75,6 +75,233 @@ drmmode_zaphod_string_matches(ScrnInfoPtr scrn, const char 
*s, char *output_name
 return ret;
 }
 
+static uint64_t
+drmmode_prop_get_value(drmmode_prop_info_ptr info,
+   drmModeObjectPropertiesPtr props,
+   uint64_t def)
+{
+unsigned int i;
+
+if (info->prop_id == 0)
+return def;
+
+for (i = 0; i < props->count_props; i++) {
+unsigned int j;
+
+if (props->props[i] != info->prop_id)
+continue;
+
+/* Simple (non-enum) types can return the value directly */
+if (info->num_enum_values == 0)
+return props->prop_values[i];
+
+/* Map from raw value to enum value */
+for (j = 0; j < info->num_enum_values; j++) {
+if (!info->enum_values[j].valid)
+continue;
+if (info->enum_values[j].value != props->prop_values[i])
+continue;
+
+return j;
+}
+}
+
+return def;
+}
+
+static uint32_t
+drmmode_prop_info_update(drmmode_ptr drmmode,
+ drmmode_prop_info_ptr info,
+ unsigned int num_infos,
+ drmModeObjectProperties *props)
+{
+drmModePropertyRes *prop;
+uint32_t valid_mask = 0;
+unsigned i, j;
+
+assert(num_infos <= 32 && "update return type");
+
+for (i = 0; i < props->count_props; i++) {
+Bool props_incomplete = FALSE;
+unsigned int k;
+
+for (j = 0; j < num_infos; j++) {
+if (info[j].prop_id == props->props[i])
+break;
+if (!info[j].prop_id)
+props_incomplete = TRUE;
+}
+
+/* We've already discovered this property. */
+if (j != num_infos)
+continue;
+
+/* We haven't found this property ID, but as we've already
+ * found all known properties, we don't need to look any
+ * further. */
+if (!props_incomplete)

[PATCH xserver v8 05/14] modesetting: Use atomic modesetting API for pageflip if available

2018-02-27 Thread Daniel Stone

From: Louis-Francis Ratté-Boulianne 

In order to flip between compressed and uncompressed buffers -
something drmModePageFlip explicitly bans us from doing - we need
to port use the atomic modesetting API. It's only 'fake' atomic
though given we still commit for each CRTC separately and
CRTC and connector properties are not set with the atomic API.

The helper functions to retrieve DRM properties have been borrowed
from Weston.

Signed-off-by: Louis-Francis Ratté-Boulianne 
Reviewed-by: Daniel Stone 
---
 configure.ac |   3 +
 hw/xfree86/drivers/modesetting/driver.c  |   6 +
 hw/xfree86/drivers/modesetting/driver.h  |   1 +
 hw/xfree86/drivers/modesetting/drmmode_display.c | 374 ++-
 hw/xfree86/drivers/modesetting/drmmode_display.h |  36 +++
 hw/xfree86/drivers/modesetting/pageflip.c|  22 +-
 include/dix-config.h.in  |   3 +
 include/meson.build  |   2 +
 8 files changed, 443 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index cef9503d7..46662867f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2106,6 +2106,9 @@ if test "x$GLAMOR" = xyes; then
fi
fi
 
+   PKG_CHECK_EXISTS(libdrm >= 2.4.62,
+[AC_DEFINE(GLAMOR_HAS_DRM_ATOMIC, 1, [libdrm supports 
atomic API])],
+[])
PKG_CHECK_EXISTS(libdrm >= 2.4.74,
 [AC_DEFINE(GLAMOR_HAS_DRM_NAME_FROM_FD_2, 1, [Have 
GLAMOR_HAS_DRM_NAME_FROM_FD_2])],
 [])
diff --git a/hw/xfree86/drivers/modesetting/driver.c 
b/hw/xfree86/drivers/modesetting/driver.c
index ec2aa9a27..88a42257c 100644
--- a/hw/xfree86/drivers/modesetting/driver.c
+++ b/hw/xfree86/drivers/modesetting/driver.c
@@ -1018,6 +1018,12 @@ PreInit(ScrnInfoPtr pScrn, int flags)
 }
 #endif
 
+#ifdef GLAMOR_HAS_DRM_ATOMIC
+ret = drmSetClientCap(ms->fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
+ret |= drmSetClientCap(ms->fd, DRM_CLIENT_CAP_ATOMIC, 1);
+ms->atomic_modeset = (ret == 0);
+#endif
+
 if (drmmode_pre_init(pScrn, >drmmode, pScrn->bitsPerPixel / 8) == 
FALSE) {
 xf86DrvMsg(pScrn->scrnIndex, X_ERROR, "KMS setup failed\n");
 goto fail;
diff --git a/hw/xfree86/drivers/modesetting/driver.h 
b/hw/xfree86/drivers/modesetting/driver.h
index fe835918b..ed32239db 100644
--- a/hw/xfree86/drivers/modesetting/driver.h
+++ b/hw/xfree86/drivers/modesetting/driver.h
@@ -105,6 +105,7 @@ typedef struct _modesettingRec {
  * Page flipping stuff.
  *  @{
  */
+Bool atomic_modeset;
 /** @} */
 
 DamagePtr damage;
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
b/hw/xfree86/drivers/modesetting/drmmode_display.c
index bfc8c50db..8674c2c16 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -75,6 +75,233 @@ drmmode_zaphod_string_matches(ScrnInfoPtr scrn, const char 
*s, char *output_name
 return ret;
 }
 
+static uint64_t
+drmmode_prop_get_value(drmmode_prop_info_ptr info,
+   drmModeObjectPropertiesPtr props,
+   uint64_t def)
+{
+unsigned int i;
+
+if (info->prop_id == 0)
+return def;
+
+for (i = 0; i < props->count_props; i++) {
+unsigned int j;
+
+if (props->props[i] != info->prop_id)
+continue;
+
+/* Simple (non-enum) types can return the value directly */
+if (info->num_enum_values == 0)
+return props->prop_values[i];
+
+/* Map from raw value to enum value */
+for (j = 0; j < info->num_enum_values; j++) {
+if (!info->enum_values[j].valid)
+continue;
+if (info->enum_values[j].value != props->prop_values[i])
+continue;
+
+return j;
+}
+}
+
+return def;
+}
+
+static uint32_t
+drmmode_prop_info_update(drmmode_ptr drmmode,
+ drmmode_prop_info_ptr info,
+ unsigned int num_infos,
+ drmModeObjectProperties *props)
+{
+drmModePropertyRes *prop;
+uint32_t valid_mask = 0;
+unsigned i, j;
+
+assert(num_infos <= 32 && "update return type");
+
+for (i = 0; i < props->count_props; i++) {
+Bool props_incomplete = FALSE;
+unsigned int k;
+
+for (j = 0; j < num_infos; j++) {
+if (info[j].prop_id == props->props[i])
+break;
+if (!info[j].prop_id)
+props_incomplete = TRUE;
+}
+
+/* We've already discovered this property. */
+if (j != num_infos)
+continue;
+
+/* We haven't found this property ID, but as we've already
+ * found all known properties, we don't need to look any
+ * further. */
+if (!props_incomplete)

Re: [PATCH xserver v8 05/14] modesetting: Use atomic modesetting API for pageflip if available

Re: [PATCH xserver v8 05/14] modesetting: Use atomic modesetting API for pageflip if available

Re: [PATCH xserver v8 05/14] modesetting: Use atomic modesetting API for pageflip if available

[PATCH xserver v8 05/14] modesetting: Use atomic modesetting API for pageflip if available

[PATCH xserver v8 05/14] modesetting: Use atomic modesetting API for pageflip if available

5 matches

Site Navigation

Mail list logo

Footer information