Re: [PATCH v2 2/3] drm/i915/guc: Drop union guc_log_control

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 19 Mar 2018 16:32:05 +0100, Michał Winiarski <michal.winiarski@xxxxxxxxx> wrote:

On Mon, Mar 19, 2018 at 01:49:23PM +0000, Michal Wajdeczko wrote:
Usually we use shift/mask macros for bit field definitions.
Union guc_log_control was not following that pattern.

Additional bonus:

add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-25 (-25)
Function                                     old     new   delta
intel_guc_log_level_set                      388     363     -25

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Michal Winiarski <michal.winiarski@xxxxxxxxx>
Cc: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
---
 drivers/gpu/drm/i915/intel_guc_fwif.h | 16 +++++-----------
 drivers/gpu/drm/i915/intel_guc_log.c  | 11 +++--------
 2 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 4971685..72941bd 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -534,17 +534,6 @@ struct guc_log_buffer_state {
 	u32 version;
 } __packed;

-union guc_log_control {
-	struct {
-		u32 logging_enabled:1;
-		u32 reserved1:3;
-		u32 verbosity:4;
-		u32 default_logging:1;
-		u32 reserved2:23;
-	};
-	u32 value;
-} __packed;
-
 struct guc_ctx_report {
 	u32 report_return_status;
 	u32 reserved1[64];
@@ -603,6 +592,11 @@ enum intel_guc_report_status {
 	INTEL_GUC_REPORT_STATUS_COMPLETE = 0x4,
 };

+#define GUC_LOG_CONTROL_LOGGING_ENABLED	(1 << 0)
+#define GUC_LOG_CONTROL_VERBOSITY_SHIFT	4
+#define GUC_LOG_CONTROL_VERBOSITY_MASK (0xF << GUC_LOG_CONTROL_VERBOSITY_SHIFT)

You're not using the mask anywhere. Is it just to describe the iface?

yep, only bit-field interface

Or was it supposed to be used in guc_action_control_log?
(there's a slight change in behavior here, which may lead to confusion when
passing out-of-range verbosity)

As you already check for out-of-range verbosity level passed by user:

	if (val < GUC_LOG_LEVEL_DISABLED ||
	    val > GUC_VERBOSITY_TO_LOG_LEVEL(GUC_LOG_VERBOSITY_MAX))
		return -EINVAL;

I can make my check in guc_action() more assertive:

	GEM_BUG_ON(verbosity > GUC_LOG_VERBOSITY_MAX)

Thanks,
/m
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux