From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yu Dai Subject: Re: [PATCH 07/13 v4] drm/i915: GuC submission setup, phase 1 Date: Mon, 27 Jul 2015 17:18:05 -0700 Message-ID: <55B6CA3D.7080803@intel.com> References: <1436466554-24806-1-git-send-email-david.s.gordon@intel.com> <1436466554-24806-8-git-send-email-david.s.gordon@intel.com> <20150724223158.GC142616@torourke-desk1> <55B6B39B.7000202@intel.com> <20150727231206.GB76344@torourke-desk1> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1442151387==" Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id A1E3989E69 for ; Mon, 27 Jul 2015 17:19:26 -0700 (PDT) In-Reply-To: <20150727231206.GB76344@torourke-desk1> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "O'Rourke, Tom" Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org This is a multi-part message in MIME format. --===============1442151387== Content-Type: multipart/alternative; boundary="------------070201010603000108050102" This is a multi-part message in MIME format. --------------070201010603000108050102 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: quoted-printable On 07/27/2015 04:12 PM, O'Rourke, Tom wrote: > On Mon, Jul 27, 2015 at 03:41:31PM -0700, Yu Dai wrote: > > > > > > On 07/24/2015 03:31 PM, O'Rourke, Tom wrote: > > >[TOR:] When I see "phase 1" I also look for "phase 2". > > >A subject that better describes the change in this patch > > >would help. > > > > > >On Thu, Jul 09, 2015 at 07:29:08PM +0100, Dave Gordon wrote: > > >> From: Alex Dai > > >> > > >> This adds the first of the data structures used to communicate wit= h the > > >> GuC (the pool of guc_context structures). > > >> > > >> We create a GuC-specific wrapper round the GEM object allocator as= all > > >> GEM objects shared with the GuC must be pinned into GGTT space at = an > > >> address that is NOT in the range [0..WOPCM_SIZE), as that range of= GGTT > > >> addresses is not accessible to the GuC (from the GuC's point of vi= ew, > > >> it's permanently reserved for other objects such as the BootROM & = SRAM). > > >[TOR:] I would like a clarfication on the excluded range. > > >The excluded range should be 0 to "size for guc within > > >WOPCM area" and not 0 to "size of WOPCM area". > > > > Nope, GGTT range [0..WOPCM_SIZE) should be excluded from GuC usage. > > BSpec clearly says, from 0 to WOPCM_TOP-1 is for BootROM, SRAM and > > WOPCM. From WOPCM_TOP and above is GFX DRAM. Be note that, that GGTT > > space is still available to any gfx obj as long as it is not > > accessed by GuC (OK to pass through GuC). > > > [TOR:] Should we take a closer look at the pin offset bias > for guc objects? GUC_WOPCM_SIZE_VALUE is not the full size > of WOPCM area. > =20 Yes, GUC_WOPCM_SIZE_VALUE is not the full size of WOPCM area. You can=20 blame BSpec for it asks SW to program this 'WOPCM Size' register to be=20 (*Physical WOPCM allocated size - GuC WOPCM Base*). So if you offset=20 range [GuC WOPCM Base, WOPCM_TOP] by GuC WOPCM Base, it will be [0,=20 GUC_WOPCM_SIZE]. This is the GuC 'accessible' range we would like to=20 exclude here. Alex > > >> > > >> Later, we will need to allocate additional GuC-sharable objects fo= r the > > >> submission client(s) and the GuC's debug log. > > >> > > >> v2: > > >> Remove redundant initialisation [Chris Wilson] > > >> Defer adding struct members until needed [Chris Wilson] > > >> Local functions should pass dev_priv rather than dev [Chris Wi= lson] > > >> > > >> v4: > > >> Rebased > > >> > > >> Issue: VIZ-4884 > > >> Signed-off-by: Alex Dai > > >> Signed-off-by: Dave Gordon > > >> --- > > >> drivers/gpu/drm/i915/Makefile | 3 +- > > >> drivers/gpu/drm/i915/i915_guc_submission.c | 114 ++++++++++++++++= +++++++++++++ > > >> drivers/gpu/drm/i915/intel_guc.h | 7 ++ > > >> drivers/gpu/drm/i915/intel_guc_loader.c | 19 +++++ > > >> 4 files changed, 142 insertions(+), 1 deletion(-) > > >> create mode 100644 drivers/gpu/drm/i915/i915_guc_submission.c > > >> > > >> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/= Makefile > > >> index e604cfe..23f5612 100644 > > >> --- a/drivers/gpu/drm/i915/Makefile > > >> +++ b/drivers/gpu/drm/i915/Makefile > > >> @@ -40,7 +40,8 @@ i915-y +=3D i915_cmd_parser.o \ > > >> intel_uncore.o > > >> > > >> # general-purpose microcontroller (GuC) support > > >> -i915-y +=3D intel_guc_loader.o > > >> +i915-y +=3D intel_guc_loader.o \ > > >> + i915_guc_submission.o > > >> > > >> # autogenerated null render state > > >> i915-y +=3D intel_renderstate_gen6.o \ > > >> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/= gpu/drm/i915/i915_guc_submission.c > > >> new file mode 100644 > > >> index 0000000..70a0405 > > >> --- /dev/null > > >> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > > >> @@ -0,0 +1,114 @@ > > >> +/* > > >> + * Copyright =A9 2014 Intel Corporation > > >> + * > > >> + * Permission is hereby granted, free of charge, to any person ob= taining 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, s= ublicense, > > >> + * and/or sell copies of the Software, and to permit persons to w= hom the > > >> + * Software is furnished to do so, subject to the following condi= tions: > > >> + * > > >> + * The above copyright notice and this permission notice (includi= ng the next > > >> + * paragraph) shall be included in all copies or substantial port= ions 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 MERCHA= NTABILITY, > > >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO E= VENT SHALL > > >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMA= GES 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 > > >> +#include > > >> +#include "i915_drv.h" > > >> +#include "intel_guc.h" > > >> + > > >> +/** > > >> + * gem_allocate_guc_obj() - Allocate gem object for GuC usage > > >> + * @dev: drm device > > >> + * @size: size of object > > >> + * > > >> + * This is a wrapper to create a gem obj. In order to use it insi= de GuC, the > > >> + * object needs to be pinned lifetime. Also we must pin it to gtt= space other > > >> + * than [0, GUC_WOPCM_SIZE] because this range is reserved inside= GuC. > > >> + * > > >> + * Return: A drm_i915_gem_object if successful, otherwise NULL. > > >> + */ > > >> +static struct drm_i915_gem_object *gem_allocate_guc_obj(struct dr= m_device *dev, > > >> + u32 size) > > >> +{ > > >> + struct drm_i915_gem_object *obj; > > >> + > > >> + obj =3D i915_gem_alloc_object(dev, size); > > >> + if (!obj) > > >> + return NULL; > > >> + > > >> + if (i915_gem_object_get_pages(obj)) { > > >> + drm_gem_object_unreference(&obj->base); > > >> + return NULL; > > >> + } > > >> + > > >> + if (i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, > > >> + PIN_OFFSET_BIAS | GUC_WOPCM_SIZE_VALUE)) { > > >> + drm_gem_object_unreference(&obj->base); > > >> + return NULL; > > >> + } > > >> + > > >> + return obj; > > >> +} > > >> + > > >> +/** > > >> + * gem_release_guc_obj() - Release gem object allocated for GuC u= sage > > >> + * @obj: gem obj to be released > > >> + */ > > >> +static void gem_release_guc_obj(struct drm_i915_gem_object *obj) > > >> +{ > > >> + if (!obj) > > >> + return; > > >> + > > >> + if (i915_gem_obj_is_pinned(obj)) > > >> + i915_gem_object_ggtt_unpin(obj); > > >> + > > >> + drm_gem_object_unreference(&obj->base); > > >> +} > > >> + > > >> +/* > > >> + * Set up the memory resources to be shared with the GuC. At thi= s point, > > >> + * we require just one object that can be mapped through the GGTT= . > > >> + */ > > >> +int i915_guc_submission_init(struct drm_device *dev) > > >> +{ > > >> + struct drm_i915_private *dev_priv =3D dev->dev_private; > > >> + const size_t ctxsize =3D sizeof(struct guc_context_desc); > > >> + const size_t poolsize =3D GUC_MAX_GPU_CONTEXTS * ctxsize; > > >> + const size_t gemsize =3D round_up(poolsize, PAGE_SIZE); > > >> + struct intel_guc *guc =3D &dev_priv->guc; > > >> + > > >> + if (!i915.enable_guc_submission) > > >> + return 0; /* not enabled */ > > >> + > > >> + if (guc->ctx_pool_obj) > > >> + return 0; /* already allocated */ > > >> + > > >> + guc->ctx_pool_obj =3D gem_allocate_guc_obj(dev_priv->dev, gemsiz= e); > > >> + if (!guc->ctx_pool_obj) > > >> + return -ENOMEM; > > >> + > > >> + ida_init(&guc->ctx_ids); > > >> + > > >> + return 0; > > >> +} > > >> + > > >> +void i915_guc_submission_fini(struct drm_device *dev) > > >> +{ > > >> + struct drm_i915_private *dev_priv =3D dev->dev_private; > > >> + struct intel_guc *guc =3D &dev_priv->guc; > > >> + > > >> + if (guc->ctx_pool_obj) > > >> + ida_destroy(&guc->ctx_ids); > > >> + gem_release_guc_obj(guc->ctx_pool_obj); > > >> + guc->ctx_pool_obj =3D NULL; > > >> +} > > >> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i9= 15/intel_guc.h > > >> index 2846b6d..be3cad8 100644 > > >> --- a/drivers/gpu/drm/i915/intel_guc.h > > >> +++ b/drivers/gpu/drm/i915/intel_guc.h > > >> @@ -56,6 +56,9 @@ struct intel_guc { > > >> struct intel_guc_fw guc_fw; > > >> > > >> uint32_t log_flags; > > >> + > > >> + struct drm_i915_gem_object *ctx_pool_obj; > > >> + struct ida ctx_ids; > > >> }; > > >> > > >> /* intel_guc_loader.c */ > > >> @@ -64,4 +67,8 @@ extern int intel_guc_ucode_load(struct drm_devic= e *dev); > > >> extern void intel_guc_ucode_fini(struct drm_device *dev); > > >> extern const char *intel_guc_fw_status_repr(enum intel_guc_fw_sta= tus status); > > >> > > >> +/* i915_guc_submission.c */ > > >> +int i915_guc_submission_init(struct drm_device *dev); > > >> +void i915_guc_submission_fini(struct drm_device *dev); > > >> + > > >[TOR:] i915_guc_submission_init gets used in this patch. > > >Unexpectedly, i915_guc_submission_fini does not get used > > >in this patch. > > > > > >A later patch in this series adds the call to > > >i915_guc_submission_fini in intel_guc_ucode_fini. > > >Should that call be added in this patch? > > > > > >> #endif > > >> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu= /drm/i915/intel_guc_loader.c > > >> index 2080bca..e5d7136 100644 > > >> --- a/drivers/gpu/drm/i915/intel_guc_loader.c > > >> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > > >> @@ -128,6 +128,21 @@ static void set_guc_init_params(struct drm_i9= 15_private *dev_priv) > > >> i915.guc_log_level << GUC_LOG_VERBOSITY_SHIFT; > > >> } > > >> > > >> + /* If GuC scheduling is enabled, setup params here. */ > > >[TOR:] I assume from this "GuC scheduling" =3D=3D "GuC submission". > > >This is a little confusing. I recommend either reword > > >"GuC scheduling" or add comment text to explain. > > > > For now, yes the GuC scheduling is only doing the 'submission' work > > because of the current kernel in-order queue. If we have client > > direct submission enabled, there WILL BE GuC scheduling inside > > firmware according to the priority of each queue etc. > > > > Thanks, > > Alex --------------070201010603000108050102 Content-Type: text/html; charset=windows-1252 Content-Transfer-Encoding: quoted-printable

On 07/27/2015 04:12 PM, O'Rourke, Tom wrote:
On Mon, Jul 27, 2015 at 03:41:31PM -0700, Yu Dai wro=
te:
>=20
>=20
> On 07/24/2015 03:31 PM, O'Rourke, Tom wrote:
> >[TOR:] When I see "phase 1" I also look for "phase 2".
> >A subject that better describes the change in this patch
> >would help.
> >
> >On Thu, Jul 09, 2015 at 07:29:08PM +0100, Dave Gordon wrote:
> >> From: Alex Dai <yu.dai@intel.com>
> >>
> >> This adds the first of the data structures used to communic=
ate with the
> >> GuC (the pool of guc_context structures).
> >>
> >> We create a GuC-specific wrapper round the GEM object alloc=
ator as all
> >> GEM objects shared with the GuC must be pinned into GGTT sp=
ace at an
> >> address that is NOT in the range [0..WOPCM_SIZE), as that r=
ange of GGTT
> >> addresses is not accessible to the GuC (from the GuC's poin=
t of view,
> >> it's permanently reserved for other objects such as the Boo=
tROM & SRAM).
> >[TOR:] I would like a clarfication on the excluded range.
> >The excluded range should be 0 to "size for guc within
> >WOPCM area" and not 0 to "size of WOPCM area".
>=20
> Nope, GGTT range [0..WOPCM_SIZE) should be excluded from GuC usage.
> BSpec clearly says, from 0 to WOPCM_TOP-1 is for BootROM, SRAM and
> WOPCM. From WOPCM_TOP and above is GFX DRAM. Be note that, that GGTT
> space is still available to any gfx obj as long as it is not
> accessed by GuC (OK to pass through GuC).
>
[TOR:] Should we take a closer look at the pin offset bias
for guc objects?  GUC_WOPCM_SIZE_VALUE is not the full size
of WOPCM area.

Yes, GUC_WOPCM_SIZE_VALUE is not the full size of WOPCM area. You can blame BSpec for it asks SW to program this 'WOPCM Size' register to be (Physical WOPCM allocated size - GuC WOPCM Base). So if you offset range [GuC WOPCM Base, WOPCM_TOP] by GuC WOPCM Base, it will be [0, GUC_WOPCM_SIZE]. This is the GuC 'accessible' range we would like to exclude here.

Alex

> >>
> >> Later, we will need to allocate additional GuC-sharable obj=
ects for the
> >> submission client(s) and the GuC's debug log.
> >>
> >> v2:
> >>     Remove redundant initialisation [Chris Wilson]
> >>     Defer adding struct members until needed [Chris Wilson]
> >>     Local functions should pass dev_priv rather than dev [C=
hris Wilson]
> >>
> >> v4:
> >>     Rebased
> >>
> >> Issue: VIZ-4884
> >> Signed-off-by: Alex Dai <yu.dai@intel.com>
> >> Signed-off-by: Dave Gordon <david.s.gordon@intel.com&=
gt;
> >> ---
> >>  drivers/gpu/drm/i915/Makefile              |   3 +-
> >>  drivers/gpu/drm/i915/i915_guc_submission.c | 114 +++++++++=
++++++++++++++++++++
> >>  drivers/gpu/drm/i915/intel_guc.h           |   7 ++
> >>  drivers/gpu/drm/i915/intel_guc_loader.c    |  19 +++++
> >>  4 files changed, 142 insertions(+), 1 deletion(-)
> >>  create mode 100644 drivers/gpu/drm/i915/i915_guc_submissio=
n.c
> >>
> >> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/dr=
m/i915/Makefile
> >> index e604cfe..23f5612 100644
> >> --- a/drivers/gpu/drm/i915/Makefile
> >> +++ b/drivers/gpu/drm/i915/Makefile
> >> @@ -40,7 +40,8 @@ i915-y +=3D i915_cmd_parser.o \
> >>  	  intel_uncore.o
> >>
> >>  # general-purpose microcontroller (GuC) support
> >> -i915-y +=3D intel_guc_loader.o
> >> +i915-y +=3D intel_guc_loader.o \
> >> +	  i915_guc_submission.o
> >>
> >>  # autogenerated null render state
> >>  i915-y +=3D intel_renderstate_gen6.o \
> >> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/d=
rivers/gpu/drm/i915/i915_guc_submission.c
> >> new file mode 100644
> >> index 0000000..70a0405
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> >> @@ -0,0 +1,114 @@
> >> +/*
> >> + * Copyright =A9 2014 Intel Corporation
> >> + *
> >> + * Permission is hereby granted, free of charge, to any pe=
rson obtaining a
> >> + * copy of this software and associated documentation file=
s (the "Software"),
> >> + * to deal in the Software without restriction, including =
without limitation
> >> + * the rights to use, copy, modify, merge, publish, distri=
bute, sublicense,
> >> + * and/or sell copies of the Software, and to permit perso=
ns to whom the
> >> + * Software is furnished to do so, subject to the followin=
g conditions:
> >> + *
> >> + * The above copyright notice and this permission notice (=
including the next
> >> + * paragraph) shall be included in all copies or substanti=
al portions of the
> >> + * Software.
> >> + *
> >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF A=
NY 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 CLAI=
M, DAMAGES OR OTHER
> >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OT=
HERWISE, ARISING
> >> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE =
USE OR OTHER DEALINGS
> >> + * IN THE SOFTWARE.
> >> + *
> >> + */
> >> +#include <linux/firmware.h>
> >> +#include <linux/circ_buf.h>
> >> +#include "i915_drv.h"
> >> +#include "intel_guc.h"
> >> +
> >> +/**
> >> + * gem_allocate_guc_obj() - Allocate gem object for GuC us=
age
> >> + * @dev:	drm device
> >> + * @size:	size of object
> >> + *
> >> + * This is a wrapper to create a gem obj. In order to use =
it inside GuC, the
> >> + * object needs to be pinned lifetime. Also we must pin it=
 to gtt space other
> >> + * than [0, GUC_WOPCM_SIZE] because this range is reserved=
 inside GuC.
> >> + *
> >> + * Return:	A drm_i915_gem_object if successful, otherwise =
NULL.
> >> + */
> >> +static struct drm_i915_gem_object *gem_allocate_guc_obj(st=
ruct drm_device *dev,
> >> +							u32 size)
> >> +{
> >> +	struct drm_i915_gem_object *obj;
> >> +
> >> +	obj =3D i915_gem_alloc_object(dev, size);
> >> +	if (!obj)
> >> +		return NULL;
> >> +
> >> +	if (i915_gem_object_get_pages(obj)) {
> >> +		drm_gem_object_unreference(&obj->base);
> >> +		return NULL;
> >> +	}
> >> +
> >> +	if (i915_gem_obj_ggtt_pin(obj, PAGE_SIZE,
> >> +			PIN_OFFSET_BIAS | GUC_WOPCM_SIZE_VALUE)) {
> >> +		drm_gem_object_unreference(&obj->base);
> >> +		return NULL;
> >> +	}
> >> +
> >> +	return obj;
> >> +}
> >> +
> >> +/**
> >> + * gem_release_guc_obj() - Release gem object allocated fo=
r GuC usage
> >> + * @obj:	gem obj to be released
> >> +  */
> >> +static void gem_release_guc_obj(struct drm_i915_gem_object=
 *obj)
> >> +{
> >> +	if (!obj)
> >> +		return;
> >> +
> >> +	if (i915_gem_obj_is_pinned(obj))
> >> +		i915_gem_object_ggtt_unpin(obj);
> >> +
> >> +	drm_gem_object_unreference(&obj->base);
> >> +}
> >> +
> >> +/*
> >> + * Set up the memory resources to be shared with the GuC. =
 At this point,
> >> + * we require just one object that can be mapped through t=
he GGTT.
> >> + */
> >> +int i915_guc_submission_init(struct drm_device *dev)
> >> +{
> >> +	struct drm_i915_private *dev_priv =3D dev->dev_private=
;
> >> +	const size_t ctxsize =3D sizeof(struct guc_context_desc);
> >> +	const size_t poolsize =3D GUC_MAX_GPU_CONTEXTS * ctxsize;
> >> +	const size_t gemsize =3D round_up(poolsize, PAGE_SIZE);
> >> +	struct intel_guc *guc =3D &dev_priv->guc;
> >> +
> >> +	if (!i915.enable_guc_submission)
> >> +		return 0; /* not enabled  */
> >> +
> >> +	if (guc->ctx_pool_obj)
> >> +		return 0; /* already allocated */
> >> +
> >> +	guc->ctx_pool_obj =3D gem_allocate_guc_obj(dev_priv-&g=
t;dev, gemsize);
> >> +	if (!guc->ctx_pool_obj)
> >> +		return -ENOMEM;
> >> +
> >> +	ida_init(&guc->ctx_ids);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +void i915_guc_submission_fini(struct drm_device *dev)
> >> +{
> >> +	struct drm_i915_private *dev_priv =3D dev->dev_private=
;
> >> +	struct intel_guc *guc =3D &dev_priv->guc;
> >> +
> >> +	if (guc->ctx_pool_obj)
> >> +		ida_destroy(&guc->ctx_ids);
> >> +	gem_release_guc_obj(guc->ctx_pool_obj);
> >> +	guc->ctx_pool_obj =3D NULL;
> >> +}
> >> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu=
/drm/i915/intel_guc.h
> >> index 2846b6d..be3cad8 100644
> >> --- a/drivers/gpu/drm/i915/intel_guc.h
> >> +++ b/drivers/gpu/drm/i915/intel_guc.h
> >> @@ -56,6 +56,9 @@ struct intel_guc {
> >>  	struct intel_guc_fw guc_fw;
> >>
> >>  	uint32_t log_flags;
> >> +
> >> +	struct drm_i915_gem_object *ctx_pool_obj;
> >> +	struct ida ctx_ids;
> >>  };
> >>
> >>  /* intel_guc_loader.c */
> >> @@ -64,4 +67,8 @@ extern int intel_guc_ucode_load(struct dr=
m_device *dev);
> >>  extern void intel_guc_ucode_fini(struct drm_device *dev);
> >>  extern const char *intel_guc_fw_status_repr(enum intel_guc=
_fw_status status);
> >>
> >> +/* i915_guc_submission.c */
> >> +int i915_guc_submission_init(struct drm_device *dev);
> >> +void i915_guc_submission_fini(struct drm_device *dev);
> >> +
> >[TOR:] i915_guc_submission_init gets used in this patch.
> >Unexpectedly, i915_guc_submission_fini does not get used
> >in this patch.
> >
> >A later patch in this series adds the call to
> >i915_guc_submission_fini in intel_guc_ucode_fini.
> >Should that call be added in this patch?
> >
> >>  #endif
> >> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/driv=
ers/gpu/drm/i915/intel_guc_loader.c
> >> index 2080bca..e5d7136 100644
> >> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> >> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> >> @@ -128,6 +128,21 @@ static void set_guc_init_params(struct=
 drm_i915_private *dev_priv)
> >>  			i915.guc_log_level << GUC_LOG_VERBOSITY_SHIFT;
> >>  	}
> >>
> >> +	/* If GuC scheduling is enabled, setup params here. */
> >[TOR:] I assume from this "GuC scheduling" =3D=3D "GuC submissio=
n".
> >This is a little confusing.  I recommend either reword
> >"GuC scheduling" or add comment text to explain.
>=20
> For now, yes the GuC scheduling is only doing the 'submission' work
> because of the current kernel in-order queue. If we have client
> direct submission enabled, there WILL BE GuC scheduling inside
> firmware according to the priority of each queue etc.
>=20
> Thanks,
> Alex

--------------070201010603000108050102-- --===============1442151387== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --===============1442151387==--