All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Kuehling, Felix" <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
To: "Zeng, Oak" <Oak.Zeng-5C7GfCeVMHo@public.gmane.org>,
	"amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org"
	<amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Cc: "Keely, Sean" <Sean.Keely-5C7GfCeVMHo@public.gmane.org>,
	"Koenig,
	Christian" <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>
Subject: Re: [PATCH 5/8] drm/amdkfd: Add function to set queue gws
Date: Fri, 24 May 2019 19:47:27 +0000	[thread overview]
Message-ID: <19ff1e8d-1a56-b89f-4a9b-30a17ef10655@amd.com> (raw)
In-Reply-To: <1558651263-3478-5-git-send-email-Oak.Zeng-5C7GfCeVMHo@public.gmane.org>

On 2019-05-23 6:41 p.m., Zeng, Oak wrote:
> Add functions in process queue manager to
> set/unset queue gws. Also set process's number
> of gws used. Currently only one queue in
> process can use and use all gws.
>
> Change-Id: I03e480c8692db3eabfc3a188cce8904d5d962ab7
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h              |  6 +++
>   .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 57 ++++++++++++++++++++++
>   2 files changed, 63 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 5969e37..40a5c67 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -454,6 +454,9 @@ struct queue_properties {
>    *
>    * @device: The kfd device that created this queue.
>    *
> + * @gws: Pointing to gws kgd_mem if this is a gws control queue; NULL
> + * otherwise.
> + *
>    * This structure represents user mode compute queues.
>    * It contains all the necessary data to handle such queues.
>    *
> @@ -475,6 +478,7 @@ struct queue {
>   
>   	struct kfd_process	*process;
>   	struct kfd_dev		*device;
> +	void *gws;
>   };
>   
>   /*
> @@ -868,6 +872,8 @@ int pqm_update_queue(struct process_queue_manager *pqm, unsigned int qid,
>   			struct queue_properties *p);
>   int pqm_set_cu_mask(struct process_queue_manager *pqm, unsigned int qid,
>   			struct queue_properties *p);
> +int pqm_set_gws(struct process_queue_manager *pqm, unsigned int qid,
> +			void *gws);
>   struct kernel_queue *pqm_get_kernel_queue(struct process_queue_manager *pqm,
>   						unsigned int qid);
>   int pqm_get_wave_state(struct process_queue_manager *pqm,
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> index e652e25..5764491 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> @@ -26,6 +26,7 @@
>   #include "kfd_device_queue_manager.h"
>   #include "kfd_priv.h"
>   #include "kfd_kernel_queue.h"
> +#include "amdgpu_amdkfd.h"
>   
>   static inline struct process_queue_node *get_queue_by_qid(
>   			struct process_queue_manager *pqm, unsigned int qid)
> @@ -74,6 +75,55 @@ void kfd_process_dequeue_from_device(struct kfd_process_device *pdd)
>   	pdd->already_dequeued = true;
>   }
>   
> +int pqm_set_gws(struct process_queue_manager *pqm, unsigned int qid,
> +			void *gws)
> +{
> +	struct kfd_dev *dev = NULL;
> +	struct process_queue_node *pqn;
> +	struct kfd_process_device *pdd;
> +	struct kgd_mem *mem;
> +	int ret;
> +
> +	pqn = get_queue_by_qid(pqm, qid);
> +	if (!pqn) {
> +		pr_err("Queue id does not match any known queue\n");
> +		return -EINVAL;
> +	}
> +
> +	if (pqn->q)
> +		dev = pqn->q->device;
> +	if (WARN_ON(!dev))
> +		return -ENODEV;
> +
> +	pdd = kfd_get_process_device_data(dev, pqm->process);
> +	if (!pdd) {
> +		pr_err("Process device data doesn't exist\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Only allow one queue per process can have GWS assigned */
> +	if (gws && pdd->qpd.num_gws)
> +		return -EINVAL;
> +
> +	if (!gws && pdd->qpd.num_gws == 0)
> +		return -EINVAL;
> +
> +	if (gws)
> +		ret = amdgpu_amdkfd_add_gws_to_process(pdd->process->kgd_process_info,
> +			gws, &mem);
> +	else
> +		amdgpu_amdkfd_remove_gws_from_process(pdd->process->kgd_process_info,
> +			pqn->q->gws);
> +	if (unlikely(ret))

ret may be uninitialized here. You probably get a compiler warning for 
that. Please check for compiler warnings and initialize ret at the start 
of the function.


> +		return ret;
> +
> +	pqn->q->gws = gws ? mem : NULL;

I think there is a chance that some compilers will complain here that 
mem can be used uninitialized. It may be safer to initialize mem to NULL 
at the start of the function, just in case. Then you can also just 
assign mem here without the conditional.

With these two issues fixed, this patch is Reviewed-by: Felix Kuehling 
<Felix.Kuehling@amd.com>


> +	pdd->qpd.num_gws = gws ? amdgpu_amdkfd_get_num_gws(dev->kgd) : 0;
> +
> +	return pqn->q->device->dqm->ops.update_queue(pqn->q->device->dqm,
> +							pqn->q);
> +}
> +
>   void kfd_process_dequeue_from_all_devices(struct kfd_process *p)
>   {
>   	struct kfd_process_device *pdd;
> @@ -330,6 +380,13 @@ int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid)
>   			if (retval != -ETIME)
>   				goto err_destroy_queue;
>   		}
> +
> +		if (pqn->q->gws) {
> +			amdgpu_amdkfd_remove_gws_from_process(pqm->process->kgd_process_info,
> +				pqn->q->gws);
> +			pdd->qpd.num_gws = 0;
> +		}
> +
>   		kfree(pqn->q->properties.cu_mask);
>   		pqn->q->properties.cu_mask = NULL;
>   		uninit_queue(pqn->q);
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

  parent reply	other threads:[~2019-05-24 19:47 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-23 22:41 [PATCH 1/8] drm/amdkfd: Add gws number to kfd topology node properties Zeng, Oak
     [not found] ` <1558651263-3478-1-git-send-email-Oak.Zeng-5C7GfCeVMHo@public.gmane.org>
2019-05-23 22:41   ` [PATCH 2/8] drm/amdgpu: Add interface to alloc gws from amdgpu Zeng, Oak
2019-05-23 22:41   ` [PATCH 3/8] drm/amdkfd: Allocate gws on device initialization Zeng, Oak
2019-05-23 22:41   ` [PATCH 4/8] drm/amdgpu: Add function to add/remove gws to kfd process Zeng, Oak
2019-05-23 22:41   ` [PATCH 5/8] drm/amdkfd: Add function to set queue gws Zeng, Oak
     [not found]     ` <1558651263-3478-5-git-send-email-Oak.Zeng-5C7GfCeVMHo@public.gmane.org>
2019-05-24 19:47       ` Kuehling, Felix [this message]
2019-05-23 22:41   ` [PATCH 6/8] drm/amdkfd: New IOCTL to allocate queue GWS Zeng, Oak
     [not found]     ` <1558651263-3478-6-git-send-email-Oak.Zeng-5C7GfCeVMHo@public.gmane.org>
2019-05-24 19:48       ` Kuehling, Felix
     [not found]         ` <4f49af4d-0953-030c-bb1b-47b7a5fa7c7c-5C7GfCeVMHo@public.gmane.org>
2019-05-31  3:13           ` Dave Airlie
     [not found]             ` <CAPM=9tzs9adV7_ef4hpgswB-S==iQizAF_=FTarVm3EYwPCg0w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2019-05-31 20:04               ` Kuehling, Felix
     [not found]                 ` <e857f0f4-341d-1e64-0ffe-5b6812ca5b7f-5C7GfCeVMHo@public.gmane.org>
2019-05-31 20:53                   ` Dave Airlie
     [not found]                     ` <CAPM=9tzQB_SbCHcAcs3PRq-maed9XdMroROWNQuf9DdgOH5zcQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2019-05-31 21:18                       ` Kuehling, Felix
2019-05-23 22:41   ` [PATCH 7/8] drm/amdkfd: PM4 packets change to support GWS Zeng, Oak
2019-05-23 22:41   ` [PATCH 8/8] drm/amdkfd: Use kfd fd to mmap mmio Zeng, Oak
     [not found]     ` <1558651263-3478-8-git-send-email-Oak.Zeng-5C7GfCeVMHo@public.gmane.org>
2019-05-24  8:47       ` Christian König
2019-05-24 18:59       ` Kuehling, Felix
  -- strict thread matches above, loose matches on Subject: below --
2019-05-13 18:50 [PATCH 5/8] drm/amdkfd: Add function to set queue gws Zeng, Oak
2019-05-10 16:01 [PATCH 1/8] drm/amdkfd: Add gws number to kfd topology node properties Zeng, Oak
     [not found] ` <1557504063-1559-1-git-send-email-Oak.Zeng-5C7GfCeVMHo@public.gmane.org>
2019-05-10 16:01   ` [PATCH 5/8] drm/amdkfd: Add function to set queue gws Zeng, Oak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=19ff1e8d-1a56-b89f-4a9b-30a17ef10655@amd.com \
    --to=felix.kuehling-5c7gfcevmho@public.gmane.org \
    --cc=Christian.Koenig-5C7GfCeVMHo@public.gmane.org \
    --cc=Oak.Zeng-5C7GfCeVMHo@public.gmane.org \
    --cc=Sean.Keely-5C7GfCeVMHo@public.gmane.org \
    --cc=amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
    --subject='Re: [PATCH 5/8] drm/amdkfd: Add function to set queue gws' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.