Re: [Mesa-dev] [PATCH 04/14] intel: Add simple logging façade for Android

2017-10-09 Thread Dylan Baker
Quoting Chad Versace (2017-09-27 17:11:50)
> I'm bringing up Vulkan in the Android container of Chrome OS (ARC++).
> 
> On Android, stdio goes to /dev/null. On Android, remote gdb is even more
> painful than the usual remote gdb. On Android, nothing works like you
> expect and debugging is hell. I need logging.
> 
> This patch introduces a small, simple logging API that can easily wrap
> Android's API. On non-Android platforms, this logger does nothing fancy.
> It follows the time-honored Unix tradition of spewing everything to
> stderr with minimal fuss.
> 
> My goal here is not perfection. My goal is to make a minimal, clean API,
> that people hate merely a little instead of a lot, and that's good
> enough to let me bring up Android Vulkan.  And it needs to be fast,
> which means it must be small. No one wants to their game to miss frames
> while aiming a flaming bow into the jaws of an angry robot t-rex, and
> thus become t-rex breakfast, because some fool had too much fun desiging
> a bloated, ideal logging API.
> 
> If people like it, perhaps we should quickly promote it to src/util.
> 
> The API looks like this:
> 
> #define INTEL_LOG_TAG "intel-vulkan"
> #define DEBUG
> 
> intel_logd("try hard thing with foo=%d", foo);
> 
> n = try_foo(...);
> if (n < 0) {
> intel_loge("%s:%d: foo failed bigtime", __FILE__, __LINE__);
> return VK_ERROR_DEVICE_LOST;
> }
> 
> And produces this on non-Android:
> 
> intel-vulkan: debug: try hard thing with foo=93
> intel-vulkan: error: anv_device.c:182: foo failed bigtime
> ---
>  src/intel/Makefile.sources   |  4 +-
>  src/intel/common/intel_log.c | 87 
> 
>  src/intel/common/intel_log.h | 82 +
>  3 files changed, 172 insertions(+), 1 deletion(-)
>  create mode 100644 src/intel/common/intel_log.c
>  create mode 100644 src/intel/common/intel_log.h
> 
> diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
> index bca7a132b26..64a2dfe10e7 100644
> --- a/src/intel/Makefile.sources
> +++ b/src/intel/Makefile.sources
> @@ -18,7 +18,9 @@ COMMON_FILES = \
> common/gen_l3_config.c \
> common/gen_l3_config.h \
> common/gen_urb_config.c \
> -   common/gen_sample_positions.h
> +   common/gen_sample_positions.h \
> +   common/intel_log.c \
> +   common/intel_log.h

I hate to do this, but this is going to break meson. Could you add these two
files to src/intel/common/meson.build? I'm not sure but the previous patch
changes may be needed as well (I haven't gotten around to getting android hooked
up in meson, so it may not be needed. Maybe I can convince krh to do that ;)

Dylan


signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 04/14] intel: Add simple logging façade for Android

2017-09-27 Thread Chad Versace
I'm bringing up Vulkan in the Android container of Chrome OS (ARC++).

On Android, stdio goes to /dev/null. On Android, remote gdb is even more
painful than the usual remote gdb. On Android, nothing works like you
expect and debugging is hell. I need logging.

This patch introduces a small, simple logging API that can easily wrap
Android's API. On non-Android platforms, this logger does nothing fancy.
It follows the time-honored Unix tradition of spewing everything to
stderr with minimal fuss.

My goal here is not perfection. My goal is to make a minimal, clean API,
that people hate merely a little instead of a lot, and that's good
enough to let me bring up Android Vulkan.  And it needs to be fast,
which means it must be small. No one wants to their game to miss frames
while aiming a flaming bow into the jaws of an angry robot t-rex, and
thus become t-rex breakfast, because some fool had too much fun desiging
a bloated, ideal logging API.

If people like it, perhaps we should quickly promote it to src/util.

The API looks like this:

#define INTEL_LOG_TAG "intel-vulkan"
#define DEBUG

intel_logd("try hard thing with foo=%d", foo);

n = try_foo(...);
if (n < 0) {
intel_loge("%s:%d: foo failed bigtime", __FILE__, __LINE__);
return VK_ERROR_DEVICE_LOST;
}

And produces this on non-Android:

intel-vulkan: debug: try hard thing with foo=93
intel-vulkan: error: anv_device.c:182: foo failed bigtime
---
 src/intel/Makefile.sources   |  4 +-
 src/intel/common/intel_log.c | 87 
 src/intel/common/intel_log.h | 82 +
 3 files changed, 172 insertions(+), 1 deletion(-)
 create mode 100644 src/intel/common/intel_log.c
 create mode 100644 src/intel/common/intel_log.h

diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
index bca7a132b26..64a2dfe10e7 100644
--- a/src/intel/Makefile.sources
+++ b/src/intel/Makefile.sources
@@ -18,7 +18,9 @@ COMMON_FILES = \
common/gen_l3_config.c \
common/gen_l3_config.h \
common/gen_urb_config.c \
-   common/gen_sample_positions.h
+   common/gen_sample_positions.h \
+   common/intel_log.c \
+   common/intel_log.h
 
 COMPILER_FILES = \
compiler/brw_cfg.cpp \
diff --git a/src/intel/common/intel_log.c b/src/intel/common/intel_log.c
new file mode 100644
index 000..cebdd6dd6d8
--- /dev/null
+++ b/src/intel/common/intel_log.c
@@ -0,0 +1,87 @@
+/*
+ * Copyright © 2017 Google, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include 
+
+#ifdef ANDROID
+#include 
+#else
+#include 
+#endif
+
+#include "intel_log.h"
+
+#ifdef ANDROID
+static inline android_LogPriority
+level_to_android(enum intel_log_level l)
+{
+   switch (l) {
+   case INTEL_LOG_ERROR: return ANDROID_LOG_ERROR;
+   case INTEL_LOG_WARN: return ANDROID_LOG_WARN;
+   case INTEL_LOG_INFO: return ANDROID_LOG_INFO;
+   case INTEL_LOG_DEBUG: return ANDROID_LOG_DEBUG;
+   }
+
+   unreachable("bad intel_log_level");
+}
+#endif
+
+#ifndef ANDROID
+static inline const char *
+level_to_str(enum intel_log_level l)
+{
+   switch (l) {
+   case INTEL_LOG_ERROR: return "error";
+   case INTEL_LOG_WARN: return "warning";
+   case INTEL_LOG_INFO: return "info";
+   case INTEL_LOG_DEBUG: return "debug";
+   }
+
+   unreachable("bad intel_log_level");
+}
+#endif
+
+void
+intel_log(enum intel_log_level level, const char *tag, const char *format, ...)
+{
+   va_list va;
+
+   va_start(va, format);
+   intel_log_v(level, tag, format, va);
+   va_end(va);
+}
+
+void
+intel_log_v(enum intel_log_level level, const char *tag, const char *format,
+va_list va)
+{
+#ifdef ANDROID
+   __android_log_vprint(level_to_android(level), tag, format, va);
+#else
+   flockfile(stderr);
+   fprintf(stderr, "%s: %s: ", tag, level_to_str(level));
+   vfprintf(stderr, format, va);
+