Re: [CI v4 2/3] drm/i915/guc: Introduce buffer based cmd transport

[Chris Wilson <chris@chris-wilson.co.uk>]

On Fri, May 26, 2017 at 11:13:25AM +0000, Michal Wajdeczko wrote:
> Buffer based command transport can replace MMIO based mechanism.
> It may be used to perform host-2-guc and guc-to-host communication.
> 
> Portions of this patch are based on work by:
>  Michel Thierry <michel.thierry@intel.com>
>  Robert Beckett <robert.beckett@intel.com>
>  Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> v2: use gem_object_pin_map (Chris)
>     don't use DEBUG_RATELIMITED (Chris)
>     don't track action stats (Chris)
>     simplify next fence (Chris)
>     use READ_ONCE (Chris)
>     move blob allocation to new function (Chris)
> 
> v3: use static owner id (Daniele)
> v4: but keep channel initialization generic (Daniele)
>     and introduce owner_sub_id (Daniele)
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile         |   1 +
>  drivers/gpu/drm/i915/i915_drv.h       |   2 +
>  drivers/gpu/drm/i915/intel_guc_ct.c   | 461 ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_guc_ct.h   |  86 +++++++
>  drivers/gpu/drm/i915/intel_guc_fwif.h |  43 ++++
>  drivers/gpu/drm/i915/intel_uc.c       |  19 +-
>  drivers/gpu/drm/i915/intel_uc.h       |   3 +-
>  7 files changed, 613 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_guc_ct.c
>  create mode 100644 drivers/gpu/drm/i915/intel_guc_ct.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 7b05fb8..16dccf5 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -58,6 +58,7 @@ i915-y += i915_cmd_parser.o \
>  
>  # general-purpose microcontroller (GuC) support
>  i915-y += intel_uc.o \
> +	  intel_guc_ct.o \
>  	  intel_guc_log.o \
>  	  intel_guc_loader.o \
>  	  intel_huc.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9ba2242..d2a5749 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -760,6 +760,7 @@ struct intel_csr {
>  	func(has_gmbus_irq); \
>  	func(has_gmch_display); \
>  	func(has_guc); \
> +	func(has_guc_ct); \
>  	func(has_hotplug); \
>  	func(has_l3_dpf); \
>  	func(has_llc); \
> @@ -2947,6 +2948,7 @@ intel_info(const struct drm_i915_private *dev_priv)
>   * properties, so we have separate macros to test them.
>   */
>  #define HAS_GUC(dev_priv)	((dev_priv)->info.has_guc)
> +#define HAS_GUC_CT(dev_priv)	((dev_priv)->info.has_guc_ct)
>  #define HAS_GUC_UCODE(dev_priv)	(HAS_GUC(dev_priv))
>  #define HAS_GUC_SCHED(dev_priv)	(HAS_GUC(dev_priv))
>  #define HAS_HUC_UCODE(dev_priv)	(HAS_GUC(dev_priv))
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
> new file mode 100644
> index 0000000..c4cbec1
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -0,0 +1,461 @@
> +/*
> + * Copyright © 2016-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.
> + */
> +
> +#include "i915_drv.h"
> +#include "intel_guc_ct.h"
> +
> +enum { CTB_SEND = 0, CTB_RECV = 1 };
> +
> +enum { CTB_OWNER_HOST = 0 };
> +
> +void intel_guc_ct_init_early(struct intel_guc_ct *ct)
> +{
> +	/* we're using static channel owners */
> +	ct->host_channel.owner = CTB_OWNER_HOST;
> +}
> +
> +static inline const char *guc_ct_buffer_type_to_str(u32 type)
> +{
> +	switch (type) {
> +	case INTEL_GUC_CT_BUFFER_TYPE_SEND:
> +		return "SEND";
> +	case INTEL_GUC_CT_BUFFER_TYPE_RECV:
> +		return "RECV";
> +	default:
> +		return "<invalid>";
> +	}
> +}
> +
> +static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc,
> +				    u32 cmds_addr, u32 size, u32 owner)
> +{
> +	DRM_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
> +			 desc, cmds_addr, size, owner);
> +	memset(desc, 0, sizeof(*desc));
> +	desc->addr = cmds_addr;
> +	desc->size = size;
> +	desc->owner = owner;
> +}
> +
> +static void guc_ct_buffer_desc_reset(struct guc_ct_buffer_desc *desc)
> +{
> +	DRM_DEBUG_DRIVER("CT: desc %p reset head=%u tail=%u\n",
> +			 desc, desc->head, desc->tail);
> +	desc->head = 0;
> +	desc->tail = 0;
> +	desc->is_in_error = 0;
> +}
> +
> +static int guc_action_register_ct_buffer(struct intel_guc *guc,
> +					 u32 desc_addr,
> +					 u32 type)
> +{
> +	u32 action[] = {
> +		INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER,
> +		desc_addr,
> +		sizeof(struct guc_ct_buffer_desc),
> +		type
> +	};
> +	int err;
> +
> +	/* Can't use generic send(), CT registration must go over MMIO */
> +	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action));
> +	if (err)
> +		DRM_ERROR("CT: register %s buffer failed; err=%d\n",
> +			  guc_ct_buffer_type_to_str(type), err);
> +	return err;
> +}
> +
> +static int guc_action_deregister_ct_buffer(struct intel_guc *guc,
> +					   u32 owner,
> +					   u32 type)
> +{
> +	u32 action[] = {
> +		INTEL_GUC_ACTION_DEREGISTER_COMMAND_TRANSPORT_BUFFER,
> +		owner,
> +		type
> +	};
> +	int err;
> +
> +	/* Can't use generic send(), CT deregistration must go over MMIO */
> +	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action));
> +	if (err)
> +		DRM_ERROR("CT: deregister %s buffer failed; owner=%d err=%d\n",
> +			  guc_ct_buffer_type_to_str(type), owner, err);
> +	return err;
> +}
> +
> +static bool ctch_is_open(struct intel_guc_ct_channel *ctch)
> +{
> +	return ctch->vma != NULL;
> +}
> +
> +static int ctch_init(struct intel_guc *guc,
> +		     struct intel_guc_ct_channel *ctch)
> +{
> +	struct i915_vma *vma;
> +	void *blob;
> +	int err;
> +	int i;
> +
> +	GEM_BUG_ON(ctch->vma);
> +
> +	/* We allocate 1 page to hold both descriptors and both buffers.
> +	 *       ___________.....................
> +	 *      |desc (SEND)|                   :
> +	 *      |___________|                   PAGE/4
> +	 *      :___________....................:
> +	 *      |desc (RECV)|                   :
> +	 *      |___________|                   PAGE/4
> +	 *      :_______________________________:
> +	 *      |cmds (SEND)                    |
> +	 *      |                               PAGE/4
> +	 *      |_______________________________|
> +	 *      |cmds (RECV)                    |
> +	 *      |                               PAGE/4
> +	 *      |_______________________________|
> +	 *
> +	 * Each message can use a maximum of 32 dwords and we don't expect to
> +	 * have more than 1 in flight at any time, so we have enough space.
> +	 * Some logic further ahead will rely on the fact that there is only 1
> +	 * page and that it is always mapped, so if the size is changed the
> +	 * other code will need updating as well.
> +	 */
> +
> +	/* allocate vma */
> +	vma = intel_guc_allocate_vma(guc, PAGE_SIZE);
> +	if (IS_ERR(vma)) {
> +		err = PTR_ERR(vma);
> +		goto err_out;
> +	}
> +	ctch->vma = vma;
> +
> +	/* map first page */
> +	blob = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
> +	if (IS_ERR(blob)) {
> +		err = PTR_ERR(blob);
> +		goto err_vma;
> +	}
> +	DRM_DEBUG_DRIVER("CT: vma base=%#x\n", guc_ggtt_offset(ctch->vma));
> +
> +	/* store pointers to desc and cmds */
> +	for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
> +		GEM_BUG_ON((i != CTB_SEND) && (i != CTB_RECV));
> +		ctch->ctbs[i].desc = blob + PAGE_SIZE/4 * i;
> +		ctch->ctbs[i].cmds = blob + PAGE_SIZE/4 * i + PAGE_SIZE/2;
> +	}
> +
> +	return 0;
> +
> +err_vma:
> +	i915_vma_unpin_and_release(&ctch->vma);
> +err_out:
> +	DRM_DEBUG_DRIVER("CT: channel %d initialization failed; err=%d\n",
> +			 ctch->owner, err);
> +	return err;
> +}
> +
> +static void ctch_fini(struct intel_guc *guc,
> +		      struct intel_guc_ct_channel *ctch)
> +{
> +	GEM_BUG_ON(!ctch->vma);
> +
> +	i915_gem_object_unpin_map(ctch->vma->obj);
> +	i915_vma_unpin_and_release(&ctch->vma);
> +}
> +
> +static int ctch_open(struct intel_guc *guc,
> +		     struct intel_guc_ct_channel *ctch)
> +{
> +	u32 base;
> +	int err;
> +	int i;
> +
> +	DRM_DEBUG_DRIVER("CT: channel %d reopen=%s\n",
> +			 ctch->owner, yesno(ctch_is_open(ctch)));
> +
> +	if (!ctch->vma) {
> +		err = ctch_init(guc, ctch);
> +		if (unlikely(err))
> +			goto err_out;
> +	}
> +
> +	/* vma should be already allocated and map'ed */
> +	base = guc_ggtt_offset(ctch->vma);
> +
> +	/* (re)initialize descriptors
> +	 * cmds buffers are in the second half of the blob page
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
> +		GEM_BUG_ON((i != CTB_SEND) && (i != CTB_RECV));
> +		guc_ct_buffer_desc_init(ctch->ctbs[i].desc,
> +					base + PAGE_SIZE/4 * i + PAGE_SIZE/2,
> +					PAGE_SIZE/4,

Hmm, I would like to see these reuse the offsets computed in
chct_init(). And for ctcb_write() to take advantage of the pot size
(that will at least make it look like a more regular circ_buf.h)

But not a blocker, so pushed.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx