Re: [Intel-gfx] [PATCH 3/3] drm/i915/guc: revisit GuC loader message levels
On 12/07/16 10:26, Tvrtko Ursulin wrote:
On 11/07/16 19:01, Dave Gordon wrote:
Some downgraded from DRM_ERROR() to DRM_WARN(), some eliminated,
and a few upgraded from DRM_INFO() to DRM_NOTE() or DRM_WARN().
Signed-off-by: Dave Gordon
---
drivers/gpu/drm/i915/intel_guc_loader.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 605c696..fd032eb 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -140,12 +140,14 @@ static u32 get_gttype(struct drm_i915_private
*dev_priv)
static u32 get_core_family(struct drm_i915_private *dev_priv)
{
-switch (INTEL_INFO(dev_priv)->gen) {
+u32 gen = INTEL_GEN(dev_priv);
+
+switch (gen) {
case 9:
return GFXCORE_FAMILY_GEN9;
default:
-DRM_ERROR("GUC: unsupported core family\n");
+DRM_WARN("GEN%d does not support GuC operation\n", gen);
return GFXCORE_FAMILY_UNKNOWN;
}
}
@@ -433,7 +435,7 @@ int intel_guc_setup(struct drm_device *dev)
goto fail;
} else if (*fw_path == '\0') {
/* Device has a GuC but we don't know what f/w to load? */
-DRM_INFO("No GuC firmware known for this platform\n");
+DRM_WARN("No GuC firmware known for this platform\n");
err = -ENODEV;
goto fail;
}
@@ -471,10 +473,8 @@ int intel_guc_setup(struct drm_device *dev)
* that the state and timing are fairly predictable
*/
err = i915_reset_guc(dev_priv);
-if (err) {
-DRM_ERROR("GuC reset failed: %d\n", err);
+if (err)
goto fail;
-}
err = guc_ucode_xfer(dev_priv);
if (!err)
@@ -532,15 +532,15 @@ int intel_guc_setup(struct drm_device *dev)
else if (err == 0)
DRM_INFO("GuC firmware load skipped\n");
else if (ret != -EIO)
-DRM_INFO("GuC firmware load failed: %d\n", err);
+DRM_NOTE("GuC firmware load failed: %d\n", err);
else
-DRM_ERROR("GuC firmware load failed: %d\n", err);
+DRM_WARN("GuC firmware load failed: %d\n", err);
if (i915.enable_guc_submission) {
if (fw_path == NULL)
DRM_INFO("GuC submission without firmware not
supported\n");
if (ret == 0)
-DRM_INFO("Falling back from GuC submission to execlist
mode\n");
+DRM_NOTE("Falling back from GuC submission to execlist
mode\n");
else
DRM_ERROR("GuC init failed: %d\n", ret);
}
@@ -656,7 +656,7 @@ static void guc_fw_fetch(struct drm_device *dev,
struct intel_guc_fw *guc_fw)
fail:
DRM_DEBUG_DRIVER("GuC fw fetch status FAIL; err %d, fw %p, obj
%p\n",
err, fw, guc_fw->guc_fw_obj);
-DRM_ERROR("Failed to fetch GuC firmware from %s (error %d)\n",
+DRM_WARN("Failed to fetch GuC firmware from %s (error %d)\n",
guc_fw->guc_fw_path, err);
mutex_lock(&dev->struct_mutex);
R-b if you also change all the other DRM_ERRORs in guc_fw_fetch to
DRM_DEBUG_DRIVER and merge this last two log lines (DRM_DEBUG_DRIVER +
DRM_WARN) to one. :)
Regards,
Tvrtko
No, that wouldn't be appropriate. We want the user to be informed if any
of these failures occurs, because it means their system is in some way
misconfigured e.g. corrupted firmware file. That's definitely not a
DEBUG-only event, and it must be logged even if we're going to try to
continue in fallback mode.
I could change all the earlier ERRORs to NOTEs and leave just the last
one as an ERROR i.e. explanation first, consequence after.
As for the DEBUG, that's for a different purpose. Whereas the various
ERROR/NOTE/INFO messages relate to the existence, format, or content of
the required firmware file in the filesystem or ramdisk, the DEBUG is
about internal failures such as not being able to allocate memory, over
which the user/administrator has no direct control.
I might swap them round though (i.e. DEBUG after the ERROR, to explain
further than I want to in a user-facing message).
.Dave.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915/guc: revisit GuC loader message levels
On 11/07/16 19:01, Dave Gordon wrote:
Some downgraded from DRM_ERROR() to DRM_WARN(), some eliminated,
and a few upgraded from DRM_INFO() to DRM_NOTE() or DRM_WARN().
Signed-off-by: Dave Gordon
---
drivers/gpu/drm/i915/intel_guc_loader.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 605c696..fd032eb 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -140,12 +140,14 @@ static u32 get_gttype(struct drm_i915_private *dev_priv)
static u32 get_core_family(struct drm_i915_private *dev_priv)
{
- switch (INTEL_INFO(dev_priv)->gen) {
+ u32 gen = INTEL_GEN(dev_priv);
+
+ switch (gen) {
case 9:
return GFXCORE_FAMILY_GEN9;
default:
- DRM_ERROR("GUC: unsupported core family\n");
+ DRM_WARN("GEN%d does not support GuC operation\n", gen);
return GFXCORE_FAMILY_UNKNOWN;
}
}
@@ -433,7 +435,7 @@ int intel_guc_setup(struct drm_device *dev)
goto fail;
} else if (*fw_path == '\0') {
/* Device has a GuC but we don't know what f/w to load? */
- DRM_INFO("No GuC firmware known for this platform\n");
+ DRM_WARN("No GuC firmware known for this platform\n");
err = -ENODEV;
goto fail;
}
@@ -471,10 +473,8 @@ int intel_guc_setup(struct drm_device *dev)
* that the state and timing are fairly predictable
*/
err = i915_reset_guc(dev_priv);
- if (err) {
- DRM_ERROR("GuC reset failed: %d\n", err);
+ if (err)
goto fail;
- }
err = guc_ucode_xfer(dev_priv);
if (!err)
@@ -532,15 +532,15 @@ int intel_guc_setup(struct drm_device *dev)
else if (err == 0)
DRM_INFO("GuC firmware load skipped\n");
else if (ret != -EIO)
- DRM_INFO("GuC firmware load failed: %d\n", err);
+ DRM_NOTE("GuC firmware load failed: %d\n", err);
else
- DRM_ERROR("GuC firmware load failed: %d\n", err);
+ DRM_WARN("GuC firmware load failed: %d\n", err);
if (i915.enable_guc_submission) {
if (fw_path == NULL)
DRM_INFO("GuC submission without firmware not
supported\n");
if (ret == 0)
- DRM_INFO("Falling back from GuC submission to execlist
mode\n");
+ DRM_NOTE("Falling back from GuC submission to execlist
mode\n");
else
DRM_ERROR("GuC init failed: %d\n", ret);
}
@@ -656,7 +656,7 @@ static void guc_fw_fetch(struct drm_device *dev, struct
intel_guc_fw *guc_fw)
fail:
DRM_DEBUG_DRIVER("GuC fw fetch status FAIL; err %d, fw %p, obj %p\n",
err, fw, guc_fw->guc_fw_obj);
- DRM_ERROR("Failed to fetch GuC firmware from %s (error %d)\n",
+ DRM_WARN("Failed to fetch GuC firmware from %s (error %d)\n",
guc_fw->guc_fw_path, err);
mutex_lock(&dev->struct_mutex);
R-b if you also change all the other DRM_ERRORs in guc_fw_fetch to
DRM_DEBUG_DRIVER and merge this last two log lines (DRM_DEBUG_DRIVER +
DRM_WARN) to one. :)
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915/guc: revisit GuC loader message levels
Some downgraded from DRM_ERROR() to DRM_WARN(), some eliminated,
and a few upgraded from DRM_INFO() to DRM_NOTE() or DRM_WARN().
Signed-off-by: Dave Gordon
---
drivers/gpu/drm/i915/intel_guc_loader.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 605c696..fd032eb 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -140,12 +140,14 @@ static u32 get_gttype(struct drm_i915_private *dev_priv)
static u32 get_core_family(struct drm_i915_private *dev_priv)
{
- switch (INTEL_INFO(dev_priv)->gen) {
+ u32 gen = INTEL_GEN(dev_priv);
+
+ switch (gen) {
case 9:
return GFXCORE_FAMILY_GEN9;
default:
- DRM_ERROR("GUC: unsupported core family\n");
+ DRM_WARN("GEN%d does not support GuC operation\n", gen);
return GFXCORE_FAMILY_UNKNOWN;
}
}
@@ -433,7 +435,7 @@ int intel_guc_setup(struct drm_device *dev)
goto fail;
} else if (*fw_path == '\0') {
/* Device has a GuC but we don't know what f/w to load? */
- DRM_INFO("No GuC firmware known for this platform\n");
+ DRM_WARN("No GuC firmware known for this platform\n");
err = -ENODEV;
goto fail;
}
@@ -471,10 +473,8 @@ int intel_guc_setup(struct drm_device *dev)
* that the state and timing are fairly predictable
*/
err = i915_reset_guc(dev_priv);
- if (err) {
- DRM_ERROR("GuC reset failed: %d\n", err);
+ if (err)
goto fail;
- }
err = guc_ucode_xfer(dev_priv);
if (!err)
@@ -532,15 +532,15 @@ int intel_guc_setup(struct drm_device *dev)
else if (err == 0)
DRM_INFO("GuC firmware load skipped\n");
else if (ret != -EIO)
- DRM_INFO("GuC firmware load failed: %d\n", err);
+ DRM_NOTE("GuC firmware load failed: %d\n", err);
else
- DRM_ERROR("GuC firmware load failed: %d\n", err);
+ DRM_WARN("GuC firmware load failed: %d\n", err);
if (i915.enable_guc_submission) {
if (fw_path == NULL)
DRM_INFO("GuC submission without firmware not
supported\n");
if (ret == 0)
- DRM_INFO("Falling back from GuC submission to execlist
mode\n");
+ DRM_NOTE("Falling back from GuC submission to execlist
mode\n");
else
DRM_ERROR("GuC init failed: %d\n", ret);
}
@@ -656,7 +656,7 @@ static void guc_fw_fetch(struct drm_device *dev, struct
intel_guc_fw *guc_fw)
fail:
DRM_DEBUG_DRIVER("GuC fw fetch status FAIL; err %d, fw %p, obj %p\n",
err, fw, guc_fw->guc_fw_obj);
- DRM_ERROR("Failed to fetch GuC firmware from %s (error %d)\n",
+ DRM_WARN("Failed to fetch GuC firmware from %s (error %d)\n",
guc_fw->guc_fw_path, err);
mutex_lock(&dev->struct_mutex);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
