[PATCH 2/3] drm_hwcomposer: Stop using libsync to provide sw_sync wrappers

[PATCH 2/3] drm_hwcomposer: Stop using libsync to provide sw_sync wrappers

[Alistair Strachan]

Use of the sw_sync API is not allowed any more. Until drm_hwcomposer is
weaned off of sw_sync, build our own copy.

Cc: John Stultz <john.stultz@linaro.org>
Cc: Rob Herring <rob.herring@linaro.org>
Cc: Sean Paul <seanpaul@google.com>
Signed-off-by: Alistair Strachan <astrachan@google.com>
---
 Android.mk | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/Android.mk b/Android.mk
index 573c5aa..747bf27 100644
--- a/Android.mk
+++ b/Android.mk
@@ -14,15 +14,38 @@
 
 ifeq ($(strip $(BOARD_USES_DRM_HWCOMPOSER)),true)
 
-LOCAL_PATH := $(call my-dir)
+__this_dir := $(call my-dir)
+
+# =====================
+# libdrmhwc_sync.a
+# =====================
+include $(CLEAR_VARS)
+
+LOCAL_PATH := system/core/libsync
+
+LOCAL_SRC_FILES := sync.c
+
+LOCAL_CFLAGS := -Wno-unused-variable
+
+LOCAL_MODULE := libdrmhwc_sync
+
+LOCAL_VENDOR_MODULE := true
+
+LOCAL_C_INCLUDES := $(LOCAL_PATH)/include
+
+LOCAL_EXPORT_C_INCLUDE_DIRS := \
+	$(LOCAL_PATH) $(LOCAL_PATH)/include
+
+include $(BUILD_STATIC_LIBRARY)
 
 # =====================
 # libdrmhwc_utils.a
 # =====================
 include $(CLEAR_VARS)
 
-LOCAL_SRC_FILES := \
-	worker.cpp
+LOCAL_PATH := $(__this_dir)
+
+LOCAL_SRC_FILES := worker.cpp
 
 LOCAL_MODULE := libdrmhwc_utils
 LOCAL_VENDOR_MODULE := true
@@ -34,6 +57,8 @@ include $(BUILD_STATIC_LIBRARY)
 # =====================
 include $(CLEAR_VARS)
 
+LOCAL_PATH := $(__this_dir)
+
 LOCAL_SHARED_LIBRARIES := \
 	libcutils \
 	libdrm \
@@ -41,14 +66,10 @@ LOCAL_SHARED_LIBRARIES := \
 	libGLESv2 \
 	libhardware \
 	liblog \
-	libsync \
 	libui \
 	libutils
 
-LOCAL_STATIC_LIBRARIES := libdrmhwc_utils
-
-LOCAL_C_INCLUDES := \
-	system/core/libsync
+LOCAL_STATIC_LIBRARIES := libdrmhwc_utils libdrmhwc_sync
 
 LOCAL_SRC_FILES := \
 	autolock.cpp \
-- 
2.17.0.441.gb46fe60e1d-goog

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm_hwcomposer: Stop using libsync to provide sw_sync wrappers

[John Stultz]

On Wed, May 2, 2018 at 4:56 PM, Alistair Strachan <astrachan@google.com> wrote:
> Use of the sw_sync API is not allowed any more. Until drm_hwcomposer is
> weaned off of sw_sync, build our own copy.
>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Rob Herring <rob.herring@linaro.org>
> Cc: Sean Paul <seanpaul@google.com>
> Signed-off-by: Alistair Strachan <astrachan@google.com>

This patch is particularly important for building drm_hwc with Treble enabled.

Acked-by: John Stultz <john.stultz@linaro.org>

thanks
-john
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm_hwcomposer: Stop using libsync to provide sw_sync wrappers

[Sean Paul]

On Wed, May 02, 2018 at 04:56:29PM -0700, Alistair Strachan wrote:
> Use of the sw_sync API is not allowed any more. Until drm_hwcomposer is
> weaned off of sw_sync, build our own copy.

I don't think it is used any longer. AFAICT, with 2 seconds of grep, it's
referenced in drmcompositorworker.cpp, virtualcompositorworker.cpp, and
hwcomposer.cpp

I think these are all HWC1 legacy files, so they can probably just get cleaned
up.

Sean

> 
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Rob Herring <rob.herring@linaro.org>
> Cc: Sean Paul <seanpaul@google.com>
> Signed-off-by: Alistair Strachan <astrachan@google.com>
> ---
>  Android.mk | 37 +++++++++++++++++++++++++++++--------
>  1 file changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/Android.mk b/Android.mk
> index 573c5aa..747bf27 100644
> --- a/Android.mk
> +++ b/Android.mk
> @@ -14,15 +14,38 @@
>  
>  ifeq ($(strip $(BOARD_USES_DRM_HWCOMPOSER)),true)
>  
> -LOCAL_PATH := $(call my-dir)
> +__this_dir := $(call my-dir)
> +
> +# =====================
> +# libdrmhwc_sync.a
> +# =====================
> +include $(CLEAR_VARS)
> +
> +LOCAL_PATH := system/core/libsync
> +
> +LOCAL_SRC_FILES := sync.c
> +
> +LOCAL_CFLAGS := -Wno-unused-variable
> +
> +LOCAL_MODULE := libdrmhwc_sync
> +
> +LOCAL_VENDOR_MODULE := true
> +
> +LOCAL_C_INCLUDES := $(LOCAL_PATH)/include
> +
> +LOCAL_EXPORT_C_INCLUDE_DIRS := \
> +	$(LOCAL_PATH) $(LOCAL_PATH)/include
> +
> +include $(BUILD_STATIC_LIBRARY)
>  
>  # =====================
>  # libdrmhwc_utils.a
>  # =====================
>  include $(CLEAR_VARS)
>  
> -LOCAL_SRC_FILES := \
> -	worker.cpp
> +LOCAL_PATH := $(__this_dir)
> +
> +LOCAL_SRC_FILES := worker.cpp
>  
>  LOCAL_MODULE := libdrmhwc_utils
>  LOCAL_VENDOR_MODULE := true
> @@ -34,6 +57,8 @@ include $(BUILD_STATIC_LIBRARY)
>  # =====================
>  include $(CLEAR_VARS)
>  
> +LOCAL_PATH := $(__this_dir)
> +
>  LOCAL_SHARED_LIBRARIES := \
>  	libcutils \
>  	libdrm \
> @@ -41,14 +66,10 @@ LOCAL_SHARED_LIBRARIES := \
>  	libGLESv2 \
>  	libhardware \
>  	liblog \
> -	libsync \
>  	libui \
>  	libutils
>  
> -LOCAL_STATIC_LIBRARIES := libdrmhwc_utils
> -
> -LOCAL_C_INCLUDES := \
> -	system/core/libsync
> +LOCAL_STATIC_LIBRARIES := libdrmhwc_utils libdrmhwc_sync
>  
>  LOCAL_SRC_FILES := \
>  	autolock.cpp \
> -- 
> 2.17.0.441.gb46fe60e1d-goog
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm_hwcomposer: Stop using libsync to provide sw_sync wrappers

[Rob Herring]

On Thu, May 3, 2018 at 8:47 AM, Sean Paul <seanpaul@chromium.org> wrote:
> On Wed, May 02, 2018 at 04:56:29PM -0700, Alistair Strachan wrote:
>> Use of the sw_sync API is not allowed any more. Until drm_hwcomposer is
>> weaned off of sw_sync, build our own copy.
>
> I don't think it is used any longer. AFAICT, with 2 seconds of grep, it's
> referenced in drmcompositorworker.cpp, virtualcompositorworker.cpp, and
> hwcomposer.cpp
>
> I think these are all HWC1 legacy files, so they can probably just get cleaned
> up.

Are you sure? Doesn't the GL compositor have to wait on the fences of
the input buffers and create new fences for the GL compositing output
buffer(s) to pass into the kernel? It looked to me like sw_sync fences
are used in the latter case and that needs to be converted to use
eglCreateSyncKHR.

Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm_hwcomposer: Stop using libsync to provide sw_sync wrappers

[Sean Paul]

On Thu, May 03, 2018 at 09:14:41AM -0500, Rob Herring wrote:
> On Thu, May 3, 2018 at 8:47 AM, Sean Paul <seanpaul@chromium.org> wrote:
> > On Wed, May 02, 2018 at 04:56:29PM -0700, Alistair Strachan wrote:
> >> Use of the sw_sync API is not allowed any more. Until drm_hwcomposer is
> >> weaned off of sw_sync, build our own copy.
> >
> > I don't think it is used any longer. AFAICT, with 2 seconds of grep, it's
> > referenced in drmcompositorworker.cpp, virtualcompositorworker.cpp, and
> > hwcomposer.cpp
> >
> > I think these are all HWC1 legacy files, so they can probably just get cleaned
> > up.
> 
> Are you sure? Doesn't the GL compositor have to wait on the fences of
> the input buffers and create new fences for the GL compositing output
> buffer(s) to pass into the kernel? It looked to me like sw_sync fences
> are used in the latter case and that needs to be converted to use
> eglCreateSyncKHR.

Hmmm, yeah thanks for pointing that out. It does look like drmdisplaycomposition
is using sw_sync to handle the GL compositor release fences.

I'm pretty confident that despite the known issues causing GLC to not work on
open stacks, there are probably a bunch of unknown issues that have been
introduced while we try to tiptoe around it.

If anyone wants to remove it, I'd certainly be interested in reviewing that
patch :-). I'd do it myself, but unfortunately the amount of time I have
to dedicate to drm_hwc at this point is just enough to perform [incorrect]
reviews.

Sean


> 
> Rob

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm_hwcomposer: Stop using libsync to provide sw_sync wrappers

[Rob Herring]

On Thu, May 3, 2018 at 9:32 AM, Sean Paul <seanpaul@chromium.org> wrote:
> On Thu, May 03, 2018 at 09:14:41AM -0500, Rob Herring wrote:
>> On Thu, May 3, 2018 at 8:47 AM, Sean Paul <seanpaul@chromium.org> wrote:
>> > On Wed, May 02, 2018 at 04:56:29PM -0700, Alistair Strachan wrote:
>> >> Use of the sw_sync API is not allowed any more. Until drm_hwcomposer is
>> >> weaned off of sw_sync, build our own copy.
>> >
>> > I don't think it is used any longer. AFAICT, with 2 seconds of grep, it's
>> > referenced in drmcompositorworker.cpp, virtualcompositorworker.cpp, and
>> > hwcomposer.cpp
>> >
>> > I think these are all HWC1 legacy files, so they can probably just get cleaned
>> > up.
>>
>> Are you sure? Doesn't the GL compositor have to wait on the fences of
>> the input buffers and create new fences for the GL compositing output
>> buffer(s) to pass into the kernel? It looked to me like sw_sync fences
>> are used in the latter case and that needs to be converted to use
>> eglCreateSyncKHR.
>
> Hmmm, yeah thanks for pointing that out. It does look like drmdisplaycomposition
> is using sw_sync to handle the GL compositor release fences.
>
> I'm pretty confident that despite the known issues causing GLC to not work on
> open stacks, there are probably a bunch of unknown issues that have been
> introduced while we try to tiptoe around it.
>
> If anyone wants to remove it, I'd certainly be interested in reviewing that
> patch :-). I'd do it myself, but unfortunately the amount of time I have
> to dedicate to drm_hwc at this point is just enough to perform [incorrect]
> reviews.

Removing GL compositor or sw_sync? I've already written a patch to do
the former.

Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm_hwcomposer: Stop using libsync to provide sw_sync wrappers

[Sean Paul]

On Thu, May 03, 2018 at 09:39:31AM -0500, Rob Herring wrote:
> On Thu, May 3, 2018 at 9:32 AM, Sean Paul <seanpaul@chromium.org> wrote:
> > On Thu, May 03, 2018 at 09:14:41AM -0500, Rob Herring wrote:
> >> On Thu, May 3, 2018 at 8:47 AM, Sean Paul <seanpaul@chromium.org> wrote:
> >> > On Wed, May 02, 2018 at 04:56:29PM -0700, Alistair Strachan wrote:
> >> >> Use of the sw_sync API is not allowed any more. Until drm_hwcomposer is
> >> >> weaned off of sw_sync, build our own copy.
> >> >
> >> > I don't think it is used any longer. AFAICT, with 2 seconds of grep, it's
> >> > referenced in drmcompositorworker.cpp, virtualcompositorworker.cpp, and
> >> > hwcomposer.cpp
> >> >
> >> > I think these are all HWC1 legacy files, so they can probably just get cleaned
> >> > up.
> >>
> >> Are you sure? Doesn't the GL compositor have to wait on the fences of
> >> the input buffers and create new fences for the GL compositing output
> >> buffer(s) to pass into the kernel? It looked to me like sw_sync fences
> >> are used in the latter case and that needs to be converted to use
> >> eglCreateSyncKHR.
> >
> > Hmmm, yeah thanks for pointing that out. It does look like drmdisplaycomposition
> > is using sw_sync to handle the GL compositor release fences.
> >
> > I'm pretty confident that despite the known issues causing GLC to not work on
> > open stacks, there are probably a bunch of unknown issues that have been
> > introduced while we try to tiptoe around it.
> >
> > If anyone wants to remove it, I'd certainly be interested in reviewing that
> > patch :-). I'd do it myself, but unfortunately the amount of time I have
> > to dedicate to drm_hwc at this point is just enough to perform [incorrect]
> > reviews.
> 
> Removing GL compositor or sw_sync? I've already written a patch to do
> the former.

Removing GL compositor. Is your patch on the list?

Sean

> 
> Rob

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm_hwcomposer: Stop using libsync to provide sw_sync wrappers

[Rob Herring]

On Thu, May 3, 2018 at 9:45 AM, Sean Paul <seanpaul@chromium.org> wrote:
> On Thu, May 03, 2018 at 09:39:31AM -0500, Rob Herring wrote:
>> On Thu, May 3, 2018 at 9:32 AM, Sean Paul <seanpaul@chromium.org> wrote:
>> > On Thu, May 03, 2018 at 09:14:41AM -0500, Rob Herring wrote:
>> >> On Thu, May 3, 2018 at 8:47 AM, Sean Paul <seanpaul@chromium.org> wrote:
>> >> > On Wed, May 02, 2018 at 04:56:29PM -0700, Alistair Strachan wrote:
>> >> >> Use of the sw_sync API is not allowed any more. Until drm_hwcomposer is
>> >> >> weaned off of sw_sync, build our own copy.
>> >> >
>> >> > I don't think it is used any longer. AFAICT, with 2 seconds of grep, it's
>> >> > referenced in drmcompositorworker.cpp, virtualcompositorworker.cpp, and
>> >> > hwcomposer.cpp
>> >> >
>> >> > I think these are all HWC1 legacy files, so they can probably just get cleaned
>> >> > up.
>> >>
>> >> Are you sure? Doesn't the GL compositor have to wait on the fences of
>> >> the input buffers and create new fences for the GL compositing output
>> >> buffer(s) to pass into the kernel? It looked to me like sw_sync fences
>> >> are used in the latter case and that needs to be converted to use
>> >> eglCreateSyncKHR.
>> >
>> > Hmmm, yeah thanks for pointing that out. It does look like drmdisplaycomposition
>> > is using sw_sync to handle the GL compositor release fences.
>> >
>> > I'm pretty confident that despite the known issues causing GLC to not work on
>> > open stacks, there are probably a bunch of unknown issues that have been
>> > introduced while we try to tiptoe around it.
>> >
>> > If anyone wants to remove it, I'd certainly be interested in reviewing that
>> > patch :-). I'd do it myself, but unfortunately the amount of time I have
>> > to dedicate to drm_hwc at this point is just enough to perform [incorrect]
>> > reviews.
>>
>> Removing GL compositor or sw_sync? I've already written a patch to do
>> the former.
>
> Removing GL compositor. Is your patch on the list?

No, I never got around to discussing it. I somewhat figured you would
not be in favor of that.

I'll need to rebase it as it's about 3 months old now.

Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm_hwcomposer: Stop using libsync to provide sw_sync wrappers

[Sean Paul]

On Thu, May 03, 2018 at 10:30:35AM -0500, Rob Herring wrote:
> On Thu, May 3, 2018 at 9:45 AM, Sean Paul <seanpaul@chromium.org> wrote:
> > On Thu, May 03, 2018 at 09:39:31AM -0500, Rob Herring wrote:
> >> On Thu, May 3, 2018 at 9:32 AM, Sean Paul <seanpaul@chromium.org> wrote:
> >> > On Thu, May 03, 2018 at 09:14:41AM -0500, Rob Herring wrote:
> >> >> On Thu, May 3, 2018 at 8:47 AM, Sean Paul <seanpaul@chromium.org> wrote:
> >> >> > On Wed, May 02, 2018 at 04:56:29PM -0700, Alistair Strachan wrote:
> >> >> >> Use of the sw_sync API is not allowed any more. Until drm_hwcomposer is
> >> >> >> weaned off of sw_sync, build our own copy.
> >> >> >
> >> >> > I don't think it is used any longer. AFAICT, with 2 seconds of grep, it's
> >> >> > referenced in drmcompositorworker.cpp, virtualcompositorworker.cpp, and
> >> >> > hwcomposer.cpp
> >> >> >
> >> >> > I think these are all HWC1 legacy files, so they can probably just get cleaned
> >> >> > up.
> >> >>
> >> >> Are you sure? Doesn't the GL compositor have to wait on the fences of
> >> >> the input buffers and create new fences for the GL compositing output
> >> >> buffer(s) to pass into the kernel? It looked to me like sw_sync fences
> >> >> are used in the latter case and that needs to be converted to use
> >> >> eglCreateSyncKHR.
> >> >
> >> > Hmmm, yeah thanks for pointing that out. It does look like drmdisplaycomposition
> >> > is using sw_sync to handle the GL compositor release fences.
> >> >
> >> > I'm pretty confident that despite the known issues causing GLC to not work on
> >> > open stacks, there are probably a bunch of unknown issues that have been
> >> > introduced while we try to tiptoe around it.
> >> >
> >> > If anyone wants to remove it, I'd certainly be interested in reviewing that
> >> > patch :-). I'd do it myself, but unfortunately the amount of time I have
> >> > to dedicate to drm_hwc at this point is just enough to perform [incorrect]
> >> > reviews.
> >>
> >> Removing GL compositor or sw_sync? I've already written a patch to do
> >> the former.
> >
> > Removing GL compositor. Is your patch on the list?
> 
> No, I never got around to discussing it. I somewhat figured you would
> not be in favor of that.
> 

The writeback series definitely helped sway my opinion. I would really like to
have our own super-spiffy GL compositor, since it was a net gain for us.
However, in its current state, it's just bringing us down.

> I'll need to rebase it as it's about 3 months old now.

Cool! I'm relieved I didn't miss it :-)

Sean

> 
> Rob

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel