[Intel-gfx] [PATCH] drm/i915/guc: Don't make assumptions while getting the lrca offset
Michel Thierry
michel.thierry at intel.com
Wed Jul 12 00:14:46 UTC 2017
On 7/11/2017 5:07 PM, Chris Wilson wrote:
> Quoting Chris Wilson (2017-07-12 01:00:02)
>> Also feel free to try and fixup the other confusion in intel_lrc.c.
>> Perhaps something like:
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> index b0738d2..f498aa6 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -2031,8 +2031,9 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
>>
>> context_size = round_up(engine->context_size, I915_GTT_PAGE_SIZE);
>>
>> - /* One extra page as the sharing data between driver and GuC */
>> - context_size += PAGE_SIZE * LRC_PPHWSP_PN;
>> + /* Add an extra page at the start for sharing data with the GuC */
>> + BUILD_BUG_ON(LRC_PPHWSP_PN != 1);
>> + context_size += PAGE_SIZE;
>>
>> ctx_obj = i915_gem_object_create(ctx->i915, context_size);
>> if (IS_ERR(ctx_obj)) {
>
> Or better
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index b0738d2..0b7fb38 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2031,8 +2031,8 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
>
> context_size = round_up(engine->context_size, I915_GTT_PAGE_SIZE);
>
> - /* One extra page as the sharing data between driver and GuC */
> - context_size += PAGE_SIZE * LRC_PPHWSP_PN;
> + /* Add an extra page at the start for sharing data with the GuC */
> + context_size += LRC_GUCSHR_PAGES * PAGE_SIZE;
>
> ctx_obj = i915_gem_object_create(ctx->i915, context_size);
> if (IS_ERR(ctx_obj)) {
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> index 52b3a1f..848ca21 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -72,7 +72,8 @@ int logical_xcs_ring_init(struct intel_engine_cs *engine);
>
> /* One extra page is added before LRC for GuC as shared data */
> #define LRC_GUCSHR_PN (0)
> -#define LRC_PPHWSP_PN (LRC_GUCSHR_PN + 1)
> +#define LRC_GUCSHR_PAGES 1
> +#define LRC_PPHWSP_PN (LRC_GUCSHR_PN + LRC_GUCSHR_PAGES)
> #define LRC_STATE_PN (LRC_PPHWSP_PN + 1)
>
> struct drm_i915_private;
>
> Nowhere do we use LRC_GUCSHR_PN. Having it just gives us the mirage of
> flexibility...
LRC_GUCSHR_PN is not used because people assumed it is always page 0 of
the kernel ctx :S
One more thing to fix, use LRC_GUCSHR_PN in all the places where this
_shared_ GuC page is used (guc_ggtt_offset(ctx->engine[RCS].state))
> -Chris
>
More information about the Intel-gfx
mailing list