[PATCH xserver] modesetting: Fix page flipping under DRI 3.2.

Mon Mar 26 03:46:38 UTC 2018

Hi,

What do you mean by pageflipping not properly working? I've tried to
reproduce the issue, and pageflipping seems to work fine.
Also, we can't unfortunately just use always XRGB formats in glamor (we
need to check wheither depth is 24 or 32) as it would break actual
partly-transparent or translucent pixmaps.

Thank you,

--
Louis-Francis

On Fri, 2018-03-23 at 15:07 +0100, Mario Kleiner wrote:
> Unfortunately not the real fix. I developed the same patch like you
> propose in that bugzilla as a 1st try:
> 
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> @@ -711,9 +711,9 @@ drmmode_create_bo(drmmode_ptr drmmode, drmmode_bo
> *bo,
>      uint32_t format;
> 
>      if (drmmode->scrn->depth == 30)
> -        format = GBM_FORMAT_ARGB2101010;
> +        format = GBM_FORMAT_XRGB2101010;
>      else
> -        format = GBM_FORMAT_ARGB8888;
> +        format = GBM_FORMAT_XRGB8888;
> 
>      bo->width = width;
>      bo->height = height;
> 
> While that makes modesetting work, now pageflipping is broken again.

> And if i select screen depth 30 i don't make it through server
> startup, but the server crashes somewhere deep inside pixman. And if
> i
> change formats inside glamor/glamor_egl.c to be consistent with the
> new gbm bo then that doesn't help either. For reference, part II of
> doesn't work either:
> 
> diff --git a/glamor/glamor_egl.c b/glamor/glamor_egl.c
> index 2ea3efc58..787a16620 100644
> --- a/glamor/glamor_egl.c
> +++ b/glamor/glamor_egl.c
> @@ -272,9 +272,9 @@ glamor_make_pixmap_exportable(PixmapPtr pixmap)
>      }
> 
>      if (pixmap->drawable.depth == 30)
> -       format = GBM_FORMAT_ARGB2101010;
> +        format = GBM_FORMAT_XRGB2101010;
>      else
> -        format = GBM_FORMAT_ARGB8888;
> +        format = GBM_FORMAT_XRGB8888;
> 
>  #ifdef GBM_BO_WITH_MODIFIERS
>      if (glamor_egl->dmabuf_capable) {
> @@ -442,9 +442,9 @@ glamor_back_pixmap_from_fd(PixmapPtr pixmap,
>      import_data.height = height;
>      import_data.stride = stride;
>      if (depth == 30)
> -        import_data.format = GBM_FORMAT_ARGB2101010;
> +        import_data.format = GBM_FORMAT_XRGB2101010;
>      else
> -        import_data.format = GBM_FORMAT_ARGB8888;
> +        import_data.format = GBM_FORMAT_XRGB8888;
>      bo = gbm_bo_import(glamor_egl->gbm, GBM_BO_IMPORT_FD,
> &import_data, 0);
>      if (!bo)
>          return FALSE;
> @@ -465,7 +465,7 @@ gbm_format_for_depth(CARD8 depth)
>      case 24:
>          return GBM_FORMAT_XRGB8888;
>      case 30:
> -        return GBM_FORMAT_ARGB2101010;
> +        return GBM_FORMAT_XRGB2101010;
>      default:
>          ErrorF("unexpected depth: %d\n", depth);
>      case 32:
> 
> So we need something more clever. weston does use fallback formats,
> substituting ARGB by XRGB for what it passes to the kernel, if it
> wants to do a zero-copy present of a fullscreen wayland
> surface/buffer
> via pageflipping (in compositor-drm.c
> drm_output_check_scanout_format()).
> 
> -mario
> 
> 
> 
> On Fri, Mar 23, 2018 at 9:03 AM, Olivier Fourdan <ofourdan at redhat.com
> > wrote:
> > Hi
> > 
> > On Wed, Mar 21, 2018 at 10:09 PM, Mario Kleiner <mario.kleiner.de at g
> > mail.com>
> > wrote:
> > > 
> > > Thanks. However, seems we may have one more problem with the new
> > > code
> > > on slightly older < gen 9 Intel hw. Either that, or something is
> > > botched on my system atm., after pulling in all kind of updates
> > > to
> > > xorg-proto and xcb-proto and libdrm and stuff to make the server
> > > and
> > > mesa compile again.
> > > 
> > > Using x-server/mesa master from last friday plus my patch,
> > > everything
> > > was fine when testing on radeon-kms and nouveau-kms, but when i
> > > tested
> > > on Intel IvyBridge (gen 5), i stared at a blank screen. Xorg log
> > > reported modeset failure with EINVAL iirc, the kernel at
> > > drm.debug
> > > level 1 reported:
> > > 
> > > Mar 16 19:30:48 kleinerm-MaxtorLinux kernel: [  576.235103]
> > > [drm:drm_atomic_set_mode_prop_for_crtc [drm]] Set [MODE:] for
> > > CRTC
> > > state ffff9496185e3000
> > > Mar 16 19:30:48 kleinerm-MaxtorLinux kernel: [  576.235112]
> > > [drm:drm_mode_object_put [drm]] OBJ ID: 81 (4)
> > > Mar 16 19:30:48 kleinerm-MaxtorLinux kernel: [  576.235122]
> > > [drm:drm_mode_object_put [drm]] OBJ ID: 81 (3)
> > > Mar 16 19:30:48 kleinerm-MaxtorLinux kernel: [  576.235133]
> > > [drm:drm_mode_object_get [drm]] OBJ ID: 50 (5)
> > > Mar 16 19:30:48 kleinerm-MaxtorLinux kernel: [  576.235142]
> > > [drm:drm_mode_object_get [drm]] OBJ ID: 50 (6)
> > > Mar 16 19:30:48 kleinerm-MaxtorLinux kernel: [  576.235152]
> > > [drm:drm_atomic_get_connector_state [drm]] Added
> > > [CONNECTOR:50:HDMI-A-1] ffff9495e9629c00 state to
> > > ffff94961bb27400
> > > Mar 16 19:30:48 kleinerm-MaxtorLinux kernel: [  576.235162]
> > > [drm:drm_mode_object_put [drm]] OBJ ID: 50 (7)
> > > Mar 16 19:30:48 kleinerm-MaxtorLinux kernel: [  576.235171]
> > > [drm:drm_atomic_check_only [drm]] checking ffff94961bb27400
> > > 
> > > HERE -->
> > > Mar 16 19:30:48 kleinerm-MaxtorLinux kernel: [  576.235181]
> > > [drm:drm_atomic_check_only [drm]] Invalid pixel format AR24
> > > little-endian (0x34325241)
> > > Mar 16 19:30:48 kleinerm-MaxtorLinux kernel: [  576.235189]
> > > [drm:drm_atomic_check_only [drm]] [PLANE:26:primary A] atomic
> > > core
> > > check failed
> > > Mar 16 19:30:48 kleinerm-MaxtorLinux kernel: [  576.235198]
> > > [drm:drm_atomic_state_default_clear [drm]] Clearing atomic state
> > > ffff94961bb27400
> > > 
> > > This makes sense, as looking at i915-kms, Intel hw before Skylake
> > > does
> > > not support setting ARGB pixel formats, only Skylake and later
> > > do.
> > > Iirc radeon/amdgpu/nouveau-kms treat ARGB formats like XRGB
> > > formats,
> > > so they are nominally supported, even if ARGB is simply mapped to
> > > XRGB
> > > hw programming for scanout on some hw.
> > > 
> > > In modesetting ddx, we create gbm buffers as either ARGB8888 or
> > > ARGB2101010, not checking if hw supports this. When using the new
> > > drmAddFB2WithModifiers ioctl, we choose format to be whatever the
> > > gbm
> > > buffers format is, so that might end badly. When i tried the
> > > obvious
> > > hack of switching to depth 24 -> XRGB8888 and depth 30 ->
> > > XRGB2101010
> > > i gbm buffer creation, that made modesetting work again at depth
> > > 24,
> > > but now broke pageflipping again. For depth 30 it died somewhere
> > > deep
> > > in pixman during server startup.
> > > 
> > > Looking at the current code in glamor and modesetting ddx, using
> > > XRGB8888 and XRGB2101010 consistently for depth 24 or depth 30
> > > would
> > > actually make more sense to me than ARGB8888 and ARGB2101010, but
> > > hacking that in didn't immediately improve the situation.
> > > 
> > > That was the point when i decided it is weekend.
> > > 
> > > Maybe we need some fallback mapping of gbm formats to xrgb
> > > fallback
> > > pixel formats, just for the code path of drmAddFB2WithModifiers,
> > > similar to what we do in weston for the direct scanout path?
> > > Create
> > > buffers in ARGB, but submit them as XRGB for scanout?
> > 
> > 
> > Yeap, I ended up at the same point in downstream bug
> > https://bugzilla.redhat.com/show_bug.cgi?id=1558476, yet using
> > XRGB8888
> > instead of ARGB8888 fixes the issue reliably here...
> > 
> > Cheers,
> > Olivier