[PATCH 1/3] drm/radeon: use RCU query for GEM_BUSY syscall

[PATCH 1/3] drm/radeon: use RCU query for GEM_BUSY syscall

[Grigori Goronzy]

We don't need to call the (expensive) radeon_bo_wait, checking the
fences via RCU is much faster. The reservation done by radeon_bo_wait
does not save us from any race conditions.
---
 drivers/gpu/drm/radeon/radeon_gem.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index ac3c131..7199e19 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -428,7 +428,6 @@ int radeon_gem_mmap_ioctl(struct drm_device *dev, void *data,
 int radeon_gem_busy_ioctl(struct drm_device *dev, void *data,
 			  struct drm_file *filp)
 {
-	struct radeon_device *rdev = dev->dev_private;
 	struct drm_radeon_gem_busy *args = data;
 	struct drm_gem_object *gobj;
 	struct radeon_bo *robj;
@@ -440,10 +439,16 @@ int radeon_gem_busy_ioctl(struct drm_device *dev, void *data,
 		return -ENOENT;
 	}
 	robj = gem_to_radeon_bo(gobj);
-	r = radeon_bo_wait(robj, &cur_placement, true);
+
+	r = reservation_object_test_signaled_rcu(robj->tbo.resv, true);
+	if (r == 0)
+		r = -EBUSY;
+	else
+		r = 0;
+
+	cur_placement = ACCESS_ONCE(robj->tbo.mem.mem_type);
 	args->domain = radeon_mem_type_to_domain(cur_placement);
 	drm_gem_object_unreference_unlocked(gobj);
-	r = radeon_gem_handle_lockup(rdev, r);
 	return r;
 }
 
-- 
1.9.1

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

[PATCH 2/3] drm/radeon: fix HDP flushing

[Grigori Goronzy]

This was regressed by commit 39e7f6f8, although I don't know of any
actual issues caused by it.

The storage domain is read without TTM locking now, but the lock
never helped to prevent any actual races.

Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_gem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index 7199e19..013ec71 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -476,6 +476,7 @@ int radeon_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
 		r = ret;
 
 	/* Flush HDP cache via MMIO if necessary */
+	cur_placement = ACCESS_ONCE(robj->tbo.mem.mem_type);
 	if (rdev->asic->mmio_hdp_flush &&
 	    radeon_mem_type_to_domain(cur_placement) == RADEON_GEM_DOMAIN_VRAM)
 		robj->rdev->asic->mmio_hdp_flush(rdev);
-- 
1.9.1

[PATCH 2/3] drm/radeon: fix HDP flushing

[Grigori Goronzy]

This was regressed by commit 39e7f6f8, although I don't know of any
actual issues caused by it.

The storage domain is read without TTM locking now, but the lock
never helped to prevent any actual races.

Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_gem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index 7199e19..013ec71 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -476,6 +476,7 @@ int radeon_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
 		r = ret;
 
 	/* Flush HDP cache via MMIO if necessary */
+	cur_placement = ACCESS_ONCE(robj->tbo.mem.mem_type);
 	if (rdev->asic->mmio_hdp_flush &&
 	    radeon_mem_type_to_domain(cur_placement) == RADEON_GEM_DOMAIN_VRAM)
 		robj->rdev->asic->mmio_hdp_flush(rdev);
-- 
1.9.1

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

[PATCH 3/3] drm/radeon: default to 2048 MB GART size on SI+

[Grigori Goronzy]

Newer ASICs have more VRAM on average and allocating more GART as
well can have advantages. Also see commit edcd26e8.

Ideally, we should scale GART size based on actual VRAM size, but
that requires significant restructuring of initialization.
---
 drivers/gpu/drm/radeon/radeon_device.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index a7fdfa4..b6769b0 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1098,7 +1098,9 @@ static void radeon_check_arguments(struct radeon_device *rdev)
 
 	if (radeon_gart_size == -1) {
 		/* default to a larger gart size on newer asics */
-		if (rdev->family >= CHIP_RV770)
+		if (rdev->family >= CHIP_TAHITI)
+			radeon_gart_size = 2048;
+		else if (rdev->family >= CHIP_RV770)
 			radeon_gart_size = 1024;
 		else
 			radeon_gart_size = 512;
-- 
1.9.1

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

Re: [PATCH 2/3] drm/radeon: fix HDP flushing

[Greg KH]

On Thu, Jul 02, 2015 at 11:41:11AM +0200, Grigori Goronzy wrote:
> This was regressed by commit 39e7f6f8, although I don't know of any
> actual issues caused by it.
> 
> The storage domain is read without TTM locking now, but the lock
> never helped to prevent any actual races.
> 
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/radeon/radeon_gem.c | 1 +
>  1 file changed, 1 insertion(+)

No signed-off-by?

Re: [PATCH 2/3] drm/radeon: fix HDP flushing

[Greg KH]

On Thu, Jul 02, 2015 at 11:41:11AM +0200, Grigori Goronzy wrote:
> This was regressed by commit 39e7f6f8, although I don't know of any
> actual issues caused by it.
> 
> The storage domain is read without TTM locking now, but the lock
> never helped to prevent any actual races.
> 
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/radeon/radeon_gem.c | 1 +
>  1 file changed, 1 insertion(+)

No signed-off-by?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/radeon: use RCU query for GEM_BUSY syscall

[Alex Deucher]

On Thu, Jul 2, 2015 at 5:41 AM, Grigori Goronzy <greg@chown.ath.cx> wrote:
> We don't need to call the (expensive) radeon_bo_wait, checking the
> fences via RCU is much faster. The reservation done by radeon_bo_wait
> does not save us from any race conditions.

Can you add your signed-off by to these patches?

Alex

> ---
>  drivers/gpu/drm/radeon/radeon_gem.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
> index ac3c131..7199e19 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -428,7 +428,6 @@ int radeon_gem_mmap_ioctl(struct drm_device *dev, void *data,
>  int radeon_gem_busy_ioctl(struct drm_device *dev, void *data,
>                           struct drm_file *filp)
>  {
> -       struct radeon_device *rdev = dev->dev_private;
>         struct drm_radeon_gem_busy *args = data;
>         struct drm_gem_object *gobj;
>         struct radeon_bo *robj;
> @@ -440,10 +439,16 @@ int radeon_gem_busy_ioctl(struct drm_device *dev, void *data,
>                 return -ENOENT;
>         }
>         robj = gem_to_radeon_bo(gobj);
> -       r = radeon_bo_wait(robj, &cur_placement, true);
> +
> +       r = reservation_object_test_signaled_rcu(robj->tbo.resv, true);
> +       if (r == 0)
> +               r = -EBUSY;
> +       else
> +               r = 0;
> +
> +       cur_placement = ACCESS_ONCE(robj->tbo.mem.mem_type);
>         args->domain = radeon_mem_type_to_domain(cur_placement);
>         drm_gem_object_unreference_unlocked(gobj);
> -       r = radeon_gem_handle_lockup(rdev, r);
>         return r;
>  }
>
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel