Re: [PATCH v2] drm/xe/vram: fix ccs offset calculation

[Matthew Auld <matthew.auld@intel.com>]

On 15/09/2024 00:29, Lin, Shuicheng wrote:
>>> Spec says SW is expected to round up to the nearest 128K, if not
>>> already aligned for the CC unit view of CCS. We are seeing the assert
>>> sometimes pop on BMG to tell us that there is a hole between GSM and
>>> CCS, as well as popping other asserts with having a vram size with
>>> strange alignment, which is likely caused by misaligned offset here.
>>>
>>> BSpec: 68023
>>> Fixes: b5c2ca0372dc ("drm/xe/xe2hpg: Determine flat ccs offset for
>>> vram")
>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>>> Cc: Akshata Jahagirdar <akshata.jahagirdar@intel.com>
>>> Cc: Shuicheng Lin <shuicheng.lin@intel.com>
>>> Cc: Matt Roper <matthew.d.roper@intel.com>
>>> Cc: <stable@vger.kernel.org> # v6.10+
>>> ---
>>>   drivers/gpu/drm/xe/xe_vram.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_vram.c
>>> b/drivers/gpu/drm/xe/xe_vram.c index
>>> 7e765b1499b1..8e65cb4cc477 100644
>>> --- a/drivers/gpu/drm/xe/xe_vram.c
>>> +++ b/drivers/gpu/drm/xe/xe_vram.c
>>> @@ -181,6 +181,7 @@ static inline u64 get_flat_ccs_offset(struct xe_gt
>>> *gt, u64
>>> tile_size)
>>>
>>>   		offset = offset_hi << 32; /* HW view bits 39:32 */
>>>   		offset |= offset_lo << 6; /* HW view bits 31:6 */
>>> +		offset = round_up(offset, SZ_128K); /* SW must round up to
>>> nearest
>>> +128K */
>>>   		offset *= num_enabled; /* convert to SW view */
> f> >
>>>   		/* We don't expect any holes */
>>> --
>>> 2.46.0
>>
>> The patch works in my platform.
>> Tested-by: Shuicheng Lin <shuicheng.lin@intel.com>
> The round up should be applied to the SW address. So, the right sequence should be as below:
>    		offset *= num_enabled; /* convert to SW view */
> +		offset = round_up(offset, SZ_128K); /* SW must round up to nearest +128K */
> 
> I applied the patch manually and didn't notice the sequence difference. With upper sequence, the patch could fix the misaligned offset issue.

Ok, will move this. Thanks for testing.