[Intel-gfx] [PATCH 2/2] drm/i915/guc: Don't make assumptions while getting the lrca offset
Daniele Ceraolo Spurio
daniele.ceraolospurio at intel.com
Wed Jul 12 20:07:41 UTC 2017
On 12/07/17 12:49, Chris Wilson wrote:
> Quoting Michel Thierry (2017-07-12 20:30:32)
>> Using the HWSP ggtt_offset to get the lrca offset is only correct if the
>> HWSP happens to be before it (when we reuse the PPHWSP of the kernel
>> context as the engine HWSP). Instead of making this assumption, get the
>> lrca offset from the kernel_context engine state.
>>
>> And while looking at this part of the GuC interaction, it was also
>> noticed that the firmware expects the size of only the engine context
>> (context minus the execlist part, i.e. don't include the first 80
>> dwords), so pass the right size.
>>
>> v2: Use the new macros to prevent abusive overuse of the old ones (Chris).
>>
>> Reported-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>> Signed-off-by: Michel Thierry <michel.thierry at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Oscar Mateo <oscar.mateo at intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_guc_submission.c | 21 ++++++++++++++++++---
>> 1 file changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
>> index b7ca13860677..8b96935cf96a 100644
>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>> @@ -1018,6 +1018,12 @@ static void guc_policies_init(struct guc_policies *policies)
>> policies->is_valid = 1;
>> }
>>
>> +/*
>> + * The first 80 dwords of the register state context, containing the
>> + * execlists and ppgtt registers.
>> + */
>> +#define LR_HW_CONTEXT_SIZE (80 * sizeof(u32))
>> +
>> static int guc_ads_create(struct intel_guc *guc)
>> {
>> struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> @@ -1032,6 +1038,8 @@ static int guc_ads_create(struct intel_guc *guc)
>> } __packed *blob;
>> struct intel_engine_cs *engine;
>> enum intel_engine_id id;
>> + const u32 skipped_offset = LRC_HEADER_PAGES * PAGE_SIZE;
>> + const u32 skipped_size = LRC_PPHWSP_SZ * PAGE_SIZE + LR_HW_CONTEXT_SIZE;
>
> I think I preferred the constants rather than the variables, but that's
> a matter of taste. Since you confirmed the guc works the way you
> describe and now have a big warning in place,
>
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
>
> Though I would get Daniele to confirm the interpretation.
> -Chris
>
LGTM.
Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
-Daniele
More information about the Intel-gfx
mailing list