Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 86aa8247 authored by Michał Winiarski's avatar Michał Winiarski Committed by Chris Wilson
Browse files

drm/i915/guc: Tidy guc_log_control



We plan to decouple log runtime (mapping + relay) from verbosity control.
Let's tidy the code now to reduce the churn in the following patches.

v2: Tidy macros, keep debug messages, use helper var for enable,
    correct typo (Michał)
    Fix incorrect input validaction (Sagar)

Signed-off-by: default avatarMichał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: default avatarSagar Arun Kamble <sagar.a.kamble@intel.com>
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20180308154707.21716-1-michal.winiarski@intel.com
parent df947168
Loading
Loading
Loading
Loading
+4 −7
Original line number Original line Diff line number Diff line
@@ -2499,13 +2499,10 @@ static int i915_guc_log_control_get(void *data, u64 *val)
{
{
	struct drm_i915_private *dev_priv = data;
	struct drm_i915_private *dev_priv = data;


	if (!HAS_GUC(dev_priv))
	if (!USES_GUC(dev_priv))
		return -ENODEV;
		return -ENODEV;


	if (!dev_priv->guc.log.vma)
	*val = intel_guc_log_control_get(&dev_priv->guc);
		return -EINVAL;

	*val = i915_modparams.guc_log_level;


	return 0;
	return 0;
}
}
@@ -2514,10 +2511,10 @@ static int i915_guc_log_control_set(void *data, u64 val)
{
{
	struct drm_i915_private *dev_priv = data;
	struct drm_i915_private *dev_priv = data;


	if (!HAS_GUC(dev_priv))
	if (!USES_GUC(dev_priv))
		return -ENODEV;
		return -ENODEV;


	return intel_guc_log_control(&dev_priv->guc, val);
	return intel_guc_log_control_set(&dev_priv->guc, val);
}
}


DEFINE_SIMPLE_ATTRIBUTE(i915_guc_log_control_fops,
DEFINE_SIMPLE_ATTRIBUTE(i915_guc_log_control_fops,
+47 −33
Original line number Original line Diff line number Diff line
@@ -659,51 +659,63 @@ void intel_guc_log_destroy(struct intel_guc *guc)
	i915_vma_unpin_and_release(&guc->log.vma);
	i915_vma_unpin_and_release(&guc->log.vma);
}
}


int intel_guc_log_control(struct intel_guc *guc, u64 control_val)
int intel_guc_log_control_get(struct intel_guc *guc)
{
	GEM_BUG_ON(!guc->log.vma);
	GEM_BUG_ON(i915_modparams.guc_log_level < 0);

	return i915_modparams.guc_log_level;
}

#define GUC_LOG_LEVEL_DISABLED		0
#define LOG_LEVEL_TO_ENABLED(x)		((x) > 0)
#define LOG_LEVEL_TO_VERBOSITY(x) ({		\
	typeof(x) _x = (x);			\
	LOG_LEVEL_TO_ENABLED(_x) ? _x - 1 : 0;	\
})
#define VERBOSITY_TO_LOG_LEVEL(x)  ((x) + 1)
int intel_guc_log_control_set(struct intel_guc *guc, u64 val)
{
{
	struct drm_i915_private *dev_priv = guc_to_i915(guc);
	struct drm_i915_private *dev_priv = guc_to_i915(guc);
	bool enable_logging = control_val > 0;
	bool enabled = LOG_LEVEL_TO_ENABLED(val);
	u32 verbosity;
	int ret;
	int ret;


	if (!guc->log.vma)
	BUILD_BUG_ON(GUC_LOG_VERBOSITY_MIN != 0);
		return -ENODEV;
	GEM_BUG_ON(!guc->log.vma);
	GEM_BUG_ON(i915_modparams.guc_log_level < 0);


	BUILD_BUG_ON(GUC_LOG_VERBOSITY_MIN);
	/*
	if (control_val > 1 + GUC_LOG_VERBOSITY_MAX)
	 * GuC is recognizing log levels starting from 0 to max, we're using 0
	 * as indication that logging should be disabled.
	 */
	if (val < GUC_LOG_LEVEL_DISABLED ||
	    val > VERBOSITY_TO_LOG_LEVEL(GUC_LOG_VERBOSITY_MAX))
		return -EINVAL;
		return -EINVAL;


	/* This combination doesn't make sense & won't have any effect */
	mutex_lock(&dev_priv->drm.struct_mutex);
	if (!enable_logging && !i915_modparams.guc_log_level)
		return 0;


	verbosity = enable_logging ? control_val - 1 : 0;
	if (i915_modparams.guc_log_level == val) {
		ret = 0;
		goto out_unlock;
	}


	ret = mutex_lock_interruptible(&dev_priv->drm.struct_mutex);
	if (ret)
		return ret;
	intel_runtime_pm_get(dev_priv);
	intel_runtime_pm_get(dev_priv);
	ret = guc_log_control(guc, enable_logging, verbosity);
	ret = guc_log_control(guc, enabled, LOG_LEVEL_TO_VERBOSITY(val));
	intel_runtime_pm_put(dev_priv);
	intel_runtime_pm_put(dev_priv);
	mutex_unlock(&dev_priv->drm.struct_mutex);
	if (ret) {

		DRM_DEBUG_DRIVER("guc_log_control action failed %d\n", ret);
	if (ret < 0) {
		goto out_unlock;
		DRM_DEBUG_DRIVER("guc_logging_control action failed %d\n", ret);
		return ret;
	}
	}


	if (enable_logging) {
	i915_modparams.guc_log_level = val;
		i915_modparams.guc_log_level = 1 + verbosity;


		/*
	mutex_unlock(&dev_priv->drm.struct_mutex);
		 * If log was disabled at boot time, then the relay channel file

		 * wouldn't have been created by now and interrupts also would
	if (enabled && !guc_log_has_runtime(guc)) {
		 * not have been enabled. Try again now, just in case.
		 */
		ret = guc_log_late_setup(guc);
		ret = guc_log_late_setup(guc);
		if (ret < 0) {
		if (ret) {
			DRM_DEBUG_DRIVER("GuC log late setup failed %d\n", ret);
			DRM_DEBUG_DRIVER("GuC log late setup failed %d\n", ret);
			return ret;
			goto out;
		}
		}


		/* GuC logging is currently the only user of Guc2Host interrupts */
		/* GuC logging is currently the only user of Guc2Host interrupts */
@@ -712,7 +724,7 @@ int intel_guc_log_control(struct intel_guc *guc, u64 control_val)
		gen9_enable_guc_interrupts(dev_priv);
		gen9_enable_guc_interrupts(dev_priv);
		intel_runtime_pm_put(dev_priv);
		intel_runtime_pm_put(dev_priv);
		mutex_unlock(&dev_priv->drm.struct_mutex);
		mutex_unlock(&dev_priv->drm.struct_mutex);
	} else {
	} else if (!enabled && guc_log_has_runtime(guc)) {
		/*
		/*
		 * Once logging is disabled, GuC won't generate logs & send an
		 * Once logging is disabled, GuC won't generate logs & send an
		 * interrupt. But there could be some data in the log buffer
		 * interrupt. But there could be some data in the log buffer
@@ -720,11 +732,13 @@ int intel_guc_log_control(struct intel_guc *guc, u64 control_val)
		 * buffer state and then collect the left over logs.
		 * buffer state and then collect the left over logs.
		 */
		 */
		guc_flush_logs(guc);
		guc_flush_logs(guc);

		/* As logging is disabled, update log level to reflect that */
		i915_modparams.guc_log_level = 0;
	}
	}


	return 0;

out_unlock:
	mutex_unlock(&dev_priv->drm.struct_mutex);
out:
	return ret;
	return ret;
}
}


+2 −1
Original line number Original line Diff line number Diff line
@@ -64,7 +64,8 @@ void intel_guc_log_destroy(struct intel_guc *guc);
void intel_guc_log_init_early(struct intel_guc *guc);
void intel_guc_log_init_early(struct intel_guc *guc);
int intel_guc_log_relay_create(struct intel_guc *guc);
int intel_guc_log_relay_create(struct intel_guc *guc);
void intel_guc_log_relay_destroy(struct intel_guc *guc);
void intel_guc_log_relay_destroy(struct intel_guc *guc);
int intel_guc_log_control(struct intel_guc *guc, u64 control_val);
int intel_guc_log_control_get(struct intel_guc *guc);
int intel_guc_log_control_set(struct intel_guc *guc, u64 control_val);
void i915_guc_log_register(struct drm_i915_private *dev_priv);
void i915_guc_log_register(struct drm_i915_private *dev_priv);
void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
void i915_guc_log_unregister(struct drm_i915_private *dev_priv);