All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/radeon/kms: switch to condition waiting for reclocking
@ 2010-03-02 21:06 Rafał Miłecki
  2010-03-02 21:06 ` [PATCH 2/2] drm/radeon/kms: prepare for more reclocking operations Rafał Miłecki
  2010-03-03 18:47 ` [PATCH 1/2] drm/radeon/kms: switch to condition waiting for reclocking Jaime Velasco Juan
  0 siblings, 2 replies; 4+ messages in thread
From: Rafał Miłecki @ 2010-03-02 21:06 UTC (permalink / raw)
  To: dri-devel, Dave Airlie; +Cc: Jaime Velasco Juan

We tried to implement interruptible waiting with timeout (it was broken
anyway) which was not a good idea as explained by Andrew. It's possible
to avoid using additional variable but actually it inroduces using more
complex in-kernel tools. So simply add one variable for condition.

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 drivers/gpu/drm/radeon/r100.c      |    2 ++
 drivers/gpu/drm/radeon/r600.c      |    2 ++
 drivers/gpu/drm/radeon/radeon.h    |    1 +
 drivers/gpu/drm/radeon/radeon_pm.c |    8 +++++---
 drivers/gpu/drm/radeon/rs600.c     |    2 ++
 5 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index 91eb762..73f9a79 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -312,10 +312,12 @@ int r100_irq_process(struct radeon_device *rdev)
 		/* Vertical blank interrupts */
 		if (status & RADEON_CRTC_VBLANK_STAT) {
 			drm_handle_vblank(rdev->ddev, 0);
+			rdev->pm.vblank_sync = true;
 			wake_up(&rdev->irq.vblank_queue);
 		}
 		if (status & RADEON_CRTC2_VBLANK_STAT) {
 			drm_handle_vblank(rdev->ddev, 1);
+			rdev->pm.vblank_sync = true;
 			wake_up(&rdev->irq.vblank_queue);
 		}
 		if (status & RADEON_FP_DETECT_STAT) {
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index c522901..5b56a1b 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -2765,6 +2765,7 @@ restart_ih:
 			case 0: /* D1 vblank */
 				if (disp_int & LB_D1_VBLANK_INTERRUPT) {
 					drm_handle_vblank(rdev->ddev, 0);
+					rdev->pm.vblank_sync = true;
 					wake_up(&rdev->irq.vblank_queue);
 					disp_int &= ~LB_D1_VBLANK_INTERRUPT;
 					DRM_DEBUG("IH: D1 vblank\n");
@@ -2786,6 +2787,7 @@ restart_ih:
 			case 0: /* D2 vblank */
 				if (disp_int & LB_D2_VBLANK_INTERRUPT) {
 					drm_handle_vblank(rdev->ddev, 1);
+					rdev->pm.vblank_sync = true;
 					wake_up(&rdev->irq.vblank_queue);
 					disp_int &= ~LB_D2_VBLANK_INTERRUPT;
 					DRM_DEBUG("IH: D2 vblank\n");
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 829e26e..0d7caee 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -687,6 +687,7 @@ struct radeon_pm {
 	bool 			downclocked;
 	int			active_crtcs;
 	int			req_vblank;
+	bool			vblank_sync;
 	fixed20_12		max_bandwidth;
 	fixed20_12		igp_sideport_mclk;
 	fixed20_12		igp_system_mclk;
diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c
index d4d1c39..d800b86 100644
--- a/drivers/gpu/drm/radeon/radeon_pm.c
+++ b/drivers/gpu/drm/radeon/radeon_pm.c
@@ -353,10 +353,12 @@ static void radeon_pm_set_clocks(struct radeon_device *rdev)
 		rdev->pm.req_vblank |= (1 << 1);
 		drm_vblank_get(rdev->ddev, 1);
 	}
-	if (rdev->pm.active_crtcs)
-		wait_event_interruptible_timeout(
-			rdev->irq.vblank_queue, 0,
+	if (rdev->pm.active_crtcs) {
+		rdev->pm.vblank_sync = false;
+		wait_event_timeout(
+			rdev->irq.vblank_queue, rdev->pm.vblank_sync,
 			msecs_to_jiffies(RADEON_WAIT_VBLANK_TIMEOUT));
+	}
 	if (rdev->pm.req_vblank & (1 << 0)) {
 		rdev->pm.req_vblank &= ~(1 << 0);
 		drm_vblank_put(rdev->ddev, 0);
diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c
index 47f046b..ac7c27a 100644
--- a/drivers/gpu/drm/radeon/rs600.c
+++ b/drivers/gpu/drm/radeon/rs600.c
@@ -392,10 +392,12 @@ int rs600_irq_process(struct radeon_device *rdev)
 		/* Vertical blank interrupts */
 		if (G_007EDC_LB_D1_VBLANK_INTERRUPT(r500_disp_int)) {
 			drm_handle_vblank(rdev->ddev, 0);
+			rdev->pm.vblank_sync = true;
 			wake_up(&rdev->irq.vblank_queue);
 		}
 		if (G_007EDC_LB_D2_VBLANK_INTERRUPT(r500_disp_int)) {
 			drm_handle_vblank(rdev->ddev, 1);
+			rdev->pm.vblank_sync = true;
 			wake_up(&rdev->irq.vblank_queue);
 		}
 		if (G_007EDC_DC_HOT_PLUG_DETECT1_INTERRUPT(r500_disp_int)) {
-- 
1.6.4.2


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

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

* [PATCH 2/2] drm/radeon/kms: prepare for more reclocking operations
  2010-03-02 21:06 [PATCH 1/2] drm/radeon/kms: switch to condition waiting for reclocking Rafał Miłecki
@ 2010-03-02 21:06 ` Rafał Miłecki
  2010-03-03 18:47 ` [PATCH 1/2] drm/radeon/kms: switch to condition waiting for reclocking Jaime Velasco Juan
  1 sibling, 0 replies; 4+ messages in thread
From: Rafał Miłecki @ 2010-03-02 21:06 UTC (permalink / raw)
  To: dri-devel, Dave Airlie; +Cc: Jaime Velasco Juan

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 drivers/gpu/drm/radeon/radeon_pm.c |   39 ++++++++++++++++++++++++++---------
 1 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c
index d800b86..4f37b52 100644
--- a/drivers/gpu/drm/radeon/radeon_pm.c
+++ b/drivers/gpu/drm/radeon/radeon_pm.c
@@ -28,6 +28,7 @@
 #define RADEON_RECLOCK_DELAY_MS 200
 #define RADEON_WAIT_VBLANK_TIMEOUT 200
 
+static bool radeon_pm_debug_check_in_vbl(struct radeon_device *rdev, bool finish);
 static void radeon_pm_set_clocks_locked(struct radeon_device *rdev);
 static void radeon_pm_set_clocks(struct radeon_device *rdev);
 static void radeon_pm_idle_work_handler(struct work_struct *work);
@@ -179,6 +180,16 @@ static void radeon_get_power_state(struct radeon_device *rdev,
 		 rdev->pm.requested_power_state->non_clock_info.pcie_lanes);
 }
 
+static inline void radeon_sync_with_vblank(struct radeon_device *rdev)
+{
+	if (rdev->pm.active_crtcs) {
+		rdev->pm.vblank_sync = false;
+		wait_event_timeout(
+			rdev->irq.vblank_queue, rdev->pm.vblank_sync,
+			msecs_to_jiffies(RADEON_WAIT_VBLANK_TIMEOUT));
+	}
+}
+
 static void radeon_set_power_state(struct radeon_device *rdev)
 {
 	/* if *_clock_mode are the same, *_power_state are as well */
@@ -189,11 +200,28 @@ static void radeon_set_power_state(struct radeon_device *rdev)
 		 rdev->pm.requested_clock_mode->sclk,
 		 rdev->pm.requested_clock_mode->mclk,
 		 rdev->pm.requested_power_state->non_clock_info.pcie_lanes);
+
 	/* set pcie lanes */
+	/* TODO */
+
 	/* set voltage */
+	/* TODO */
+
 	/* set engine clock */
+	radeon_sync_with_vblank(rdev);
+	radeon_pm_debug_check_in_vbl(rdev, false);
 	radeon_set_engine_clock(rdev, rdev->pm.requested_clock_mode->sclk);
+	radeon_pm_debug_check_in_vbl(rdev, true);
+
+#if 0
 	/* set memory clock */
+	if (rdev->asic->set_memory_clock) {
+		radeon_sync_with_vblank(rdev);
+		radeon_pm_debug_check_in_vbl(rdev, false);
+		radeon_set_memory_clock(rdev, rdev->pm.requested_clock_mode->mclk);
+		radeon_pm_debug_check_in_vbl(rdev, true);
+	}
+#endif
 
 	rdev->pm.current_power_state = rdev->pm.requested_power_state;
 	rdev->pm.current_clock_mode = rdev->pm.requested_clock_mode;
@@ -333,10 +361,7 @@ static void radeon_pm_set_clocks_locked(struct radeon_device *rdev)
 		break;
 	}
 
-	/* check if we are in vblank */
-	radeon_pm_debug_check_in_vbl(rdev, false);
 	radeon_set_power_state(rdev);
-	radeon_pm_debug_check_in_vbl(rdev, true);
 	rdev->pm.planned_action = PM_ACTION_NONE;
 }
 
@@ -353,12 +378,7 @@ static void radeon_pm_set_clocks(struct radeon_device *rdev)
 		rdev->pm.req_vblank |= (1 << 1);
 		drm_vblank_get(rdev->ddev, 1);
 	}
-	if (rdev->pm.active_crtcs) {
-		rdev->pm.vblank_sync = false;
-		wait_event_timeout(
-			rdev->irq.vblank_queue, rdev->pm.vblank_sync,
-			msecs_to_jiffies(RADEON_WAIT_VBLANK_TIMEOUT));
-	}
+	radeon_pm_set_clocks_locked(rdev);
 	if (rdev->pm.req_vblank & (1 << 0)) {
 		rdev->pm.req_vblank &= ~(1 << 0);
 		drm_vblank_put(rdev->ddev, 0);
@@ -368,7 +388,6 @@ static void radeon_pm_set_clocks(struct radeon_device *rdev)
 		drm_vblank_put(rdev->ddev, 1);
 	}
 
-	radeon_pm_set_clocks_locked(rdev);
 	mutex_unlock(&rdev->cp.mutex);
 }
 
-- 
1.6.4.2


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

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

* Re: [PATCH 1/2] drm/radeon/kms: switch to condition waiting for reclocking
  2010-03-02 21:06 [PATCH 1/2] drm/radeon/kms: switch to condition waiting for reclocking Rafał Miłecki
  2010-03-02 21:06 ` [PATCH 2/2] drm/radeon/kms: prepare for more reclocking operations Rafał Miłecki
@ 2010-03-03 18:47 ` Jaime Velasco Juan
  2010-03-03 22:11   ` Rafał Miłecki
  1 sibling, 1 reply; 4+ messages in thread
From: Jaime Velasco Juan @ 2010-03-03 18:47 UTC (permalink / raw)
  To: Rafał Miłecki; +Cc: dri-devel

El mar. 02 de mar. de 2010, a las 22:06:51 +0100, Rafał Miłecki escribió:
> We tried to implement interruptible waiting with timeout (it was broken
> anyway) which was not a good idea as explained by Andrew. It's possible
> to avoid using additional variable but actually it inroduces using more
> complex in-kernel tools. So simply add one variable for condition.
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---

This seems to work now, although there are "not in vbl..." messages. It's
possible there are some corruptions still, but I haven't noticed any:

[  479.503327] [drm] Requested: e: 68000 m: 80000 p: 16
[  479.503339] [drm] Setting: e: 68000 m: 80000 p: 16
[  479.503383] [drm] not in vbl for pm change 00020002 00000000 at entry
[  479.503525] [drm] not in vbl for pm change 00020002 00000000 at exit
[  479.903366] [drm] Requested: e: 11000 m: 40500 p: 16
[  479.903376] [drm] Setting: e: 11000 m: 40500 p: 16
[  483.106679] [drm] Requested: e: 68000 m: 80000 p: 16
[  483.106689] [drm] Setting: e: 68000 m: 80000 p: 16
[  483.118542] [drm] not in vbl for pm change 00020002 00000000 at exit
[  483.616689] [drm] Requested: e: 11000 m: 40500 p: 16
[  483.616698] [drm] Setting: e: 11000 m: 40500 p: 16
[  483.617637] [drm] not in vbl for pm change 00020002 00000000 at exit

Tested-by: Jaime Velasco Juan <jsagarribay@gmail.com>

Thanks.

>  drivers/gpu/drm/radeon/r100.c      |    2 ++
>  drivers/gpu/drm/radeon/r600.c      |    2 ++
>  drivers/gpu/drm/radeon/radeon.h    |    1 +
>  drivers/gpu/drm/radeon/radeon_pm.c |    8 +++++---
>  drivers/gpu/drm/radeon/rs600.c     |    2 ++
>  5 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
> index 91eb762..73f9a79 100644
> --- a/drivers/gpu/drm/radeon/r100.c
> +++ b/drivers/gpu/drm/radeon/r100.c
> @@ -312,10 +312,12 @@ int r100_irq_process(struct radeon_device *rdev)
>  		/* Vertical blank interrupts */
>  		if (status & RADEON_CRTC_VBLANK_STAT) {
>  			drm_handle_vblank(rdev->ddev, 0);
> +			rdev->pm.vblank_sync = true;
>  			wake_up(&rdev->irq.vblank_queue);
>  		}
>  		if (status & RADEON_CRTC2_VBLANK_STAT) {
>  			drm_handle_vblank(rdev->ddev, 1);
> +			rdev->pm.vblank_sync = true;
>  			wake_up(&rdev->irq.vblank_queue);
>  		}
>  		if (status & RADEON_FP_DETECT_STAT) {
> diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
> index c522901..5b56a1b 100644
> --- a/drivers/gpu/drm/radeon/r600.c
> +++ b/drivers/gpu/drm/radeon/r600.c
> @@ -2765,6 +2765,7 @@ restart_ih:
>  			case 0: /* D1 vblank */
>  				if (disp_int & LB_D1_VBLANK_INTERRUPT) {
>  					drm_handle_vblank(rdev->ddev, 0);
> +					rdev->pm.vblank_sync = true;
>  					wake_up(&rdev->irq.vblank_queue);
>  					disp_int &= ~LB_D1_VBLANK_INTERRUPT;
>  					DRM_DEBUG("IH: D1 vblank\n");
> @@ -2786,6 +2787,7 @@ restart_ih:
>  			case 0: /* D2 vblank */
>  				if (disp_int & LB_D2_VBLANK_INTERRUPT) {
>  					drm_handle_vblank(rdev->ddev, 1);
> +					rdev->pm.vblank_sync = true;
>  					wake_up(&rdev->irq.vblank_queue);
>  					disp_int &= ~LB_D2_VBLANK_INTERRUPT;
>  					DRM_DEBUG("IH: D2 vblank\n");
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 829e26e..0d7caee 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -687,6 +687,7 @@ struct radeon_pm {
>  	bool 			downclocked;
>  	int			active_crtcs;
>  	int			req_vblank;
> +	bool			vblank_sync;
>  	fixed20_12		max_bandwidth;
>  	fixed20_12		igp_sideport_mclk;
>  	fixed20_12		igp_system_mclk;
> diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c
> index d4d1c39..d800b86 100644
> --- a/drivers/gpu/drm/radeon/radeon_pm.c
> +++ b/drivers/gpu/drm/radeon/radeon_pm.c
> @@ -353,10 +353,12 @@ static void radeon_pm_set_clocks(struct radeon_device *rdev)
>  		rdev->pm.req_vblank |= (1 << 1);
>  		drm_vblank_get(rdev->ddev, 1);
>  	}
> -	if (rdev->pm.active_crtcs)
> -		wait_event_interruptible_timeout(
> -			rdev->irq.vblank_queue, 0,
> +	if (rdev->pm.active_crtcs) {
> +		rdev->pm.vblank_sync = false;
> +		wait_event_timeout(
> +			rdev->irq.vblank_queue, rdev->pm.vblank_sync,
>  			msecs_to_jiffies(RADEON_WAIT_VBLANK_TIMEOUT));
> +	}
>  	if (rdev->pm.req_vblank & (1 << 0)) {
>  		rdev->pm.req_vblank &= ~(1 << 0);
>  		drm_vblank_put(rdev->ddev, 0);
> diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c
> index 47f046b..ac7c27a 100644
> --- a/drivers/gpu/drm/radeon/rs600.c
> +++ b/drivers/gpu/drm/radeon/rs600.c
> @@ -392,10 +392,12 @@ int rs600_irq_process(struct radeon_device *rdev)
>  		/* Vertical blank interrupts */
>  		if (G_007EDC_LB_D1_VBLANK_INTERRUPT(r500_disp_int)) {
>  			drm_handle_vblank(rdev->ddev, 0);
> +			rdev->pm.vblank_sync = true;
>  			wake_up(&rdev->irq.vblank_queue);
>  		}
>  		if (G_007EDC_LB_D2_VBLANK_INTERRUPT(r500_disp_int)) {
>  			drm_handle_vblank(rdev->ddev, 1);
> +			rdev->pm.vblank_sync = true;
>  			wake_up(&rdev->irq.vblank_queue);
>  		}
>  		if (G_007EDC_DC_HOT_PLUG_DETECT1_INTERRUPT(r500_disp_int)) {
> -- 
> 1.6.4.2

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

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

* Re: [PATCH 1/2] drm/radeon/kms: switch to condition waiting for reclocking
  2010-03-03 18:47 ` [PATCH 1/2] drm/radeon/kms: switch to condition waiting for reclocking Jaime Velasco Juan
@ 2010-03-03 22:11   ` Rafał Miłecki
  0 siblings, 0 replies; 4+ messages in thread
From: Rafał Miłecki @ 2010-03-03 22:11 UTC (permalink / raw)
  To: Jaime Velasco Juan; +Cc: dri-devel

W dniu 3 marca 2010 19:47 użytkownik Jaime Velasco Juan
<jsagarribay@gmail.com> napisał:
> El mar. 02 de mar. de 2010, a las 22:06:51 +0100, Rafał Miłecki escribió:
>> We tried to implement interruptible waiting with timeout (it was broken
>> anyway) which was not a good idea as explained by Andrew. It's possible
>> to avoid using additional variable but actually it inroduces using more
>> complex in-kernel tools. So simply add one variable for condition.
>>
>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>> ---
>
> This seems to work now, although there are "not in vbl..." messages. It's
> possible there are some corruptions still, but I haven't noticed any:
>
> [  479.503327] [drm] Requested: e: 68000 m: 80000 p: 16
> [  479.503339] [drm] Setting: e: 68000 m: 80000 p: 16
> [  479.503383] [drm] not in vbl for pm change 00020002 00000000 at entry
> [  479.503525] [drm] not in vbl for pm change 00020002 00000000 at exit
> [  479.903366] [drm] Requested: e: 11000 m: 40500 p: 16
> [  479.903376] [drm] Setting: e: 11000 m: 40500 p: 16
> [  483.106679] [drm] Requested: e: 68000 m: 80000 p: 16
> [  483.106689] [drm] Setting: e: 68000 m: 80000 p: 16
> [  483.118542] [drm] not in vbl for pm change 00020002 00000000 at exit
> [  483.616689] [drm] Requested: e: 11000 m: 40500 p: 16
> [  483.616698] [drm] Setting: e: 11000 m: 40500 p: 16
> [  483.617637] [drm] not in vbl for pm change 00020002 00000000 at exit
>
> Tested-by: Jaime Velasco Juan <jsagarribay@gmail.com>

Thanks Jaime. I'm afraid that not being in vblank is not really PM-strict issue.

Let me test & prepare patch for testing.

-- 
Rafał

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

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

end of thread, other threads:[~2010-03-03 22:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-02 21:06 [PATCH 1/2] drm/radeon/kms: switch to condition waiting for reclocking Rafał Miłecki
2010-03-02 21:06 ` [PATCH 2/2] drm/radeon/kms: prepare for more reclocking operations Rafał Miłecki
2010-03-03 18:47 ` [PATCH 1/2] drm/radeon/kms: switch to condition waiting for reclocking Jaime Velasco Juan
2010-03-03 22:11   ` Rafał Miłecki

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.