[Intel-gfx] [PATCH] drm/i915/dpt: Use shmem for dpt objects

[Tvrtko Ursulin]

On 18/07/2023 06:33, Radhakrishna Sripada wrote:
> Dpt objects that are created from internal get evicted when there is
> memory pressure and do not get restored when pinned during scanout. The
> pinned page table entries look corrupted and programming the display
> engine with the incorrect pte's result in DE throwing pipe faults.
> 
> Create DPT objects from shmem and mark the object as dirty when pinning so
> that the object is restored when shrinker evicts an unpinned buffer object.
> 
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>

Fixes: 0dc987b699ce ("drm/i915/display: Add smem fallback allocation for dpt")
Cc: <stable at vger.kernel.org> # v6.0+

Not sure which platforms it actually applies so just mentioning to pick the right one.

> Suggested-by: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Fei Yang <fei.yang at intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dpt.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c
> index 7c5fddb203ba..a57d18550a46 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpt.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpt.c
> @@ -166,6 +166,9 @@ struct i915_vma *intel_dpt_pin(struct i915_address_space *vm)
>   		i915_vma_get(vma);
>   	}
>   
> +	if (i915_gem_object_is_shmem(dpt->obj))
> +		dpt->obj->cache_dirty = true;

GPU writes to this object behind the covers or what is supposed to be the purpose of this?

> +
>   	atomic_dec(&i915->gpu_error.pending_fb_pin);
>   	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
>   
> @@ -261,7 +264,7 @@ intel_dpt_create(struct intel_framebuffer *fb)
>   		dpt_obj = i915_gem_object_create_stolen(i915, size);
>   	if (IS_ERR(dpt_obj) && !HAS_LMEM(i915)) {
>   		drm_dbg_kms(&i915->drm, "Allocating dpt from smem\n");
> -		dpt_obj = i915_gem_object_create_internal(i915, size);
> +		dpt_obj = i915_gem_object_create_shmem(i915, size);

Yeah it "says on the tin" with internal objects that content is not preserved across re-pinning.

As a disclaimer,  I am not familiar with the lifetime of the DPT objects and their content. But the statement that page table entries are corrupt sounds worrying, like there is more to it than the backing store type.

Also, there is "*make_unshrinkable*" pair of functions which could be used. But again that would be about object content, not corrupt PTEs.. Unless with corrupt what is meant is stale? Maybe put a poison into dpt_clear_range to see if unpinned range is what the display engine will hit?
  
Regards,

Tvrtko

>   	}
>   	if (IS_ERR(dpt_obj))
>   		return ERR_CAST(dpt_obj);