On Mon, 2017-10-02 at 14:01 +0000, Michal Wajdeczko wrote:
> We want to keep GuC specific code in separated files.
>
> v2: move all functions in single patch (Joonas)
> fix old checkpatch issues (Sagar)
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx> #1
<SNIP>
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -0,0 +1,262 @@
> +/*
> + * Copyright © 2014-2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
We do want to include "intel_guc.h" first and then other headers. After
all, all this effort is to get rid of i915_drv.h eventually so that
inter-file dependencies are called out more explicitly.
> +#include "i915_drv.h"
> +
<SNIP>
> +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;
> +}
Have these inline functions in their natural spots below. Them being
inline or not is bound to change.
> +
> +/* intel_guc.c */
This should be obvious, comment can be dropped.
> +void intel_guc_init_early(struct intel_guc *guc);
> +void intel_guc_init_send_regs(struct intel_guc *guc);
> +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_sample_forcewake(struct intel_guc *guc);
> +int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
> +int intel_guc_suspend(struct drm_i915_private *dev_priv);
> +int intel_guc_resume(struct drm_i915_private *dev_priv);
> +struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
> +
> +/* intel_guc_loader.c */
These belong to intel_guc.c, so this comment can be dropped.
> +int intel_guc_select_fw(struct intel_guc *guc);
> +int intel_guc_init_hw(struct intel_guc *guc);
This goes under init_early at least, init_send_regs seems overly
specific and is not it like guc_init_mmio? Could be renamed later.
> +u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
> +
> +/* i915_guc_submission.c */
A separate header for these.
> +int i915_guc_submission_init(struct drm_i915_private *dev_priv);
> +int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
> +void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
> +void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
> +
> +/* intel_guc_log.c */
Ditto.
> +int intel_guc_log_create(struct intel_guc *guc);
> +void intel_guc_log_destroy(struct intel_guc *guc);
> +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);
> +
> +#endif
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
