diff mbox

[1/1] UBUNTU: psb: fully disable detearing code paths as a run time option

Message ID 1270651003-11342-2-git-send-email-colin.king@canonical.com
State Accepted
Delegated to: Andy Whitcroft
Headers show

Commit Message

Colin Ian King April 7, 2010, 2:36 p.m. UTC 
From: Colin Ian King <colin.king@canonical.com>

BugLink: http://bugs.launchpad.net/bugs/556710

Extend the semantics of the psb detear module paramater to totally
disable the detearing code paths.  This makes detear=0 functionally
indentical to compiling the psb driver with PSB_DETEAR not defined
(and hence disabled) in psb_drv.h. This provides the driver with the
ability to totally disable detearing at module load time rather than
at compile time.

Disabling detearing is a workaround to a couple of known issues:

1) Playing MS-MPEG4 encoded video with Xv which causes X to crash
2) Playing a video while the screen is turned off (with DPMPS or
xrandr) which causes X to hang

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 ubuntu/media/drm-poulsbo/psb_irq.c      |    2 +-
 ubuntu/media/drm-poulsbo/psb_schedule.c |    2 +-
 ubuntu/media/drm-poulsbo/psb_sgx.c      |    6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

Comments

Amit Kucheria April 7, 2010, 4:03 p.m. UTC | #1 
Clarified offline from Colin that that drm_psb_detear is already defined in
the driver but not used to do much currently.

Change appears only in the #ifdef PSB_DETEAR blocks of code and look good.

Acked-by: Amit Kucheria <amit.kucheria@canonical.com>


On 10 Apr 07, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/556710
> 
> Extend the semantics of the psb detear module paramater to totally
> disable the detearing code paths.  This makes detear=0 functionally
> indentical to compiling the psb driver with PSB_DETEAR not defined
> (and hence disabled) in psb_drv.h. This provides the driver with the
> ability to totally disable detearing at module load time rather than
> at compile time.
> 
> Disabling detearing is a workaround to a couple of known issues:
> 
> 1) Playing MS-MPEG4 encoded video with Xv which causes X to crash
> 2) Playing a video while the screen is turned off (with DPMPS or
> xrandr) which causes X to hang
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  ubuntu/media/drm-poulsbo/psb_irq.c      |    2 +-
>  ubuntu/media/drm-poulsbo/psb_schedule.c |    2 +-
>  ubuntu/media/drm-poulsbo/psb_sgx.c      |    6 +++---
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/ubuntu/media/drm-poulsbo/psb_irq.c b/ubuntu/media/drm-poulsbo/psb_irq.c
> index f3e19c6..51e1c41 100644
> --- a/ubuntu/media/drm-poulsbo/psb_irq.c
> +++ b/ubuntu/media/drm-poulsbo/psb_irq.c
> @@ -197,7 +197,7 @@ irqreturn_t psb_irq_handler(DRM_IRQ_ARGS)
>  
>  	if (vdc_stat) {
>  #ifdef PSB_DETEAR
> -		if(psb_blit_info.cmd_ready) {
> +		if (drm_psb_detear && psb_blit_info.cmd_ready) {
>  			psb_blit_info.cmd_ready = 0;
>  			psb_blit_2d_reg_write(dev_priv, psb_blit_info.cmdbuf);
>  			/* to resume the blocked psb_cmdbuf_2d() */
> diff --git a/ubuntu/media/drm-poulsbo/psb_schedule.c b/ubuntu/media/drm-poulsbo/psb_schedule.c
> index 959f8f9..a4b4021 100644
> --- a/ubuntu/media/drm-poulsbo/psb_schedule.c
> +++ b/ubuntu/media/drm-poulsbo/psb_schedule.c
> @@ -1208,7 +1208,7 @@ static int psb_setup_task_devlocked(struct drm_device *dev,
>  		task->scene = psb_scene_ref(scene);
>  
>  #ifdef PSB_DETEAR
> -	if(PSB_VIDEO_BLIT == arg->sVideoInfo.flag) {
> +	if (drm_psb_detear && PSB_VIDEO_BLIT == arg->sVideoInfo.flag) {
>  		task->bVideoFlag = PSB_VIDEO_BLIT;
>  		task->x = arg->sVideoInfo.x;
>  		task->y = arg->sVideoInfo.y;
> diff --git a/ubuntu/media/drm-poulsbo/psb_sgx.c b/ubuntu/media/drm-poulsbo/psb_sgx.c
> index 3027113..1722877 100644
> --- a/ubuntu/media/drm-poulsbo/psb_sgx.c
> +++ b/ubuntu/media/drm-poulsbo/psb_sgx.c
> @@ -177,7 +177,7 @@ int psb_2d_submit(struct drm_psb_private *dev_priv, uint32_t * cmdbuf,
>  #ifdef PSB_DETEAR
>  		/* delayed 2D blit tasks are not executed right now,
>  		   let's save a copy of the task */
> -		if(dev_priv->blit_2d) {
> +		if (drm_psb_detear && dev_priv->blit_2d) {
>  			/* FIXME: should use better approach other
>  			   than the dev_priv->blit_2d to distinguish
>  			   delayed 2D blit tasks */
> @@ -929,7 +929,7 @@ static int psb_cmdbuf_2d(struct drm_file *priv,
>  		return -EAGAIN;
>  
>  #ifdef PSB_DETEAR
> -	if(arg->sVideoInfo.flag == (PSB_DELAYED_2D_BLIT)) {
> +	if (drm_psb_detear && arg->sVideoInfo.flag == (PSB_DELAYED_2D_BLIT)) {
>  		dev_priv->blit_2d = 1;
>  	}
>  #endif	/* PSB_DETEAR */
> @@ -940,7 +940,7 @@ static int psb_cmdbuf_2d(struct drm_file *priv,
>  		goto out_unlock;
>  
>  #ifdef PSB_DETEAR
> -	if(arg->sVideoInfo.flag == (PSB_DELAYED_2D_BLIT)) {
> +	if (drm_psb_detear && arg->sVideoInfo.flag == (PSB_DELAYED_2D_BLIT)) {
>  		arg->sVideoInfo.flag = 0;
>  		clear_bit(0, &psb_blit_info.vdc_bit);
>  		psb_blit_info.cmd_ready = 1;
> -- 
> 1.6.3.3
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Andy Whitcroft April 8, 2010, 8:17 a.m. UTC | #2 
On Wed, Apr 07, 2010 at 03:36:43PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/556710
> 
> Extend the semantics of the psb detear module paramater to totally
> disable the detearing code paths.  This makes detear=0 functionally
> indentical to compiling the psb driver with PSB_DETEAR not defined
> (and hence disabled) in psb_drv.h. This provides the driver with the
> ability to totally disable detearing at module load time rather than
> at compile time.
> 
> Disabling detearing is a workaround to a couple of known issues:
> 
> 1) Playing MS-MPEG4 encoded video with Xv which causes X to crash
> 2) Playing a video while the screen is turned off (with DPMPS or
> xrandr) which causes X to hang
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  ubuntu/media/drm-poulsbo/psb_irq.c      |    2 +-
>  ubuntu/media/drm-poulsbo/psb_schedule.c |    2 +-
>  ubuntu/media/drm-poulsbo/psb_sgx.c      |    6 +++---
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/ubuntu/media/drm-poulsbo/psb_irq.c b/ubuntu/media/drm-poulsbo/psb_irq.c
> index f3e19c6..51e1c41 100644
> --- a/ubuntu/media/drm-poulsbo/psb_irq.c
> +++ b/ubuntu/media/drm-poulsbo/psb_irq.c
> @@ -197,7 +197,7 @@ irqreturn_t psb_irq_handler(DRM_IRQ_ARGS)
>  
>  	if (vdc_stat) {
>  #ifdef PSB_DETEAR
> -		if(psb_blit_info.cmd_ready) {
> +		if (drm_psb_detear && psb_blit_info.cmd_ready) {
>  			psb_blit_info.cmd_ready = 0;
>  			psb_blit_2d_reg_write(dev_priv, psb_blit_info.cmdbuf);
>  			/* to resume the blocked psb_cmdbuf_2d() */
> diff --git a/ubuntu/media/drm-poulsbo/psb_schedule.c b/ubuntu/media/drm-poulsbo/psb_schedule.c
> index 959f8f9..a4b4021 100644
> --- a/ubuntu/media/drm-poulsbo/psb_schedule.c
> +++ b/ubuntu/media/drm-poulsbo/psb_schedule.c
> @@ -1208,7 +1208,7 @@ static int psb_setup_task_devlocked(struct drm_device *dev,
>  		task->scene = psb_scene_ref(scene);
>  
>  #ifdef PSB_DETEAR
> -	if(PSB_VIDEO_BLIT == arg->sVideoInfo.flag) {
> +	if (drm_psb_detear && PSB_VIDEO_BLIT == arg->sVideoInfo.flag) {
>  		task->bVideoFlag = PSB_VIDEO_BLIT;
>  		task->x = arg->sVideoInfo.x;
>  		task->y = arg->sVideoInfo.y;
> diff --git a/ubuntu/media/drm-poulsbo/psb_sgx.c b/ubuntu/media/drm-poulsbo/psb_sgx.c
> index 3027113..1722877 100644
> --- a/ubuntu/media/drm-poulsbo/psb_sgx.c
> +++ b/ubuntu/media/drm-poulsbo/psb_sgx.c
> @@ -177,7 +177,7 @@ int psb_2d_submit(struct drm_psb_private *dev_priv, uint32_t * cmdbuf,
>  #ifdef PSB_DETEAR
>  		/* delayed 2D blit tasks are not executed right now,
>  		   let's save a copy of the task */
> -		if(dev_priv->blit_2d) {
> +		if (drm_psb_detear && dev_priv->blit_2d) {
>  			/* FIXME: should use better approach other
>  			   than the dev_priv->blit_2d to distinguish
>  			   delayed 2D blit tasks */
> @@ -929,7 +929,7 @@ static int psb_cmdbuf_2d(struct drm_file *priv,
>  		return -EAGAIN;
>  
>  #ifdef PSB_DETEAR
> -	if(arg->sVideoInfo.flag == (PSB_DELAYED_2D_BLIT)) {
> +	if (drm_psb_detear && arg->sVideoInfo.flag == (PSB_DELAYED_2D_BLIT)) {
>  		dev_priv->blit_2d = 1;
>  	}
>  #endif	/* PSB_DETEAR */
> @@ -940,7 +940,7 @@ static int psb_cmdbuf_2d(struct drm_file *priv,
>  		goto out_unlock;
>  
>  #ifdef PSB_DETEAR
> -	if(arg->sVideoInfo.flag == (PSB_DELAYED_2D_BLIT)) {
> +	if (drm_psb_detear && arg->sVideoInfo.flag == (PSB_DELAYED_2D_BLIT)) {
>  		arg->sVideoInfo.flag = 0;
>  		clear_bit(0, &psb_blit_info.vdc_bit);
>  		psb_blit_info.cmd_ready = 1;

drm_psb_detear does seem to be an existing module parameter, which
defaults true.  The bits seem to only be under DETEAR so it does appear
that this code stops additional code which applies detearing.
Seems it has been tested and confirmed working on affected devices.

Acked-by: Andy Whitcroft <apw@canonical.com>

-apw
diff mbox

Patch

diff --git a/ubuntu/media/drm-poulsbo/psb_irq.c b/ubuntu/media/drm-poulsbo/psb_irq.c
index f3e19c6..51e1c41 100644
--- a/ubuntu/media/drm-poulsbo/psb_irq.c
+++ b/ubuntu/media/drm-poulsbo/psb_irq.c
@@ -197,7 +197,7 @@  irqreturn_t psb_irq_handler(DRM_IRQ_ARGS)
 
 	if (vdc_stat) {
 #ifdef PSB_DETEAR
-		if(psb_blit_info.cmd_ready) {
+		if (drm_psb_detear && psb_blit_info.cmd_ready) {
 			psb_blit_info.cmd_ready = 0;
 			psb_blit_2d_reg_write(dev_priv, psb_blit_info.cmdbuf);
 			/* to resume the blocked psb_cmdbuf_2d() */
diff --git a/ubuntu/media/drm-poulsbo/psb_schedule.c b/ubuntu/media/drm-poulsbo/psb_schedule.c
index 959f8f9..a4b4021 100644
--- a/ubuntu/media/drm-poulsbo/psb_schedule.c
+++ b/ubuntu/media/drm-poulsbo/psb_schedule.c
@@ -1208,7 +1208,7 @@  static int psb_setup_task_devlocked(struct drm_device *dev,
 		task->scene = psb_scene_ref(scene);
 
 #ifdef PSB_DETEAR
-	if(PSB_VIDEO_BLIT == arg->sVideoInfo.flag) {
+	if (drm_psb_detear && PSB_VIDEO_BLIT == arg->sVideoInfo.flag) {
 		task->bVideoFlag = PSB_VIDEO_BLIT;
 		task->x = arg->sVideoInfo.x;
 		task->y = arg->sVideoInfo.y;
diff --git a/ubuntu/media/drm-poulsbo/psb_sgx.c b/ubuntu/media/drm-poulsbo/psb_sgx.c
index 3027113..1722877 100644
--- a/ubuntu/media/drm-poulsbo/psb_sgx.c
+++ b/ubuntu/media/drm-poulsbo/psb_sgx.c
@@ -177,7 +177,7 @@  int psb_2d_submit(struct drm_psb_private *dev_priv, uint32_t * cmdbuf,
 #ifdef PSB_DETEAR
 		/* delayed 2D blit tasks are not executed right now,
 		   let's save a copy of the task */
-		if(dev_priv->blit_2d) {
+		if (drm_psb_detear && dev_priv->blit_2d) {
 			/* FIXME: should use better approach other
 			   than the dev_priv->blit_2d to distinguish
 			   delayed 2D blit tasks */
@@ -929,7 +929,7 @@  static int psb_cmdbuf_2d(struct drm_file *priv,
 		return -EAGAIN;
 
 #ifdef PSB_DETEAR
-	if(arg->sVideoInfo.flag == (PSB_DELAYED_2D_BLIT)) {
+	if (drm_psb_detear && arg->sVideoInfo.flag == (PSB_DELAYED_2D_BLIT)) {
 		dev_priv->blit_2d = 1;
 	}
 #endif	/* PSB_DETEAR */
@@ -940,7 +940,7 @@  static int psb_cmdbuf_2d(struct drm_file *priv,
 		goto out_unlock;
 
 #ifdef PSB_DETEAR
-	if(arg->sVideoInfo.flag == (PSB_DELAYED_2D_BLIT)) {
+	if (drm_psb_detear && arg->sVideoInfo.flag == (PSB_DELAYED_2D_BLIT)) {
 		arg->sVideoInfo.flag = 0;
 		clear_bit(0, &psb_blit_info.vdc_bit);
 		psb_blit_info.cmd_ready = 1;