[Intel-xe] [PATCH v3 18/20] drm/xe/guc: Allocate GuC data structures in system memory for initial load

[Lucas De Marchi]

On Tue, Nov 21, 2023 at 02:42:20PM +0100, Michał Winiarski wrote:
>On Mon, Nov 20, 2023 at 02:20:22PM -0600, Lucas De Marchi wrote:
>> On Tue, Nov 14, 2023 at 02:02:29PM +0100, Michał Winiarski wrote:
>> > GuC load will need to happen at an earlier point in probe, where local
>> > memory is not yet available. Use system memory for GuC data structures
>> > used for initial "hwconfig" load, and realloc at a later,
>> > "post-hwconfig" load, when local memory is available.
>> >
>> > Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
>> > ---
>> > drivers/gpu/drm/xe/xe_guc.c     | 45 +++++++++++++++++++++++++++++++++
>> > drivers/gpu/drm/xe/xe_guc_ads.c |  2 +-
>> > drivers/gpu/drm/xe/xe_guc_ct.c  |  2 +-
>> > drivers/gpu/drm/xe/xe_guc_log.c |  2 +-
>> > 4 files changed, 48 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
>> > index d1181ddf4bdae..3f430d5bc1051 100644
>> > --- a/drivers/gpu/drm/xe/xe_guc.c
>> > +++ b/drivers/gpu/drm/xe/xe_guc.c
>> > @@ -251,6 +251,45 @@ static void guc_fini(struct drm_device *drm, void *arg)
>> > 	xe_force_wake_put(gt_to_fw(guc_to_gt(guc)), XE_FORCEWAKE_ALL);
>> > }
>> >
>> > +static int __guc_bo_reinit(struct xe_guc *guc, struct xe_bo **src)
>> > +{
>> > +	struct xe_tile *tile = gt_to_tile(guc_to_gt(guc));
>> > +	struct xe_device *xe = guc_to_xe(guc);
>> > +	struct xe_bo *bo;
>> > +
>> > +	bo = xe_bo_create_from_data(xe, tile, &(*src)->vmap.vaddr, (*src)->size, ttm_bo_type_kernel,
>>
>> in general we should not access a iosys_map like that. Maybe add an
>> assert above this line:
>>
>> 	xe_assert(xe, !(*src)->vmap.is_iomem);
>
>Makes sense - I'll add the assert.
>
>>
>> > +				    XE_BO_CREATE_VRAM_IF_DGFX(tile));
>>
>
>> humn... maybe we are missing a XE_BO_CREATE_VRAM(tile)?
>
>It's only called for "IS_DGFX", so I think we're fine for now.

but that was the point... checking IF_DGFX() everywhere, including
code paths we should only be at for dgfx seems wasteful. Anyway, this is
independent from this patch and something that can be done later.

Lucas De Marchi

>
>Thanks,
>-Michał
>
>>
>> Lucas De Marchi
>>
>> > +	if (IS_ERR(bo))
>> > +		return PTR_ERR(bo);
>> > +
>> > +	xchg(src, bo);
>> > +	xe_bo_unpin_map_no_vm(bo);
>> > +
>> > +	return 0;
>> > +}
>> > +
>> > +static int xe_guc_realloc_post_hwconfig(struct xe_guc *guc)
>> > +{
>> > +	int ret;
>> > +
>> > +	if (!IS_DGFX(guc_to_xe(guc)))
>> > +		return 0;
>> > +
>> > +	ret = __guc_bo_reinit(guc, &guc->log.bo);
>> > +	if (ret)
>> > +		return ret;
>> > +
>> > +	ret = __guc_bo_reinit(guc, &guc->ads.bo);
>> > +	if (ret)
>> > +		return ret;
>> > +
>> > +	ret = __guc_bo_reinit(guc, &guc->ct.bo);
>> > +	if (ret)
>> > +		return ret;
>> > +
>> > +	return 0;
>> > +}
>> > +
>> > int xe_guc_init(struct xe_guc *guc)
>> > {
>> > 	struct xe_device *xe = guc_to_xe(guc);
>> > @@ -309,6 +348,12 @@ int xe_guc_init(struct xe_guc *guc)
>> >  */
>> > int xe_guc_init_post_hwconfig(struct xe_guc *guc)
>> > {
>> > +	int ret;
>> > +
>> > +	ret = xe_guc_realloc_post_hwconfig(guc);
>> > +	if (ret)
>> > +		return ret;
>> > +
>> > 	guc_init_params_post_hwconfig(guc);
>> >
>> > 	return xe_guc_ads_init_post_hwconfig(&guc->ads);
>> > diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c
>> > index 88789826e7817..88514d09303d0 100644
>> > --- a/drivers/gpu/drm/xe/xe_guc_ads.c
>> > +++ b/drivers/gpu/drm/xe/xe_guc_ads.c
>> > @@ -282,7 +282,7 @@ int xe_guc_ads_init(struct xe_guc_ads *ads)
>> > 	bo = xe_bo_create_pin_map(xe, tile, NULL, guc_ads_size(ads) +
>> > 				  MAX_GOLDEN_LRC_SIZE,
>> > 				  ttm_bo_type_kernel,
>> > -				  XE_BO_CREATE_VRAM_IF_DGFX(tile) |
>> > +				  XE_BO_CREATE_SYSTEM_BIT |
>> > 				  XE_BO_CREATE_GGTT_BIT);
>> > 	if (IS_ERR(bo))
>> > 		return PTR_ERR(bo);
>> > diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
>> > index a84e111bb36ad..8f190cc0e5527 100644
>> > --- a/drivers/gpu/drm/xe/xe_guc_ct.c
>> > +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
>> > @@ -148,7 +148,7 @@ int xe_guc_ct_init(struct xe_guc_ct *ct)
>> >
>> > 	bo = xe_bo_create_pin_map(xe, tile, NULL, guc_ct_size(),
>> > 				  ttm_bo_type_kernel,
>> > -				  XE_BO_CREATE_VRAM_IF_DGFX(tile) |
>> > +				  XE_BO_CREATE_SYSTEM_BIT |
>> > 				  XE_BO_CREATE_GGTT_BIT);
>> > 	if (IS_ERR(bo))
>> > 		return PTR_ERR(bo);
>> > diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c
>> > index 45c60a9c631c3..6334311a56e63 100644
>> > --- a/drivers/gpu/drm/xe/xe_guc_log.c
>> > +++ b/drivers/gpu/drm/xe/xe_guc_log.c
>> > @@ -93,7 +93,7 @@ int xe_guc_log_init(struct xe_guc_log *log)
>> >
>> > 	bo = xe_bo_create_pin_map(xe, tile, NULL, guc_log_size(),
>> > 				  ttm_bo_type_kernel,
>> > -				  XE_BO_CREATE_VRAM_IF_DGFX(tile) |
>> > +				  XE_BO_CREATE_SYSTEM_BIT |
>> > 				  XE_BO_CREATE_GGTT_BIT);
>> > 	if (IS_ERR(bo))
>> > 		return PTR_ERR(bo);
>> > --
>> > 2.42.1
>> >