All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/radeon: Clean up reference counting and pinning of the cursor BOs
@ 2015-07-07  7:27 Michel Dänzer
  2015-07-07  7:27 ` [PATCH 2/3] drm/radeon: unpin cursor BOs on suspend and pin them again on resume Michel Dänzer
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Michel Dänzer @ 2015-07-07  7:27 UTC (permalink / raw)
  To: dri-devel

From: Michel Dänzer <michel.daenzer@amd.com>

Take a GEM reference for and pin the new cursor BO, unpin and drop the
GEM reference for the old cursor BO in radeon_crtc_cursor_set2, and use
radeon_crtc->cursor_addr in radeon_set_cursor.

This fixes radeon_cursor_reset accidentally incrementing the cursor BO
pin count, and cleans up the code a little.

Cc: stable@vger.kernel.org
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/radeon/radeon_cursor.c | 84 +++++++++++++++-------------------
 drivers/gpu/drm/radeon/radeon_mode.h   |  1 -
 2 files changed, 37 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_cursor.c b/drivers/gpu/drm/radeon/radeon_cursor.c
index 45e5406..fa66174 100644
--- a/drivers/gpu/drm/radeon/radeon_cursor.c
+++ b/drivers/gpu/drm/radeon/radeon_cursor.c
@@ -205,8 +205,9 @@ static int radeon_cursor_move_locked(struct drm_crtc *crtc, int x, int y)
 			| (x << 16)
 			| y));
 		/* offset is from DISP(2)_BASE_ADDRESS */
-		WREG32(RADEON_CUR_OFFSET + radeon_crtc->crtc_offset, (radeon_crtc->legacy_cursor_offset +
-								      (yorigin * 256)));
+		WREG32(RADEON_CUR_OFFSET + radeon_crtc->crtc_offset,
+		       radeon_crtc->cursor_addr - radeon_crtc->legacy_display_base_addr +
+		       yorigin * 256);
 	}
 
 	radeon_crtc->cursor_x = x;
@@ -227,51 +228,32 @@ int radeon_crtc_cursor_move(struct drm_crtc *crtc,
 	return ret;
 }
 
-static int radeon_set_cursor(struct drm_crtc *crtc, struct drm_gem_object *obj)
+static void radeon_set_cursor(struct drm_crtc *crtc)
 {
 	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
 	struct radeon_device *rdev = crtc->dev->dev_private;
-	struct radeon_bo *robj = gem_to_radeon_bo(obj);
-	uint64_t gpu_addr;
-	int ret;
-
-	ret = radeon_bo_reserve(robj, false);
-	if (unlikely(ret != 0))
-		goto fail;
-	/* Only 27 bit offset for legacy cursor */
-	ret = radeon_bo_pin_restricted(robj, RADEON_GEM_DOMAIN_VRAM,
-				       ASIC_IS_AVIVO(rdev) ? 0 : 1 << 27,
-				       &gpu_addr);
-	radeon_bo_unreserve(robj);
-	if (ret)
-		goto fail;
 
 	if (ASIC_IS_DCE4(rdev)) {
 		WREG32(EVERGREEN_CUR_SURFACE_ADDRESS_HIGH + radeon_crtc->crtc_offset,
-		       upper_32_bits(gpu_addr));
+		       upper_32_bits(radeon_crtc->cursor_addr));
 		WREG32(EVERGREEN_CUR_SURFACE_ADDRESS + radeon_crtc->crtc_offset,
-		       gpu_addr & 0xffffffff);
+		       lower_32_bits(radeon_crtc->cursor_addr));
 	} else if (ASIC_IS_AVIVO(rdev)) {
 		if (rdev->family >= CHIP_RV770) {
 			if (radeon_crtc->crtc_id)
-				WREG32(R700_D2CUR_SURFACE_ADDRESS_HIGH, upper_32_bits(gpu_addr));
+				WREG32(R700_D2CUR_SURFACE_ADDRESS_HIGH,
+				       upper_32_bits(radeon_crtc->cursor_addr));
 			else
-				WREG32(R700_D1CUR_SURFACE_ADDRESS_HIGH, upper_32_bits(gpu_addr));
+				WREG32(R700_D1CUR_SURFACE_ADDRESS_HIGH,
+				       upper_32_bits(radeon_crtc->cursor_addr));
 		}
 		WREG32(AVIVO_D1CUR_SURFACE_ADDRESS + radeon_crtc->crtc_offset,
-		       gpu_addr & 0xffffffff);
+		       lower_32_bits(radeon_crtc->cursor_addr));
 	} else {
-		radeon_crtc->legacy_cursor_offset = gpu_addr - radeon_crtc->legacy_display_base_addr;
 		/* offset is from DISP(2)_BASE_ADDRESS */
-		WREG32(RADEON_CUR_OFFSET + radeon_crtc->crtc_offset, radeon_crtc->legacy_cursor_offset);
+		WREG32(RADEON_CUR_OFFSET + radeon_crtc->crtc_offset,
+		       radeon_crtc->cursor_addr - radeon_crtc->legacy_display_base_addr);
 	}
-
-	return 0;
-
-fail:
-	drm_gem_object_unreference_unlocked(obj);
-
-	return ret;
 }
 
 int radeon_crtc_cursor_set2(struct drm_crtc *crtc,
@@ -283,7 +265,9 @@ int radeon_crtc_cursor_set2(struct drm_crtc *crtc,
 			    int32_t hot_y)
 {
 	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
+	struct radeon_device *rdev = crtc->dev->dev_private;
 	struct drm_gem_object *obj;
+	struct radeon_bo *robj;
 	int ret;
 
 	if (!handle) {
@@ -305,6 +289,23 @@ int radeon_crtc_cursor_set2(struct drm_crtc *crtc,
 		return -ENOENT;
 	}
 
+	robj = gem_to_radeon_bo(obj);
+	ret = radeon_bo_reserve(robj, false);
+	if (ret != 0) {
+		drm_gem_object_unreference_unlocked(obj);
+		return ret;
+	}
+	/* Only 27 bit offset for legacy cursor */
+	ret = radeon_bo_pin_restricted(robj, RADEON_GEM_DOMAIN_VRAM,
+				       ASIC_IS_AVIVO(rdev) ? 0 : 1 << 27,
+				       &radeon_crtc->cursor_addr);
+	radeon_bo_unreserve(robj);
+	if (ret) {
+		DRM_ERROR("Failed to pin new cursor BO (%d)\n", ret);
+		drm_gem_object_unreference_unlocked(obj);
+		return ret;
+	}
+
 	radeon_crtc->cursor_width = width;
 	radeon_crtc->cursor_height = height;
 
@@ -323,13 +324,8 @@ int radeon_crtc_cursor_set2(struct drm_crtc *crtc,
 		radeon_crtc->cursor_hot_y = hot_y;
 	}
 
-	ret = radeon_set_cursor(crtc, obj);
-
-	if (ret)
-		DRM_ERROR("radeon_set_cursor returned %d, not changing cursor\n",
-			  ret);
-	else
-		radeon_show_cursor(crtc);
+	radeon_set_cursor(crtc);
+	radeon_show_cursor(crtc);
 
 	radeon_lock_cursor(crtc, false);
 
@@ -341,8 +337,7 @@ unpin:
 			radeon_bo_unpin(robj);
 			radeon_bo_unreserve(robj);
 		}
-		if (radeon_crtc->cursor_bo != obj)
-			drm_gem_object_unreference_unlocked(radeon_crtc->cursor_bo);
+		drm_gem_object_unreference_unlocked(radeon_crtc->cursor_bo);
 	}
 
 	radeon_crtc->cursor_bo = obj;
@@ -360,7 +355,6 @@ unpin:
 void radeon_cursor_reset(struct drm_crtc *crtc)
 {
 	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
-	int ret;
 
 	if (radeon_crtc->cursor_bo) {
 		radeon_lock_cursor(crtc, true);
@@ -368,12 +362,8 @@ void radeon_cursor_reset(struct drm_crtc *crtc)
 		radeon_cursor_move_locked(crtc, radeon_crtc->cursor_x,
 					  radeon_crtc->cursor_y);
 
-		ret = radeon_set_cursor(crtc, radeon_crtc->cursor_bo);
-		if (ret)
-			DRM_ERROR("radeon_set_cursor returned %d, not showing "
-				  "cursor\n", ret);
-		else
-			radeon_show_cursor(crtc);
+		radeon_set_cursor(crtc);
+		radeon_show_cursor(crtc);
 
 		radeon_lock_cursor(crtc, false);
 	}
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index 6de5459..07909d8 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -343,7 +343,6 @@ struct radeon_crtc {
 	int max_cursor_width;
 	int max_cursor_height;
 	uint32_t legacy_display_base_addr;
-	uint32_t legacy_cursor_offset;
 	enum radeon_rmx_type rmx_type;
 	u8 h_border;
 	u8 v_border;
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] drm/radeon: unpin cursor BOs on suspend and pin them again on resume
  2015-07-07  7:27 [PATCH 1/3] drm/radeon: Clean up reference counting and pinning of the cursor BOs Michel Dänzer
@ 2015-07-07  7:27 ` Michel Dänzer
  2015-07-07 10:12   ` Christian König
  2015-07-07  7:27 ` [PATCH 3/3] drm/radeon: Fold radeon_set_cursor() into radeon_show_cursor() Michel Dänzer
  2015-07-08 20:33 ` [PATCH 1/3] drm/radeon: Clean up reference counting and pinning of the cursor BOs Grigori Goronzy
  2 siblings, 1 reply; 5+ messages in thread
From: Michel Dänzer @ 2015-07-07  7:27 UTC (permalink / raw)
  To: dri-devel

From: Grigori Goronzy <greg@chown.ath.cx>

Everything is evicted from VRAM before suspend, so we need to make
sure all BOs are unpinned and re-pinned after resume. Fixes broken
mouse cursor after resume introduced by commit b9729b17.

[Michel Dänzer: Add pinning BOs on resume]

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=100541
Cc: stable@vger.kernel.org
Signed-off-by: Grigori Goronzy <greg@chown.ath.cx>
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/radeon/radeon_device.c | 36 ++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 12a18c8..5a075a9 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1593,6 +1593,20 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, bool fbcon)
 		drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF);
 	}
 
+	/* unpin cursors */
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
+
+		if (radeon_crtc->cursor_bo) {
+			struct radeon_bo *robj = gem_to_radeon_bo(radeon_crtc->cursor_bo);
+			r = radeon_bo_reserve(robj, false);
+			if (r == 0) {
+				radeon_bo_unpin(robj);
+				radeon_bo_unreserve(robj);
+			}
+		}
+	}
+
 	/* unpin the front buffers */
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
 		struct radeon_framebuffer *rfb = to_radeon_framebuffer(crtc->primary->fb);
@@ -1660,6 +1674,7 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
 {
 	struct drm_connector *connector;
 	struct radeon_device *rdev = dev->dev_private;
+	struct drm_crtc *crtc;
 	int r;
 
 	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
@@ -1699,6 +1714,27 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
 
 	radeon_restore_bios_scratch_regs(rdev);
 
+	/* pin cursors */
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
+
+		if (radeon_crtc->cursor_bo) {
+			struct radeon_bo *robj = gem_to_radeon_bo(radeon_crtc->cursor_bo);
+			r = radeon_bo_reserve(robj, false);
+			if (r == 0) {
+				/* Only 27 bit offset for legacy cursor */
+				r = radeon_bo_pin_restricted(robj,
+							     RADEON_GEM_DOMAIN_VRAM,
+							     ASIC_IS_AVIVO(rdev) ?
+							     0 : 1 << 27,
+							     &radeon_crtc->cursor_addr);
+				if (r != 0)
+					DRM_ERROR("Failed to pin cursor BO (%d)\n", r);
+				radeon_bo_unreserve(robj);
+			}
+		}
+	}
+
 	/* init dig PHYs, disp eng pll */
 	if (rdev->is_atom_bios) {
 		radeon_atom_encoder_init(rdev);
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] drm/radeon: Fold radeon_set_cursor() into radeon_show_cursor()
  2015-07-07  7:27 [PATCH 1/3] drm/radeon: Clean up reference counting and pinning of the cursor BOs Michel Dänzer
  2015-07-07  7:27 ` [PATCH 2/3] drm/radeon: unpin cursor BOs on suspend and pin them again on resume Michel Dänzer
@ 2015-07-07  7:27 ` Michel Dänzer
  2015-07-08 20:33 ` [PATCH 1/3] drm/radeon: Clean up reference counting and pinning of the cursor BOs Grigori Goronzy
  2 siblings, 0 replies; 5+ messages in thread
From: Michel Dänzer @ 2015-07-07  7:27 UTC (permalink / raw)
  To: dri-devel

From: Michel Dänzer <michel.daenzer@amd.com>

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/radeon/radeon_cursor.c | 49 +++++++++++++---------------------
 1 file changed, 19 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_cursor.c b/drivers/gpu/drm/radeon/radeon_cursor.c
index fa66174..afaf346 100644
--- a/drivers/gpu/drm/radeon/radeon_cursor.c
+++ b/drivers/gpu/drm/radeon/radeon_cursor.c
@@ -91,15 +91,34 @@ static void radeon_show_cursor(struct drm_crtc *crtc)
 	struct radeon_device *rdev = crtc->dev->dev_private;
 
 	if (ASIC_IS_DCE4(rdev)) {
+		WREG32(EVERGREEN_CUR_SURFACE_ADDRESS_HIGH + radeon_crtc->crtc_offset,
+		       upper_32_bits(radeon_crtc->cursor_addr));
+		WREG32(EVERGREEN_CUR_SURFACE_ADDRESS + radeon_crtc->crtc_offset,
+		       lower_32_bits(radeon_crtc->cursor_addr));
 		WREG32(RADEON_MM_INDEX, EVERGREEN_CUR_CONTROL + radeon_crtc->crtc_offset);
 		WREG32(RADEON_MM_DATA, EVERGREEN_CURSOR_EN |
 		       EVERGREEN_CURSOR_MODE(EVERGREEN_CURSOR_24_8_PRE_MULT) |
 		       EVERGREEN_CURSOR_URGENT_CONTROL(EVERGREEN_CURSOR_URGENT_1_2));
 	} else if (ASIC_IS_AVIVO(rdev)) {
+		if (rdev->family >= CHIP_RV770) {
+			if (radeon_crtc->crtc_id)
+				WREG32(R700_D2CUR_SURFACE_ADDRESS_HIGH,
+				       upper_32_bits(radeon_crtc->cursor_addr));
+			else
+				WREG32(R700_D1CUR_SURFACE_ADDRESS_HIGH,
+				       upper_32_bits(radeon_crtc->cursor_addr));
+		}
+
+		WREG32(AVIVO_D1CUR_SURFACE_ADDRESS + radeon_crtc->crtc_offset,
+		       lower_32_bits(radeon_crtc->cursor_addr));
 		WREG32(RADEON_MM_INDEX, AVIVO_D1CUR_CONTROL + radeon_crtc->crtc_offset);
 		WREG32(RADEON_MM_DATA, AVIVO_D1CURSOR_EN |
 		       (AVIVO_D1CURSOR_MODE_24BPP << AVIVO_D1CURSOR_MODE_SHIFT));
 	} else {
+		/* offset is from DISP(2)_BASE_ADDRESS */
+		WREG32(RADEON_CUR_OFFSET + radeon_crtc->crtc_offset,
+		       radeon_crtc->cursor_addr - radeon_crtc->legacy_display_base_addr);
+
 		switch (radeon_crtc->crtc_id) {
 		case 0:
 			WREG32(RADEON_MM_INDEX, RADEON_CRTC_GEN_CNTL);
@@ -228,34 +247,6 @@ int radeon_crtc_cursor_move(struct drm_crtc *crtc,
 	return ret;
 }
 
-static void radeon_set_cursor(struct drm_crtc *crtc)
-{
-	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
-	struct radeon_device *rdev = crtc->dev->dev_private;
-
-	if (ASIC_IS_DCE4(rdev)) {
-		WREG32(EVERGREEN_CUR_SURFACE_ADDRESS_HIGH + radeon_crtc->crtc_offset,
-		       upper_32_bits(radeon_crtc->cursor_addr));
-		WREG32(EVERGREEN_CUR_SURFACE_ADDRESS + radeon_crtc->crtc_offset,
-		       lower_32_bits(radeon_crtc->cursor_addr));
-	} else if (ASIC_IS_AVIVO(rdev)) {
-		if (rdev->family >= CHIP_RV770) {
-			if (radeon_crtc->crtc_id)
-				WREG32(R700_D2CUR_SURFACE_ADDRESS_HIGH,
-				       upper_32_bits(radeon_crtc->cursor_addr));
-			else
-				WREG32(R700_D1CUR_SURFACE_ADDRESS_HIGH,
-				       upper_32_bits(radeon_crtc->cursor_addr));
-		}
-		WREG32(AVIVO_D1CUR_SURFACE_ADDRESS + radeon_crtc->crtc_offset,
-		       lower_32_bits(radeon_crtc->cursor_addr));
-	} else {
-		/* offset is from DISP(2)_BASE_ADDRESS */
-		WREG32(RADEON_CUR_OFFSET + radeon_crtc->crtc_offset,
-		       radeon_crtc->cursor_addr - radeon_crtc->legacy_display_base_addr);
-	}
-}
-
 int radeon_crtc_cursor_set2(struct drm_crtc *crtc,
 			    struct drm_file *file_priv,
 			    uint32_t handle,
@@ -324,7 +315,6 @@ int radeon_crtc_cursor_set2(struct drm_crtc *crtc,
 		radeon_crtc->cursor_hot_y = hot_y;
 	}
 
-	radeon_set_cursor(crtc);
 	radeon_show_cursor(crtc);
 
 	radeon_lock_cursor(crtc, false);
@@ -362,7 +352,6 @@ void radeon_cursor_reset(struct drm_crtc *crtc)
 		radeon_cursor_move_locked(crtc, radeon_crtc->cursor_x,
 					  radeon_crtc->cursor_y);
 
-		radeon_set_cursor(crtc);
 		radeon_show_cursor(crtc);
 
 		radeon_lock_cursor(crtc, false);
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/3] drm/radeon: unpin cursor BOs on suspend and pin them again on resume
  2015-07-07  7:27 ` [PATCH 2/3] drm/radeon: unpin cursor BOs on suspend and pin them again on resume Michel Dänzer
@ 2015-07-07 10:12   ` Christian König
  0 siblings, 0 replies; 5+ messages in thread
From: Christian König @ 2015-07-07 10:12 UTC (permalink / raw)
  To: Michel Dänzer, dri-devel

On 07.07.2015 09:27, Michel Dänzer wrote:
> From: Grigori Goronzy <greg@chown.ath.cx>
>
> Everything is evicted from VRAM before suspend, so we need to make
> sure all BOs are unpinned and re-pinned after resume. Fixes broken
> mouse cursor after resume introduced by commit b9729b17.
>
> [Michel Dänzer: Add pinning BOs on resume]
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=100541
> Cc: stable@vger.kernel.org
> Signed-off-by: Grigori Goronzy <greg@chown.ath.cx>
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> ---
>   drivers/gpu/drm/radeon/radeon_device.c | 36 ++++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> index 12a18c8..5a075a9 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -1593,6 +1593,20 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, bool fbcon)
>   		drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF);
>   	}
>   
> +	/* unpin cursors */
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {

Didn't we wanted to merge this into the other list_for_each_entry below?

Either way is fine with me and the whole series is Reviewed-by: 
Christian König <christian.koenig@amd.com> anyway.

Regards,
Christian.

> +		struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
> +
> +		if (radeon_crtc->cursor_bo) {
> +			struct radeon_bo *robj = gem_to_radeon_bo(radeon_crtc->cursor_bo);
> +			r = radeon_bo_reserve(robj, false);
> +			if (r == 0) {
> +				radeon_bo_unpin(robj);
> +				radeon_bo_unreserve(robj);
> +			}
> +		}
> +	}
> +
>   	/* unpin the front buffers */
>   	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
>   		struct radeon_framebuffer *rfb = to_radeon_framebuffer(crtc->primary->fb);
> @@ -1660,6 +1674,7 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
>   {
>   	struct drm_connector *connector;
>   	struct radeon_device *rdev = dev->dev_private;
> +	struct drm_crtc *crtc;
>   	int r;
>   
>   	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> @@ -1699,6 +1714,27 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
>   
>   	radeon_restore_bios_scratch_regs(rdev);
>   
> +	/* pin cursors */
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +		struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
> +
> +		if (radeon_crtc->cursor_bo) {
> +			struct radeon_bo *robj = gem_to_radeon_bo(radeon_crtc->cursor_bo);
> +			r = radeon_bo_reserve(robj, false);
> +			if (r == 0) {
> +				/* Only 27 bit offset for legacy cursor */
> +				r = radeon_bo_pin_restricted(robj,
> +							     RADEON_GEM_DOMAIN_VRAM,
> +							     ASIC_IS_AVIVO(rdev) ?
> +							     0 : 1 << 27,
> +							     &radeon_crtc->cursor_addr);
> +				if (r != 0)
> +					DRM_ERROR("Failed to pin cursor BO (%d)\n", r);
> +				radeon_bo_unreserve(robj);
> +			}
> +		}
> +	}
> +
>   	/* init dig PHYs, disp eng pll */
>   	if (rdev->is_atom_bios) {
>   		radeon_atom_encoder_init(rdev);

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] drm/radeon: Clean up reference counting and pinning of the cursor BOs
  2015-07-07  7:27 [PATCH 1/3] drm/radeon: Clean up reference counting and pinning of the cursor BOs Michel Dänzer
  2015-07-07  7:27 ` [PATCH 2/3] drm/radeon: unpin cursor BOs on suspend and pin them again on resume Michel Dänzer
  2015-07-07  7:27 ` [PATCH 3/3] drm/radeon: Fold radeon_set_cursor() into radeon_show_cursor() Michel Dänzer
@ 2015-07-08 20:33 ` Grigori Goronzy
  2 siblings, 0 replies; 5+ messages in thread
From: Grigori Goronzy @ 2015-07-08 20:33 UTC (permalink / raw)
  To: Michel Dänzer; +Cc: dri-devel

On 2015-07-07 09:27, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
> 
> Take a GEM reference for and pin the new cursor BO, unpin and drop the
> GEM reference for the old cursor BO in radeon_crtc_cursor_set2, and use
> radeon_crtc->cursor_addr in radeon_set_cursor.
> 
> This fixes radeon_cursor_reset accidentally incrementing the cursor BO
> pin count, and cleans up the code a little.
> 

Thank you for finishing this up!

Series is
Reviewed-by: Grigori Goronzy <greg@chown.ath.cx>

> Cc: stable@vger.kernel.org
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> ---
>  drivers/gpu/drm/radeon/radeon_cursor.c | 84 
> +++++++++++++++-------------------
>  drivers/gpu/drm/radeon/radeon_mode.h   |  1 -
>  2 files changed, 37 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_cursor.c
> b/drivers/gpu/drm/radeon/radeon_cursor.c
> index 45e5406..fa66174 100644
> --- a/drivers/gpu/drm/radeon/radeon_cursor.c
> +++ b/drivers/gpu/drm/radeon/radeon_cursor.c
> @@ -205,8 +205,9 @@ static int radeon_cursor_move_locked(struct
> drm_crtc *crtc, int x, int y)
>  			| (x << 16)
>  			| y));
>  		/* offset is from DISP(2)_BASE_ADDRESS */
> -		WREG32(RADEON_CUR_OFFSET + radeon_crtc->crtc_offset,
> (radeon_crtc->legacy_cursor_offset +
> -								      (yorigin * 256)));
> +		WREG32(RADEON_CUR_OFFSET + radeon_crtc->crtc_offset,
> +		       radeon_crtc->cursor_addr - 
> radeon_crtc->legacy_display_base_addr +
> +		       yorigin * 256);
>  	}
> 
>  	radeon_crtc->cursor_x = x;
> @@ -227,51 +228,32 @@ int radeon_crtc_cursor_move(struct drm_crtc 
> *crtc,
>  	return ret;
>  }
> 
> -static int radeon_set_cursor(struct drm_crtc *crtc, struct 
> drm_gem_object *obj)
> +static void radeon_set_cursor(struct drm_crtc *crtc)
>  {
>  	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
>  	struct radeon_device *rdev = crtc->dev->dev_private;
> -	struct radeon_bo *robj = gem_to_radeon_bo(obj);
> -	uint64_t gpu_addr;
> -	int ret;
> -
> -	ret = radeon_bo_reserve(robj, false);
> -	if (unlikely(ret != 0))
> -		goto fail;
> -	/* Only 27 bit offset for legacy cursor */
> -	ret = radeon_bo_pin_restricted(robj, RADEON_GEM_DOMAIN_VRAM,
> -				       ASIC_IS_AVIVO(rdev) ? 0 : 1 << 27,
> -				       &gpu_addr);
> -	radeon_bo_unreserve(robj);
> -	if (ret)
> -		goto fail;
> 
>  	if (ASIC_IS_DCE4(rdev)) {
>  		WREG32(EVERGREEN_CUR_SURFACE_ADDRESS_HIGH + 
> radeon_crtc->crtc_offset,
> -		       upper_32_bits(gpu_addr));
> +		       upper_32_bits(radeon_crtc->cursor_addr));
>  		WREG32(EVERGREEN_CUR_SURFACE_ADDRESS + radeon_crtc->crtc_offset,
> -		       gpu_addr & 0xffffffff);
> +		       lower_32_bits(radeon_crtc->cursor_addr));
>  	} else if (ASIC_IS_AVIVO(rdev)) {
>  		if (rdev->family >= CHIP_RV770) {
>  			if (radeon_crtc->crtc_id)
> -				WREG32(R700_D2CUR_SURFACE_ADDRESS_HIGH, upper_32_bits(gpu_addr));
> +				WREG32(R700_D2CUR_SURFACE_ADDRESS_HIGH,
> +				       upper_32_bits(radeon_crtc->cursor_addr));
>  			else
> -				WREG32(R700_D1CUR_SURFACE_ADDRESS_HIGH, upper_32_bits(gpu_addr));
> +				WREG32(R700_D1CUR_SURFACE_ADDRESS_HIGH,
> +				       upper_32_bits(radeon_crtc->cursor_addr));
>  		}
>  		WREG32(AVIVO_D1CUR_SURFACE_ADDRESS + radeon_crtc->crtc_offset,
> -		       gpu_addr & 0xffffffff);
> +		       lower_32_bits(radeon_crtc->cursor_addr));
>  	} else {
> -		radeon_crtc->legacy_cursor_offset = gpu_addr -
> radeon_crtc->legacy_display_base_addr;
>  		/* offset is from DISP(2)_BASE_ADDRESS */
> -		WREG32(RADEON_CUR_OFFSET + radeon_crtc->crtc_offset,
> radeon_crtc->legacy_cursor_offset);
> +		WREG32(RADEON_CUR_OFFSET + radeon_crtc->crtc_offset,
> +		       radeon_crtc->cursor_addr - 
> radeon_crtc->legacy_display_base_addr);
>  	}
> -
> -	return 0;
> -
> -fail:
> -	drm_gem_object_unreference_unlocked(obj);
> -
> -	return ret;
>  }
> 
>  int radeon_crtc_cursor_set2(struct drm_crtc *crtc,
> @@ -283,7 +265,9 @@ int radeon_crtc_cursor_set2(struct drm_crtc *crtc,
>  			    int32_t hot_y)
>  {
>  	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
> +	struct radeon_device *rdev = crtc->dev->dev_private;
>  	struct drm_gem_object *obj;
> +	struct radeon_bo *robj;
>  	int ret;
> 
>  	if (!handle) {
> @@ -305,6 +289,23 @@ int radeon_crtc_cursor_set2(struct drm_crtc *crtc,
>  		return -ENOENT;
>  	}
> 
> +	robj = gem_to_radeon_bo(obj);
> +	ret = radeon_bo_reserve(robj, false);
> +	if (ret != 0) {
> +		drm_gem_object_unreference_unlocked(obj);
> +		return ret;
> +	}
> +	/* Only 27 bit offset for legacy cursor */
> +	ret = radeon_bo_pin_restricted(robj, RADEON_GEM_DOMAIN_VRAM,
> +				       ASIC_IS_AVIVO(rdev) ? 0 : 1 << 27,
> +				       &radeon_crtc->cursor_addr);
> +	radeon_bo_unreserve(robj);
> +	if (ret) {
> +		DRM_ERROR("Failed to pin new cursor BO (%d)\n", ret);
> +		drm_gem_object_unreference_unlocked(obj);
> +		return ret;
> +	}
> +
>  	radeon_crtc->cursor_width = width;
>  	radeon_crtc->cursor_height = height;
> 
> @@ -323,13 +324,8 @@ int radeon_crtc_cursor_set2(struct drm_crtc *crtc,
>  		radeon_crtc->cursor_hot_y = hot_y;
>  	}
> 
> -	ret = radeon_set_cursor(crtc, obj);
> -
> -	if (ret)
> -		DRM_ERROR("radeon_set_cursor returned %d, not changing cursor\n",
> -			  ret);
> -	else
> -		radeon_show_cursor(crtc);
> +	radeon_set_cursor(crtc);
> +	radeon_show_cursor(crtc);
> 
>  	radeon_lock_cursor(crtc, false);
> 
> @@ -341,8 +337,7 @@ unpin:
>  			radeon_bo_unpin(robj);
>  			radeon_bo_unreserve(robj);
>  		}
> -		if (radeon_crtc->cursor_bo != obj)
> -			drm_gem_object_unreference_unlocked(radeon_crtc->cursor_bo);
> +		drm_gem_object_unreference_unlocked(radeon_crtc->cursor_bo);
>  	}
> 
>  	radeon_crtc->cursor_bo = obj;
> @@ -360,7 +355,6 @@ unpin:
>  void radeon_cursor_reset(struct drm_crtc *crtc)
>  {
>  	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
> -	int ret;
> 
>  	if (radeon_crtc->cursor_bo) {
>  		radeon_lock_cursor(crtc, true);
> @@ -368,12 +362,8 @@ void radeon_cursor_reset(struct drm_crtc *crtc)
>  		radeon_cursor_move_locked(crtc, radeon_crtc->cursor_x,
>  					  radeon_crtc->cursor_y);
> 
> -		ret = radeon_set_cursor(crtc, radeon_crtc->cursor_bo);
> -		if (ret)
> -			DRM_ERROR("radeon_set_cursor returned %d, not showing "
> -				  "cursor\n", ret);
> -		else
> -			radeon_show_cursor(crtc);
> +		radeon_set_cursor(crtc);
> +		radeon_show_cursor(crtc);
> 
>  		radeon_lock_cursor(crtc, false);
>  	}
> diff --git a/drivers/gpu/drm/radeon/radeon_mode.h
> b/drivers/gpu/drm/radeon/radeon_mode.h
> index 6de5459..07909d8 100644
> --- a/drivers/gpu/drm/radeon/radeon_mode.h
> +++ b/drivers/gpu/drm/radeon/radeon_mode.h
> @@ -343,7 +343,6 @@ struct radeon_crtc {
>  	int max_cursor_width;
>  	int max_cursor_height;
>  	uint32_t legacy_display_base_addr;
> -	uint32_t legacy_cursor_offset;
>  	enum radeon_rmx_type rmx_type;
>  	u8 h_border;
>  	u8 v_border;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-07-08 20:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-07  7:27 [PATCH 1/3] drm/radeon: Clean up reference counting and pinning of the cursor BOs Michel Dänzer
2015-07-07  7:27 ` [PATCH 2/3] drm/radeon: unpin cursor BOs on suspend and pin them again on resume Michel Dänzer
2015-07-07 10:12   ` Christian König
2015-07-07  7:27 ` [PATCH 3/3] drm/radeon: Fold radeon_set_cursor() into radeon_show_cursor() Michel Dänzer
2015-07-08 20:33 ` [PATCH 1/3] drm/radeon: Clean up reference counting and pinning of the cursor BOs Grigori Goronzy

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.