drm/i915: Don't mix inline functions with declarations

Commit Message

Michal Wajdeczko Sept. 26, 2017, 2:37 p.m. UTC

We group function forward declarations by file. There is no
reason to mix these declarations with our unline functions.

Signed-off-by: Michal Wajdeczko <[e-mail]>
Cc: Joonas Lahtinen <[e-mail]>
---
 drivers/gpu/drm/i915/intel_uc.h | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

Comments

Chris Wilson Sept. 26, 2017, 2:46 p.m. UTC | #1

Quoting Michal Wajdeczko (2017-09-26 15:37:23)
> We group function forward declarations by file. There is no
> reason to mix these declarations with our unline functions.

Can of worms, can of worms. Our headers are a mess because we mix types
and inlines, and so we end up with code being placed not logically where
one would expect, but because CPP dictates it so.

If you want to make a good improvements towards cleaning them up,
spitting out the types would be my first step.

/wishlist
-Chris

Joonas Lahtinen Sept. 27, 2017, 9:57 a.m. UTC | #2

On Tue, 2017-09-26 at 14:37 +0000, Michal Wajdeczko wrote:
> We group function forward declarations by file. There is no
> reason to mix these declarations with our unline functions.

I'd rather like to avoid these megaheaders completely, so that
intel_uc.h would be a base header with common structs and stuff and
intel_guc.h and intel_huc.h introduce the functions and stuff which is
then implemented by their respective ".c" files.

The files are still relatively easy, so the earlier it's done, the less
pain there should be.

Then, when the megaheader is broken, the functions should be in natural
order, regardless of being inline or not (that's a volatile thing, and
may change between kernel versions).

Regards, Joonas

Patch

diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 6966349..ae4cd0d 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -201,6 +201,24 @@  struct intel_huc {
 	/* HuC-specific additions */
 };
 
+static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
+{
+	return guc->send(guc, action, len);
+}
+
+static inline void intel_guc_notify(struct intel_guc *guc)
+{
+	guc->notify(guc);
+}
+
+static inline u32 guc_ggtt_offset(struct i915_vma *vma)
+{
+	u32 offset = i915_ggtt_offset(vma);
+	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
+	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
+	return offset;
+}
+
 /* intel_uc.c */
 void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
 void intel_uc_init_early(struct drm_i915_private *dev_priv);
@@ -213,16 +231,6 @@  int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
 int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
 
-static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
-{
-	return guc->send(guc, action, len);
-}
-
-static inline void intel_guc_notify(struct intel_guc *guc)
-{
-	guc->notify(guc);
-}
-
 /* intel_guc_loader.c */
 int intel_guc_select_fw(struct intel_guc *guc);
 int intel_guc_init_hw(struct intel_guc *guc);
@@ -244,14 +252,6 @@  int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
 void i915_guc_log_register(struct drm_i915_private *dev_priv);
 void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
 
-static inline u32 guc_ggtt_offset(struct i915_vma *vma)
-{
-	u32 offset = i915_ggtt_offset(vma);
-	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
-	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
-	return offset;
-}
-
 /* intel_huc.c */
 void intel_huc_select_fw(struct intel_huc *huc);
 void intel_huc_init_hw(struct intel_huc *huc);