[PATCH 1/2] drm/xe: Fix DSB buffer coherency

[Maarten Lankhorst]

Den 2024-09-13 kl. 19:12, skrev Ville Syrjälä:
> On Fri, Sep 13, 2024 at 01:47:53PM +0200, Maarten Lankhorst wrote:
>> Add the scanout flag to force WC caching, and add the memory barrier
>> where needed.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> ---
>>  drivers/gpu/drm/xe/display/xe_dsb_buffer.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/display/xe_dsb_buffer.c b/drivers/gpu/drm/xe/display/xe_dsb_buffer.c
>> index f99d901a3214f..f7949bf5426af 100644
>> --- a/drivers/gpu/drm/xe/display/xe_dsb_buffer.c
>> +++ b/drivers/gpu/drm/xe/display/xe_dsb_buffer.c
>> @@ -48,11 +48,12 @@ bool intel_dsb_buffer_create(struct intel_crtc *crtc, struct intel_dsb_buffer *d
>>  	if (!vma)
>>  		return false;
>>  
>> +	/* Set scanout flag for WC mapping */
>>  	obj = xe_bo_create_pin_map(xe, xe_device_get_root_tile(xe),
>>  				   NULL, PAGE_ALIGN(size),
>>  				   ttm_bo_type_kernel,
>>  				   XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) |
>> -				   XE_BO_FLAG_GGTT);
>> +				   XE_BO_FLAG_SCANOUT | XE_BO_FLAG_GGTT);
>>  	if (IS_ERR(obj)) {
>>  		kfree(vma);
>>  		return false;
>> @@ -73,5 +74,5 @@ void intel_dsb_buffer_cleanup(struct intel_dsb_buffer *dsb_buf)
>>  
>>  void intel_dsb_buffer_flush_map(struct intel_dsb_buffer *dsb_buf)
>>  {
>> -	/* TODO: add xe specific flush_map() for dsb buffer object. */
>> +	xe_device_wmb(dsb_buf->vma->bo->tile->xe);
> 
> MMIO itself should be sufficient to flush the WC buffer.
> But I guess no real harm in hammering it a bit harder.

You would say that, I still saw a spurious DSB timeout without the flush. :)

"Memory mapped I/O usually takes place through memory locations that are part of
a window in the CPU's memory space that has different properties assigned than
the usual RAM directed window.

Amongst these properties is usually the fact that such accesses bypass the
caching entirely and go directly to the device buses.  This means MMIO accesses
may, in effect, overtake accesses to cached memory that were emitted earlier."

Since the memory is write combined, the memory barrier itself is sufficient
and no further invalidation is required.

Just the workaround should be fine. The l2 flush should be moved after the mb as well.
I'll do that in a followup patch.

>>  }
>> -- 
>> 2.45.2
>