Re: [Intel-gfx] [PATCH 5/5] drm/i915: split out i915_gem.c declarations to i915_gem.h

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 05/09/2022 16:00, Jani Nikula wrote:
> Declutter i915_drv.h by splitting out the declarations for
> i915_gem.[ch].
> 
> Add a fixme comment about the rest of the stuff in i915_gem.h that
> doesn't really belong there.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h | 36 ---------------------------
>   drivers/gpu/drm/i915/i915_gem.h | 43 +++++++++++++++++++++++++++++++++
>   2 files changed, 43 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bbfc295f386b..befb167b3c49 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -972,42 +972,6 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>   
>   #define HAS_ONE_EU_PER_FUSE_BIT(i915)	(INTEL_INFO(i915)->has_one_eu_per_fuse_bit)
>   
> -/* i915_gem.c */
> -void i915_gem_init_early(struct drm_i915_private *dev_priv);
> -void i915_gem_cleanup_early(struct drm_i915_private *dev_priv);
> -
> -void i915_gem_drain_freed_objects(struct drm_i915_private *i915);
> -void i915_gem_drain_workqueue(struct drm_i915_private *i915);
> -
> -struct i915_vma * __must_check
> -i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj,
> -			    struct i915_gem_ww_ctx *ww,
> -			    const struct i915_ggtt_view *view,
> -			    u64 size, u64 alignment, u64 flags);
> -
> -struct i915_vma * __must_check
> -i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
> -			 const struct i915_ggtt_view *view,
> -			 u64 size, u64 alignment, u64 flags);
> -
> -int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
> -			   unsigned long flags);
> -#define I915_GEM_OBJECT_UNBIND_ACTIVE BIT(0)
> -#define I915_GEM_OBJECT_UNBIND_BARRIER BIT(1)
> -#define I915_GEM_OBJECT_UNBIND_TEST BIT(2)
> -#define I915_GEM_OBJECT_UNBIND_VM_TRYLOCK BIT(3)
> -#define I915_GEM_OBJECT_UNBIND_ASYNC BIT(4)
> -
> -void i915_gem_runtime_suspend(struct drm_i915_private *dev_priv);
> -
> -int __must_check i915_gem_init(struct drm_i915_private *dev_priv);
> -void i915_gem_driver_register(struct drm_i915_private *i915);
> -void i915_gem_driver_unregister(struct drm_i915_private *i915);
> -void i915_gem_driver_remove(struct drm_i915_private *dev_priv);
> -void i915_gem_driver_release(struct drm_i915_private *dev_priv);
> -
> -int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file);
> -
>   /* intel_device_info.c */
>   static inline struct intel_device_info *
>   mkwrite_device_info(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
> index 68d8d52bd541..2fccb19ed9f7 100644
> --- a/drivers/gpu/drm/i915/i915_gem.h
> +++ b/drivers/gpu/drm/i915/i915_gem.h
> @@ -26,12 +26,55 @@
>   #define __I915_GEM_H__
>   
>   #include <linux/bug.h>
> +#include <linux/types.h>
>   
>   #include <drm/drm_drv.h>
>   
>   #include "i915_utils.h"
>   
> +struct drm_file;
> +struct drm_i915_gem_object;
>   struct drm_i915_private;
> +struct i915_gem_ww_ctx;
> +struct i915_ggtt_view;
> +struct i915_vma;
> +
> +void i915_gem_init_early(struct drm_i915_private *i915);
> +void i915_gem_cleanup_early(struct drm_i915_private *i915);
> +
> +void i915_gem_drain_freed_objects(struct drm_i915_private *i915);
> +void i915_gem_drain_workqueue(struct drm_i915_private *i915);
> +
> +struct i915_vma * __must_check
> +i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj,
> +			    struct i915_gem_ww_ctx *ww,
> +			    const struct i915_ggtt_view *view,
> +			    u64 size, u64 alignment, u64 flags);
> +
> +struct i915_vma * __must_check
> +i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
> +			 const struct i915_ggtt_view *view,
> +			 u64 size, u64 alignment, u64 flags);
> +
> +int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
> +			   unsigned long flags);
> +#define I915_GEM_OBJECT_UNBIND_ACTIVE BIT(0)
> +#define I915_GEM_OBJECT_UNBIND_BARRIER BIT(1)
> +#define I915_GEM_OBJECT_UNBIND_TEST BIT(2)
> +#define I915_GEM_OBJECT_UNBIND_VM_TRYLOCK BIT(3)
> +#define I915_GEM_OBJECT_UNBIND_ASYNC BIT(4)
> +
> +void i915_gem_runtime_suspend(struct drm_i915_private *i915);
> +
> +int __must_check i915_gem_init(struct drm_i915_private *i915);
> +void i915_gem_driver_register(struct drm_i915_private *i915);
> +void i915_gem_driver_unregister(struct drm_i915_private *i915);
> +void i915_gem_driver_remove(struct drm_i915_private *i915);
> +void i915_gem_driver_release(struct drm_i915_private *i915);
> +
> +int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file);
> +
> +/* FIXME: All of the below belong somewhere else. */

For the series:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

(((
I think historically i915_gem.h started as a stash for random bits which 
felt obviously wrong to put elsewhere, but it should be fine to 
"upgrade" it to a more important status now that you are working on 
cleaning things up, especially i915_drv.h.

Where this "somewhere else" place could be is a bit tricky - I suspect 
there isn't any great urgency to re-home them. If one day splitting 
i915_gem.c into functional parts comes on the agenda so I guess then. 
But it's not that huge even so don't think it's top priority.
)))

Regards,

Tvrtko

>   
>   #ifdef CONFIG_DRM_I915_DEBUG_GEM
>