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

Tvrtko Ursulin
2022-09-05T15:58:37Z

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
>