That looks like it would, but I think it's still confusing to reference LRC state when we haven't initialized execlists at all...
Jesse
On 09/10/2015 02:56 PM, Yu Dai wrote:
> Jesse,
>
> Will the patch here fix the issue? It should help other cases where LRC_PPHWSP_PN is referenced on non-execlist / guc platforms.
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> index 4cc54b3..233a930 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -71,7 +71,7 @@ static inline void intel_logical_ring_emit(struct intel_ringbuffer *ringbuf,
>
> /* 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_PPHWSP_PN (LRC_GUCSHR_PN + i915.enable_guc_submission ? 1 : 0)
> #define LRC_STATE_PN (LRC_PPHWSP_PN + 1)
>
> void intel_lr_context_free(struct intel_context *ctx);
>
> Thanks,
> Alex
>
> On 09/10/2015 02:40 PM, Jesse Barnes wrote:
>> Looks like this was introduced in:
>> commit d1675198ed1f21aec6e036336e4340c40b726497
>> Author: Alex Dai <yu.dai@xxxxxxxxx>
>> Date: Wed Aug 12 15:43:43 2015 +0100
>>
>> drm/i915: Integrate GuC-based command submission
>>
>> This patch assumed LRC contexts and HWS layout, which is incorrect on
>> platforms without execlists. This can lead to a crash in GPU error
>> state readout on those platforms.
>>
>> I don't see a bug filed for this, but there may be one that I haven't
>> found.
>>
>> Cc: Alex Dai <yu.dai@xxxxxxxxx>
>> Signed-off-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/i915_gpu_error.c | 19 +++++++++++++------
>> 1 file changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
>> index 3379f9c..d0822f8 100644
>> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
>> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
>> @@ -457,17 +457,24 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
>> }
>> if ((obj = error->ring[i].hws_page)) {
>> + u64 hws_offset = lower_32_bits(obj->gtt_offset);
>> + u32 *hws_page = &obj->pages[0][0];
>> +
>> + if (i915.enable_execlists) {
>> + hws_offset = obj->gtt_offset + LRC_PPHWSP_PN *
>> + PAGE_SIZE;
>> + hws_page = &obj->pages[LRC_PPHWSP_PN][0];
>> + }
>> err_printf(m, "%s --- HW Status = 0x%08llx\n",
>> - dev_priv->ring[i].name,
>> - obj->gtt_offset + LRC_PPHWSP_PN * PAGE_SIZE);
>> + dev_priv->ring[i].name, hws_offset);
>> offset = 0;
>> for (elt = 0; elt < PAGE_SIZE/16; elt += 4) {
>> err_printf(m, "[%04x] %08x %08x %08x %08x\n",
>> offset,
>> - obj->pages[LRC_PPHWSP_PN][elt],
>> - obj->pages[LRC_PPHWSP_PN][elt+1],
>> - obj->pages[LRC_PPHWSP_PN][elt+2],
>> - obj->pages[LRC_PPHWSP_PN][elt+3]);
>> + hws_page[elt],
>> + hws_page[elt+1],
>> + hws_page[elt+2],
>> + hws_page[elt+3]);
>> offset += 16;
>> }
>> }
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
