diff mbox

[1/7] drm/ttm: don't wait for BO on initial allocation

Message ID 1459933928-4663-2-git-send-email-deathsimple@vodafone.de (mailing list archive)
State New, archived
Headers show

Commit Message

Christian König April 6, 2016, 9:12 a.m. UTC
From: Christian König <christian.koenig@amd.com>

When we use an extern reservation object that otherwise waits for every
fence registered with it.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

Sinclair Yeh April 6, 2016, 4:06 p.m. UTC | #1
I don't know much about AMD gpu.  Patches 1-6 look good to me.


On Wed, Apr 06, 2016 at 11:12:02AM +0200, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
> 
> When we use an extern reservation object that otherwise waits for every
> fence registered with it.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/ttm/ttm_bo.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 4cbf265..367b87b 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -998,13 +998,19 @@ static int ttm_bo_move_buffer(struct ttm_buffer_object *bo,
>  	lockdep_assert_held(&bo->resv->lock.base);
>  
>  	/*
> -	 * FIXME: It's possible to pipeline buffer moves.
> -	 * Have the driver move function wait for idle when necessary,
> -	 * instead of doing it here.
> +	 * Don't wait for the BO on initial allocation. This is important when
> +	 * the BO has an imported reservation object.
>  	 */
> -	ret = ttm_bo_wait(bo, false, interruptible, no_wait_gpu);
> -	if (ret)
> -		return ret;
> +	if (bo->mem.mem_type != TTM_PL_SYSTEM || bo->ttm != NULL) {
> +		/*
> +		 * FIXME: It's possible to pipeline buffer moves.
> +		 * Have the driver move function wait for idle when necessary,
> +		 * instead of doing it here.
> +		 */
> +		ret = ttm_bo_wait(bo, false, interruptible, no_wait_gpu);
> +		if (ret)
> +			return ret;
> +	}
>  	mem.num_pages = bo->num_pages;
>  	mem.size = mem.num_pages << PAGE_SHIFT;
>  	mem.page_alignment = bo->mem.page_alignment;
> -- 
> 2.5.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Christian König April 13, 2016, 5:38 p.m. UTC | #2
Am 06.04.2016 um 18:06 schrieb Sinclair Yeh:
> I don't know much about AMD gpu.  Patches 1-6 look good to me.

Does that count as a Reviewed-by or at least Acked-by?

Thanks for taking a look,
Christian.

>
>
> On Wed, Apr 06, 2016 at 11:12:02AM +0200, Christian König wrote:
>> From: Christian König <christian.koenig@amd.com>
>>
>> When we use an extern reservation object that otherwise waits for every
>> fence registered with it.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>>   drivers/gpu/drm/ttm/ttm_bo.c | 18 ++++++++++++------
>>   1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 4cbf265..367b87b 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -998,13 +998,19 @@ static int ttm_bo_move_buffer(struct ttm_buffer_object *bo,
>>   	lockdep_assert_held(&bo->resv->lock.base);
>>   
>>   	/*
>> -	 * FIXME: It's possible to pipeline buffer moves.
>> -	 * Have the driver move function wait for idle when necessary,
>> -	 * instead of doing it here.
>> +	 * Don't wait for the BO on initial allocation. This is important when
>> +	 * the BO has an imported reservation object.
>>   	 */
>> -	ret = ttm_bo_wait(bo, false, interruptible, no_wait_gpu);
>> -	if (ret)
>> -		return ret;
>> +	if (bo->mem.mem_type != TTM_PL_SYSTEM || bo->ttm != NULL) {
>> +		/*
>> +		 * FIXME: It's possible to pipeline buffer moves.
>> +		 * Have the driver move function wait for idle when necessary,
>> +		 * instead of doing it here.
>> +		 */
>> +		ret = ttm_bo_wait(bo, false, interruptible, no_wait_gpu);
>> +		if (ret)
>> +			return ret;
>> +	}
>>   	mem.num_pages = bo->num_pages;
>>   	mem.size = mem.num_pages << PAGE_SHIFT;
>>   	mem.page_alignment = bo->mem.page_alignment;
>> -- 
>> 2.5.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Sinclair Yeh April 13, 2016, 5:51 p.m. UTC | #3
rb for 1-6.

I can't really comment on 7.

On Wed, Apr 13, 2016 at 07:38:22PM +0200, Christian König wrote:
> Am 06.04.2016 um 18:06 schrieb Sinclair Yeh:
> >I don't know much about AMD gpu.  Patches 1-6 look good to me.
> 
> Does that count as a Reviewed-by or at least Acked-by?
> 
> Thanks for taking a look,
> Christian.
> 
> >
> >
> >On Wed, Apr 06, 2016 at 11:12:02AM +0200, Christian König wrote:
> >>From: Christian König <christian.koenig@amd.com>
> >>
> >>When we use an extern reservation object that otherwise waits for every
> >>fence registered with it.
> >>
> >>Signed-off-by: Christian König <christian.koenig@amd.com>
> >>Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> >>---
> >>  drivers/gpu/drm/ttm/ttm_bo.c | 18 ++++++++++++------
> >>  1 file changed, 12 insertions(+), 6 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> >>index 4cbf265..367b87b 100644
> >>--- a/drivers/gpu/drm/ttm/ttm_bo.c
> >>+++ b/drivers/gpu/drm/ttm/ttm_bo.c
> >>@@ -998,13 +998,19 @@ static int ttm_bo_move_buffer(struct ttm_buffer_object *bo,
> >>  	lockdep_assert_held(&bo->resv->lock.base);
> >>  	/*
> >>-	 * FIXME: It's possible to pipeline buffer moves.
> >>-	 * Have the driver move function wait for idle when necessary,
> >>-	 * instead of doing it here.
> >>+	 * Don't wait for the BO on initial allocation. This is important when
> >>+	 * the BO has an imported reservation object.
> >>  	 */
> >>-	ret = ttm_bo_wait(bo, false, interruptible, no_wait_gpu);
> >>-	if (ret)
> >>-		return ret;
> >>+	if (bo->mem.mem_type != TTM_PL_SYSTEM || bo->ttm != NULL) {
> >>+		/*
> >>+		 * FIXME: It's possible to pipeline buffer moves.
> >>+		 * Have the driver move function wait for idle when necessary,
> >>+		 * instead of doing it here.
> >>+		 */
> >>+		ret = ttm_bo_wait(bo, false, interruptible, no_wait_gpu);
> >>+		if (ret)
> >>+			return ret;
> >>+	}
> >>  	mem.num_pages = bo->num_pages;
> >>  	mem.size = mem.num_pages << PAGE_SHIFT;
> >>  	mem.page_alignment = bo->mem.page_alignment;
> >>-- 
> >>2.5.0
> >>
> >>_______________________________________________
> >>dri-devel mailing list
> >>dri-devel@lists.freedesktop.org
> >>https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel&d=BQID-g&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=w9Iu3o4zAy-3-s8MFvrNSQ&m=Zo5YNBtGT5RrPrdJp7kSFSPuSK1Lqnix7D0IkGfYbLs&s=oRLwDN0-euwuXivbCj29GleUbAb6tZnkp5UQZOmDCnM&e=
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 4cbf265..367b87b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -998,13 +998,19 @@  static int ttm_bo_move_buffer(struct ttm_buffer_object *bo,
 	lockdep_assert_held(&bo->resv->lock.base);
 
 	/*
-	 * FIXME: It's possible to pipeline buffer moves.
-	 * Have the driver move function wait for idle when necessary,
-	 * instead of doing it here.
+	 * Don't wait for the BO on initial allocation. This is important when
+	 * the BO has an imported reservation object.
 	 */
-	ret = ttm_bo_wait(bo, false, interruptible, no_wait_gpu);
-	if (ret)
-		return ret;
+	if (bo->mem.mem_type != TTM_PL_SYSTEM || bo->ttm != NULL) {
+		/*
+		 * FIXME: It's possible to pipeline buffer moves.
+		 * Have the driver move function wait for idle when necessary,
+		 * instead of doing it here.
+		 */
+		ret = ttm_bo_wait(bo, false, interruptible, no_wait_gpu);
+		if (ret)
+			return ret;
+	}
 	mem.num_pages = bo->num_pages;
 	mem.size = mem.num_pages << PAGE_SHIFT;
 	mem.page_alignment = bo->mem.page_alignment;