Re: [PATCH 03/83] drm/radeon: Report doorbell configuration to kfd

[Jerome Glisse <j.glisse@gmail.com>]

On Fri, Jul 11, 2014 at 12:50:03AM +0300, Oded Gabbay wrote:
> Radeon and KFD share the doorbell aperture.
> Radeon sets it up, takes the doorbells required for its own rings
> and reports the setup to KFD.
> Radeon reserved doorbells are at the start of the doorbell aperture.
> 
> Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>

I would need some refreshing on doorbell. You want to map it to userspace
but at the same time it is use by the radeon kernel driver when dispatching
on the compute ring (iirc the gfx ring does not use it).

So now my worry is, given usermapping is done on page granularity, what
would block one process from writting to another process doorbell ? Again
iirc the doorbell is actualy the wptr for the ring buffer associated with
said doorbell (thought i forget how doorbell are associated with a ring).

This sounds really bad.

Cheers,
Jérôme

> ---
>  drivers/gpu/drm/radeon/radeon.h        |  4 ++++
>  drivers/gpu/drm/radeon/radeon_device.c | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 7cda75d..4e7e41f 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -676,6 +676,10 @@ struct radeon_doorbell {
>  
>  int radeon_doorbell_get(struct radeon_device *rdev, u32 *page);
>  void radeon_doorbell_free(struct radeon_device *rdev, u32 doorbell);
> +void radeon_doorbell_get_kfd_info(struct radeon_device *rdev,
> +				  phys_addr_t *aperture_base,
> +				  size_t *aperture_size,
> +				  size_t *start_offset);
>  
>  /*
>   * IRQS.
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> index 03686fa..98538d2 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -328,6 +328,37 @@ void radeon_doorbell_free(struct radeon_device *rdev, u32 doorbell)
>  		__clear_bit(doorbell, rdev->doorbell.used);
>  }
>  
> +/**
> + * radeon_doorbell_get_kfd_info - Report doorbell configuration required to
> + *                                setup KFD
> + *
> + * @rdev: radeon_device pointer
> + * @aperture_base: output returning doorbell aperture base physical address
> + * @aperture_size: output returning doorbell aperture size in bytes
> + * @start_offset: output returning # of doorbell bytes reserved for radeon.
> + *
> + * Radeon and the KFD share the doorbell aperture. Radeon sets it up,
> + * takes doorbells required for its own rings and reports the setup to KFD.
> + * Radeon reserved doorbells are at the start of the doorbell aperture.
> + */
> +void radeon_doorbell_get_kfd_info(struct radeon_device *rdev,
> +				  phys_addr_t *aperture_base,
> +				  size_t *aperture_size,
> +				  size_t *start_offset)
> +{
> +	/* The first num_doorbells are used by radeon.
> +	 * KFD takes whatever's left in the aperture. */
> +	if (rdev->doorbell.size > rdev->doorbell.num_doorbells * sizeof(u32)) {
> +		*aperture_base = rdev->doorbell.base;
> +		*aperture_size = rdev->doorbell.size;
> +		*start_offset = rdev->doorbell.num_doorbells * sizeof(u32);
> +	} else {
> +		*aperture_base = 0;
> +		*aperture_size = 0;
> +		*start_offset = 0;
> +	}
> +}
> +
>  /*
>   * radeon_wb_*()
>   * Writeback is the the method by which the the GPU updates special pages
> -- 
> 1.9.1
>