[Intel-xe] [PATCH v2 12/14] drm/xe/display: annotate CC buffers with NEEDS_CPU_ACCESS
Matthew Auld
matthew.auld at intel.com
Fri Mar 3 12:12:35 UTC 2023
On 02/03/2023 11:51, Maarten Lankhorst wrote:
> Hey,
>
> On 2023-02-28 16:20, Matthew Auld wrote:
>> On 28/02/2023 14:48, Maarten Lankhorst wrote:
>>> Hey,
>>>
>>> On 2023-02-28 11:41, Matthew Auld wrote:
>>>> The display code wants to read the clear color value from the buffer.
>>>> However if the buffer is the non-mappable part of lmem then we fail the
>>>> kmap. The simplest solution is to just mark the buffer with
>>>> XE_BO_NEEDS_CPU_ACCESS, which will either allocate the buffer in the
>>>> mappable part of lmem, or migrate it there.
>>>>
>>>> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
>>>> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>>>> ---
>>>> drivers/gpu/drm/xe/display/xe_fb_pin.c | 8 ++++++++
>>>> 1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c
>>>> b/drivers/gpu/drm/xe/display/xe_fb_pin.c
>>>> index 65c0bc28a3d1..66e1309e21d8 100644
>>>> --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
>>>> +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
>>>> @@ -203,6 +203,14 @@ static struct i915_vma *__xe_pin_fb_vma(struct
>>>> intel_framebuffer *fb,
>>>> if (ret)
>>>> goto err;
>>>> + /*
>>>> + * For this type of buffer we need to able to read from the CPU
>>>> the
>>>> + * clear color value found in the buffer. This doesn't do
>>>> anything on
>>>> + * non small-bar devices.
>>>> + */
>>>> + if (intel_fb_rc_ccs_cc_plane(&fb->base) >= 0)
>>>> + bo->flags |= XE_BO_NEEDS_CPU_ACCESS;
>>>
>>> While we do need a change, do we need to do this inside pinning? We
>>> could also require userspace to create VRAM BO's with CPU_ACCESS, and
>>> reject CCS here.
>>
>> Do you mean reject CCS if CPU_ACCESS is not set? The trouble is that
>> CPU_ACCESS also requires system memory as a spill option when
>> specifying the placements, which then prevents using CCS. Or at least
>> that's how it was in i915.
> We should be able to handle it without moving to sysmem, if that's what
> we need.
Ok, so maybe drop NEEDS_CPU_ACCESS, and just have NEEDS_VISIBLE_VRAM
instead, and to get the old behaviour the user can just attach system
memory as an extra placement? And then for clear color stuff they can
use VISIBLE_VRAM by itself, and we can enforce that here?
>>>
>>> Of course we should probably also add a UAPI to allow setting the
>>> SCANOUT flag on externally imported DMA-BUF bo's, i don't think we
>>> require CPU_ACCESS flag for that as it's not our BO.
For discrete specifically, can you have externally imported dma-buf for
display (not originating from the same device)? The scanout needs vram
local to the device, right?
>>
>> Hmm, maybe if SCANOUT then internally use the mappable part of lmem by
>> default? For dumb buffers I think it does something like that. And
>> keep the above as a fallback?
> For dumb bo's, it is expected they are mapped, so we can set the
> CPU_ACCESS flag. I don't know if we need it all the time, so for VRAM
> bo's it could be specified as a separate flag, which is ignored for sysmem.
>>
>>>
>>> Might require some more thinking on how we want to handle this.
>>>
>>> Other patches look good, except for the comments I had:
>>>
>>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>>
More information about the Intel-xe
mailing list