[Mesa-dev] [PATCH mesa 1/6] vulkan/wsi/display: setup the connector earlier

[Keith Packard]

Eric Engestrom <eric.engestrom at intel.com> writes:

> Instead of setting it up when the swapchain is presented, set it up when
> creating the swapchain. This means that multiple swapchains might use
> the same crtc, but only one can be active at a time, and the connectors
> are now refcounted.
>
> This is necessary for the next commit.
>
> Signed-off-by: Eric Engestrom <eric.engestrom at intel.com>
> ---
>  src/vulkan/wsi/wsi_common_display.c | 39 +++++++++++++++++++++++------
>  1 file changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/src/vulkan/wsi/wsi_common_display.c b/src/vulkan/wsi/wsi_common_display.c
> index 1dbed08d8a750ce21846..2d378afe3d36fe7cc177 100644
> --- a/src/vulkan/wsi/wsi_common_display.c
> +++ b/src/vulkan/wsi/wsi_common_display.c
> @@ -20,6 +20,7 @@
>   * OF THIS SOFTWARE.
>   */
>  
> +#include "util/u_atomic.h"
>  #include "util/macros.h"
>  #include <stdlib.h>
>  #include <stdio.h>
> @@ -76,6 +77,7 @@ typedef struct wsi_display_connector {
>     char                         *name;
>     bool                         connected;
>     bool                         active;
> +   int                          refcount; /* swapchains using this connector */

Given that you're allocating the crtc at the same time you're setting
this value, could you stick the refcount in the crtc structure and avoid
a bunch of list walking?

>     for (uint32_t i = 0; i < chain->base.image_count; i++)
>        wsi_display_image_finish(drv_chain, allocator, &chain->images[i]);
> +
> +   wsi_display_mode *display_mode =
> +      wsi_display_mode_from_handle(chain->surface->displayMode);
> +   if (p_atomic_dec_zero(&display_mode->connector->refcount))
> +      display_mode->connector->crtc_id = 0;
> +

I'd suggest just sticking some huge mutexes around the allocation and
free process to make sure we don't have any races. Either that or assert
that the application needs to deal with the problem. In either case,
atomics don't seem indicated here. I fear any careful ordering of
operations will be hard to review and fragile in the face of future
changes.

Thanks for cleaning this up; it's much nicer in this form than what I
did.

-- 
-keith
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180926/817c52f5/attachment-0001.sig>