| Message ID | 20180502235629.105155-1-astrachan@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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
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
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
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
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
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
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 \
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(-)