[Intel-xe] [PATCH v2 12/14] drm/xe/display: annotate CC buffers with NEEDS_CPU_ACCESS

[Maarten Lankhorst]

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.

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.

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>

~Maarten