This patch fixes a bug when calling to init_pipeline() interface. The index that was passed to that function didn't take into account the first_pipe value, which represents the first pipe index that is under amdkfd's responsibility.
Signed-off-by: Oded Gabbay oded.gabbay@amd.com --- drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c index b9626ae..fbb353f 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c @@ -565,10 +565,10 @@ static int init_pipelines(struct device_queue_manager *dqm,
for (i = 0; i < pipes_num; i++) { inx = i + first_pipe; - pipe_hpd_addr = dqm->pipelines_addr + i * CIK_HPD_EOP_BYTES; + pipe_hpd_addr = dqm->pipelines_addr + inx * CIK_HPD_EOP_BYTES; pr_debug("kfd: pipeline address %llX\n", pipe_hpd_addr); /* = log2(bytes/4)-1 */ - kfd2kgd->init_pipeline(dqm->dev->kgd, i, + kfd2kgd->init_pipeline(dqm->dev->kgd, inx, CIK_HPD_EOP_BYTES_LOG2 - 3, pipe_hpd_addr); }
Hi Oded,
- pipe_hpd_addr = dqm->pipelines_addr + i * CIK_HPD_EOP_BYTES; + pipe_hpd_addr = dqm->pipelines_addr + inx * CIK_HPD_EOP_BYTES; I think 'i' should still be used here, because it is the real index in the buffer
Besides, for the code below in init_scheduler(), it looks like the KFD_DQM_FIRST_PIPE is not correct, and we probably need to use get_first_pipe(dqm) instead.
retval = init_pipelines(dqm, get_pipes_num(dqm), KFD_DQM_FIRST_PIPE);
Regards, Jammy
-----Original Message----- From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of Gabbay, Oded Sent: Thursday, January 22, 2015 5:07 PM To: dri-devel@lists.freedesktop.org Subject: [PATCH] drm/amdkfd: Fix bug in pipelines initialization
This patch fixes a bug when calling to init_pipeline() interface. The index that was passed to that function didn't take into account the first_pipe value, which represents the first pipe index that is under amdkfd's responsibility.
Signed-off-by: Oded Gabbay oded.gabbay@amd.com --- drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c index b9626ae..fbb353f 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c @@ -565,10 +565,10 @@ static int init_pipelines(struct device_queue_manager *dqm,
for (i = 0; i < pipes_num; i++) { inx = i + first_pipe; - pipe_hpd_addr = dqm->pipelines_addr + i * CIK_HPD_EOP_BYTES; + pipe_hpd_addr = dqm->pipelines_addr + inx * CIK_HPD_EOP_BYTES; pr_debug("kfd: pipeline address %llX\n", pipe_hpd_addr); /* = log2(bytes/4)-1 */ - kfd2kgd->init_pipeline(dqm->dev->kgd, i, + kfd2kgd->init_pipeline(dqm->dev->kgd, inx, CIK_HPD_EOP_BYTES_LOG2 - 3, pipe_hpd_addr); }
-- 1.9.1
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On 01/22/2015 12:22 PM, Zhou, Jammy wrote:
Hi Oded,
pipe_hpd_addr = dqm->pipelines_addr + i * CIK_HPD_EOP_BYTES;
pipe_hpd_addr = dqm->pipelines_addr + inx * CIK_HPD_EOP_BYTES;
I think 'i' should still be used here, because it is the real index in the buffer
I guess that 'i' can still be used here, because we allocate the HPD buffer in GART (which pipelines_addr points to). I'll change that. However, this made me re-check the entire call path, and I found that in the init_pipeline interface function we automatically increase the pipe_id parameter (which is 'inx' here) by 1. That is definitely wrong on all levels, as first_pipe might not be 1 and also we should pass the correct pipe_id to the function. So I'm going to fix that as well.
Besides, for the code below in init_scheduler(), it looks like the KFD_DQM_FIRST_PIPE is not correct, and we probably need to use get_first_pipe(dqm) instead.
retval = init_pipelines(dqm, get_pipes_num(dqm), KFD_DQM_FIRST_PIPE);
Yes, it seems you are correct. I'll fix it.
Thanks for the comments. Oded
Regards, Jammy
-----Original Message----- From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of Gabbay, Oded Sent: Thursday, January 22, 2015 5:07 PM To: dri-devel@lists.freedesktop.org Subject: [PATCH] drm/amdkfd: Fix bug in pipelines initialization
This patch fixes a bug when calling to init_pipeline() interface. The index that was passed to that function didn't take into account the first_pipe value, which represents the first pipe index that is under amdkfd's responsibility.
Signed-off-by: Oded Gabbay oded.gabbay@amd.com
drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c index b9626ae..fbb353f 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c @@ -565,10 +565,10 @@ static int init_pipelines(struct device_queue_manager *dqm,
for (i = 0; i < pipes_num; i++) { inx = i + first_pipe;
pipe_hpd_addr = dqm->pipelines_addr + i * CIK_HPD_EOP_BYTES;
pr_debug("kfd: pipeline address %llX\n", pipe_hpd_addr); /* = log2(bytes/4)-1 */pipe_hpd_addr = dqm->pipelines_addr + inx * CIK_HPD_EOP_BYTES;
kfd2kgd->init_pipeline(dqm->dev->kgd, i,
}kfd2kgd->init_pipeline(dqm->dev->kgd, inx, CIK_HPD_EOP_BYTES_LOG2 - 3, pipe_hpd_addr);
-- 1.9.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org