[PATCH v2 0/3] Fix bugs related to init pipelines

[PATCH v2 0/3] Fix bugs related to init pipelines

[Oded Gabbay]

Following comments on original patch, I prepared this patch-set to address
all the issues that were brought up.

	Oded

Oded Gabbay (3):
  drm/radeon: Don't increment pipe_id in kgd_init_pipeline
  drm/amdkfd: Fix bug in pipelines initialization
  drm/amdkfd: Fix bug in call to init_pipelines()

 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 8 ++++++--
 drivers/gpu/drm/radeon/radeon_kfd.c                   | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/3] drm/radeon: Don't increment pipe_id in kgd_init_pipeline

[Oded Gabbay]

This patch fixes the behavior of kgd_init_pipeline in that this function
shouldn't automatically increase the pipe_id argument by 1 right at the start
of the function.

This is because the first_pipe value might not be always 1, and because a
proper interface function should not hide this info inside its implementation.
In other words, the calling function should provide the real pipe_id and not
count on kgd_init_pipeline to "fix" it.

Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>
---
 drivers/gpu/drm/radeon/radeon_kfd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_kfd.c b/drivers/gpu/drm/radeon/radeon_kfd.c
index 8bf87f1..bef9a09 100644
--- a/drivers/gpu/drm/radeon/radeon_kfd.c
+++ b/drivers/gpu/drm/radeon/radeon_kfd.c
@@ -436,7 +436,7 @@ static int kgd_init_memory(struct kgd_dev *kgd)
 static int kgd_init_pipeline(struct kgd_dev *kgd, uint32_t pipe_id,
 				uint32_t hpd_size, uint64_t hpd_gpu_addr)
 {
-	uint32_t mec = (++pipe_id / CIK_PIPE_PER_MEC) + 1;
+	uint32_t mec = (pipe_id / CIK_PIPE_PER_MEC) + 1;
 	uint32_t pipe = (pipe_id % CIK_PIPE_PER_MEC);
 
 	lock_srbm(kgd, mec, pipe, 0, 0);
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/3] drm/amdkfd: Fix bug in pipelines initialization

[Oded Gabbay]

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 | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

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..cd0710a 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,14 @@ static int init_pipelines(struct device_queue_manager *dqm,
 
 	for (i = 0; i < pipes_num; i++) {
 		inx = i + first_pipe;
+		/*
+		 * HPD buffer on GTT is allocated by amdkfd, no need to waste
+		 * space in GTT for pipelines we don't initialize
+		 */
 		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 */
-		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

[PATCH v2 3/3] drm/amdkfd: Fix bug in call to init_pipelines()

[Oded Gabbay]

This patch fixes a bug where the first_pipe index passed into init_pipelines()
was a #define instead of the value that is passed into amdkfd by radeon

Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 cd0710a..0d8694f 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -588,7 +588,7 @@ static int init_scheduler(struct device_queue_manager *dqm)
 
 	pr_debug("kfd: In %s\n", __func__);
 
-	retval = init_pipelines(dqm, get_pipes_num(dqm), KFD_DQM_FIRST_PIPE);
+	retval = init_pipelines(dqm, get_pipes_num(dqm), get_first_pipe(dqm));
 	if (retval != 0)
 		return retval;
 
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/3] drm/amdkfd: Fix bug in call to init_pipelines()

[Alex Deucher]

On Thu, Jan 22, 2015 at 5:59 AM, Oded Gabbay <oded.gabbay@amd.com> wrote:
> This patch fixes a bug where the first_pipe index passed into init_pipelines()
> was a #define instead of the value that is passed into amdkfd by radeon
>
> Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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 cd0710a..0d8694f 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -588,7 +588,7 @@ static int init_scheduler(struct device_queue_manager *dqm)
>
>         pr_debug("kfd: In %s\n", __func__);
>
> -       retval = init_pipelines(dqm, get_pipes_num(dqm), KFD_DQM_FIRST_PIPE);
> +       retval = init_pipelines(dqm, get_pipes_num(dqm), get_first_pipe(dqm));
>         if (retval != 0)
>                 return retval;
>
> --
> 1.9.1
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/3] drm/amdkfd: Fix bug in pipelines initialization

[Alex Deucher]

On Thu, Jan 22, 2015 at 5:59 AM, Oded Gabbay <oded.gabbay@amd.com> wrote:
> 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>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> 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..cd0710a 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,14 @@ static int init_pipelines(struct device_queue_manager *dqm,
>
>         for (i = 0; i < pipes_num; i++) {
>                 inx = i + first_pipe;
> +               /*
> +                * HPD buffer on GTT is allocated by amdkfd, no need to waste
> +                * space in GTT for pipelines we don't initialize
> +                */
>                 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 */
> -               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

Re: [PATCH v2 1/3] drm/radeon: Don't increment pipe_id in kgd_init_pipeline

[Alex Deucher]

On Thu, Jan 22, 2015 at 5:59 AM, Oded Gabbay <oded.gabbay@amd.com> wrote:
> This patch fixes the behavior of kgd_init_pipeline in that this function
> shouldn't automatically increase the pipe_id argument by 1 right at the start
> of the function.
>
> This is because the first_pipe value might not be always 1, and because a
> proper interface function should not hide this info inside its implementation.
> In other words, the calling function should provide the real pipe_id and not
> count on kgd_init_pipeline to "fix" it.
>
> Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/radeon/radeon_kfd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_kfd.c b/drivers/gpu/drm/radeon/radeon_kfd.c
> index 8bf87f1..bef9a09 100644
> --- a/drivers/gpu/drm/radeon/radeon_kfd.c
> +++ b/drivers/gpu/drm/radeon/radeon_kfd.c
> @@ -436,7 +436,7 @@ static int kgd_init_memory(struct kgd_dev *kgd)
>  static int kgd_init_pipeline(struct kgd_dev *kgd, uint32_t pipe_id,
>                                 uint32_t hpd_size, uint64_t hpd_gpu_addr)
>  {
> -       uint32_t mec = (++pipe_id / CIK_PIPE_PER_MEC) + 1;
> +       uint32_t mec = (pipe_id / CIK_PIPE_PER_MEC) + 1;
>         uint32_t pipe = (pipe_id % CIK_PIPE_PER_MEC);
>
>         lock_srbm(kgd, mec, pipe, 0, 0);
> --
> 1.9.1
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH v2 0/3] Fix bugs related to init pipelines

[Zhou, Jammy]

The series are Reviewed-by: Jammy Zhou <Jammy.Zhou@amd.com>

Regards,
Jammy

-----Original Message-----
From: Gabbay, Oded 
Sent: Thursday, January 22, 2015 6:59 PM
To: dri-devel@lists.freedesktop.org; Zhou, Jammy
Cc: alexdeucher@gmail.com
Subject: [PATCH v2 0/3] Fix bugs related to init pipelines

Following comments on original patch, I prepared this patch-set to address all the issues that were brought up.

	Oded

Oded Gabbay (3):
  drm/radeon: Don't increment pipe_id in kgd_init_pipeline
  drm/amdkfd: Fix bug in pipelines initialization
  drm/amdkfd: Fix bug in call to init_pipelines()

 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 8 ++++++--
 drivers/gpu/drm/radeon/radeon_kfd.c                   | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

--
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel